Re: [PATCH 1/2] dt-bindings: display: panel: Add NewVision NV3051D panel bindings

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 07/09/2022 15:35, Chris Morgan wrote:
> On Wed, Sep 07, 2022 at 02:53:56PM +0200, Krzysztof Kozlowski wrote:
>> On 06/09/2022 20:52, Chris Morgan wrote:
>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>
>>> Add documentation for the NewVision NV3051D panel bindings.
>>> Note that for the two expected consumers of this panel binding
>>> the underlying LCD model is unknown.
>>>
>>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>>> ---
>>>  .../display/panel/newvision,nv3051d.yaml      | 48 +++++++++++++++++++
>>>  1 file changed, 48 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml
>>> new file mode 100644
>>> index 000000000000..016168d8d7b2
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml
>>> @@ -0,0 +1,48 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: https://nam12.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fdisplay%2Fpanel%2Fnewvision%2Cnv3051d.yaml%23&amp;data=05%7C01%7C%7C69d30de15aea41517acb08da90d0079f%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637981520397977782%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=qfuvbrQYP3rKnp%2ByPmPmn%2BCJJOQkNkTGT49FkJmIics%3D&amp;reserved=0
>>
>> You need to document vendor prefix... but the filename does not match
>> compatible.
> 
> Okay, will do that. This is a tricky one because while I know the panel
> controller IC (a NewVision NV3051D) I don't actually know the LCD panel
> itself, the vendor is somewhat tight lipped. I do know the product it
> goes into, so that's why I did what I did with the compatible strings.
> If that's not correct I guess let me know. I did see for other drivers
> (such as the NewVision NV3052C) the driver was written for the IC
> and the panel name was listed differently, hence what I was going for
> here.

If by "driver" you mean by "Linux driver", then it does not really
matter. You describe here the hardware. The example of NV3052C follows
this approach - proper compatible and file name matching hardware. Here
your file name does not match hardware.

> 
>>
>>> +$schema: https://nam12.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=05%7C01%7C%7C69d30de15aea41517acb08da90d0079f%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637981520397977782%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=AYwypW%2BA9xWCN6YYwC7oI5UDW6QmiP7%2FmAoKlm7x3jM%3D&amp;reserved=0
>>> +
>>> +title: NewVision NV3051D based DSI panel driver
>>
>> This is confusing - compatibles say something else.
>>
> 
> Right. Driver IC is the NV3051D, LCD panel itself is... well... not
> sure...

I guess similarly to ltk035c5444t this should be documentation of panel?

> 
>>> +
>>> +maintainers:
>>> +  - Chris Morgan <macromorgan@hotmail.com>
>>> +
>>> +allOf:
>>> +  - $ref: panel-common.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - anbernic,rg353p-panel
>>> +      - anbernic,rg353v-panel
>>
>> Missing space, missing documentation for vendor prefix.
>>
>> Strip "panel" suffix unless device is multifunctional.
>>
> 
> The device name itself is Anbernic RG353P (and RG353V). The driver is
> not multifunctional but again I don't really know what the LCD itself
> is called so I'm trying to name it after the device. I only know the
> driver IC.

So skip panel.



Best regards,
Krzysztof