diff mbox series

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

Message ID 20220906185208.13395-2-macroalpha82@gmail.com (mailing list archive)
State New, archived
Headers show
Series drm/panel: Add NewVision NV3051D Panels | expand

Commit Message

Chris Morgan Sept. 6, 2022, 6:52 p.m. UTC 
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

Comments

Krzysztof Kozlowski Sept. 7, 2022, 12:53 p.m. UTC | #1 
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: http://devicetree.org/schemas/display/panel/newvision,nv3051d.yaml#

You need to document vendor prefix... but the filename does not match
compatible.

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NewVision NV3051D based DSI panel driver

This is confusing - compatibles say something else.

> +
> +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.

> +  reg: true
> +  backlight: true


Best regards,
Krzysztof
Chris Morgan Sept. 7, 2022, 1:35 p.m. UTC | #2 
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.

> 
> > +$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...

> > +
> > +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.

> > +  reg: true
> > +  backlight: true
> 
> 
> Best regards,
> Krzysztof

Thank you once again for all your help, sorry to be such a bother
in this. I'm honestly not sure the best way to name and document a
panel when I only know one of the part numbers (the IC).
Krzysztof Kozlowski Sept. 8, 2022, 3:18 p.m. UTC | #3 
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
Rob Herring (Arm) Sept. 9, 2022, 1:42 a.m. UTC | #4 
On Wed, Sep 07, 2022 at 08:35:13AM -0500, 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.

I think most cases like this targeting a specific LCD driver IC, there's 
a driver IC compatible as a fallback. 

(TBC, 'driver' here is not Linux driver, but the h/w chip.)

Rob
Chris Morgan Sept. 12, 2022, 9:40 p.m. UTC | #5 
On Thu, Sep 08, 2022 at 08:42:35PM -0500, Rob Herring wrote:
> On Wed, Sep 07, 2022 at 08:35:13AM -0500, 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%7Cab7f68ce677846b1638508da920493a4%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637982845610791935%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=6XxrzD1zkl8SYQqV9nkH1WzKfdcy6doNfzan8r228W0%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.
> 
> I think most cases like this targeting a specific LCD driver IC, there's 
> a driver IC compatible as a fallback. 
> 
> (TBC, 'driver' here is not Linux driver, but the h/w chip.)
> 
> Rob

So in this case would my compatible string in the devicetree need to
be something like "anbernic,rg353-panel", "newvision,nv3051d" then?
And the module itself have the compatible string of "newvision,nv3051d"?

My fear is that I write this as a newvision,nv3051d kernel module
and then later on there comes a new panel using the nv3051d that wants
to use this too. Again keeping things confusing I have no LCD panel
part number from the manufacturer for this, and I cannot see anything
on the panel itself denoting what LCD it is.

Thank you for your help. Just wrapping up getting some of my favorite
devices supported in mainline. :-)
diff mbox series

Patch

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: http://devicetree.org/schemas/display/panel/newvision,nv3051d.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NewVision NV3051D based DSI panel driver
+
+maintainers:
+  - Chris Morgan <macromorgan@hotmail.com>
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    enum:
+      - anbernic,rg353p-panel
+      - anbernic,rg353v-panel
+  reg: true
+  backlight: true
+  port: true
+  reset-gpios: true
+  vdd-supply:
+    description: regulator that supplies the vdd voltage
+
+required:
+  - compatible
+  - reg
+  - backlight
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    dsi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        panel@0 {
+            compatible = "anbernic,rg353p-panel";
+            reg = <0>;
+            backlight = <&backlight>;
+            vdd-supply = <&vcc3v3_lcd>;
+        };
+    };
+
+...