| Message ID | 20220920145905.20595-3-macroalpha82@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panel: Add NewVision NV3051D Panels | expand |
On 20/09/2022 16:59, 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. Name "anbernic,rg353p-panel" > is used because the hardware itself is known as "anbernic,rg353p". > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com> > --- > .../display/panel/newvision,nv3051d.yaml | 55 +++++++++++++++++++ > 1 file changed, 55 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..d90bca4171c2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml > @@ -0,0 +1,55 @@ > +# 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 LCD panel > + > +description: | > + The NewVision NV3051D is a driver chip used to drive DSI panels. For now, > + this driver only supports the 640x480 panels found in the Anbernic RG353 > + based devices. > + > +maintainers: > + - Chris Morgan <macromorgan@hotmail.com> > + > +allOf: > + - $ref: panel-common.yaml# > + > +properties: > + compatible: > + items: > + - enum: > + - anbernic,rg353p-panel Are these vendor prefixs documented? > + - anbernic,rg353v-panel > + - const: newvision,nv3051d Blank line. > + reg: true > + backlight: true > + port: true > + reset-gpios: true > + vdd-supply: > + description: regulator that supplies the vdd voltage Skip description and make it just "true". It's kind of obvious. > + > +required: > + - compatible > + - reg > + - backlight > + - vdd-supply Best regards, Krzysztof
On Tue, 20 Sep 2022 09:59:04 -0500, 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. Name "anbernic,rg353p-panel" > is used because the hardware itself is known as "anbernic,rg353p". > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com> > --- > .../display/panel/newvision,nv3051d.yaml | 55 +++++++++++++++++++ > 1 file changed, 55 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.example.dtb: panel@0: compatible: ['anbernic,rg353p-panel'] is too short From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
On Wed, Sep 21, 2022 at 08:51:34AM +0200, Krzysztof Kozlowski wrote: > On 20/09/2022 16:59, 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. Name "anbernic,rg353p-panel" > > is used because the hardware itself is known as "anbernic,rg353p". > > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com> > > --- > > .../display/panel/newvision,nv3051d.yaml | 55 +++++++++++++++++++ > > 1 file changed, 55 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..d90bca4171c2 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml > > @@ -0,0 +1,55 @@ > > +# 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&data=05%7C01%7C%7C844872fdf91b413aa65a08da9b9db9e7%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637993398994635658%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=eTM2IFjR0TKPlQNYfoq3Poao8QYLSHRVaqiXtufJ7VA%3D&reserved=0 > > +$schema: https://nam12.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7C%7C844872fdf91b413aa65a08da9b9db9e7%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637993398994635658%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=sjb7x2Z2wKu1C8mMBW1epwuXipe8V26zxpHcCAuKLZY%3D&reserved=0 > > + > > +title: NewVision NV3051D based LCD panel > > + > > +description: | > > + The NewVision NV3051D is a driver chip used to drive DSI panels. For now, > > + this driver only supports the 640x480 panels found in the Anbernic RG353 > > + based devices. > > + > > +maintainers: > > + - Chris Morgan <macromorgan@hotmail.com> > > + > > +allOf: > > + - $ref: panel-common.yaml# > > + > > +properties: > > + compatible: > > + items: > > + - enum: > > + - anbernic,rg353p-panel > > Are these vendor prefixs documented? Yes, they are in another patch series referenced in the cover letter. They were added for the Anbernic devicetrees and should (I believe) land in 6.1. > > > + - anbernic,rg353v-panel > > + - const: newvision,nv3051d > > Blank line. > Ack. > > + reg: true > > + backlight: true > > + port: true > > + reset-gpios: true > > + vdd-supply: > > + description: regulator that supplies the vdd voltage > > Skip description and make it just "true". It's kind of obvious. > Ack. > > + > > +required: > > + - compatible > > + - reg > > + - backlight > > + - vdd-supply > > Best regards, > Krzysztof
On 21/09/2022 16:38, Chris Morgan wrote: >>> + compatible: >>> + items: >>> + - enum: >>> + - anbernic,rg353p-panel >> >> Are these vendor prefixs documented? > > Yes, they are in another patch series referenced in the cover letter. > They were added for the Anbernic devicetrees and should (I believe) > land in 6.1. OK... you still need to test your bindings. Your patch was clearly not tested before sending. :( Best regards, Krzysztof
On Wed, Sep 21, 2022 at 05:21:19PM +0200, Krzysztof Kozlowski wrote: > On 21/09/2022 16:38, Chris Morgan wrote: > >>> + compatible: > >>> + items: > >>> + - enum: > >>> + - anbernic,rg353p-panel > >> > >> Are these vendor prefixs documented? > > > > Yes, they are in another patch series referenced in the cover letter. > > They were added for the Anbernic devicetrees and should (I believe) > > land in 6.1. > > OK... you still need to test your bindings. Your patch was clearly not > tested before sending. :( I did: yamllint, make dt_binding_check (with DT_SCHEMA_FILES specified), and make dtbs_check (with DT_SCHEMA_FILES specified again). That's the proper testing flow correct? In this case it's the pre-requisite that's causing the issue as I see on a pristine master tree I'm warned about the missing vendor prefix for anbernic. Should I wait for that to go upstream before I submit this again? I'll make the other change about the space and the description of the vdd-supply when I resubmit. Are we good with the panel compatibles? I'm still not entirely sure the best thing to name them as I have no part number whatsoever except the driver IC. Thank you. > > Best regards, > Krzysztof >
On 21/09/2022 17:50, Chris Morgan wrote: > On Wed, Sep 21, 2022 at 05:21:19PM +0200, Krzysztof Kozlowski wrote: >> On 21/09/2022 16:38, Chris Morgan wrote: >>>>> + compatible: >>>>> + items: >>>>> + - enum: >>>>> + - anbernic,rg353p-panel >>>> >>>> Are these vendor prefixs documented? >>> >>> Yes, they are in another patch series referenced in the cover letter. >>> They were added for the Anbernic devicetrees and should (I believe) >>> land in 6.1. >> >> OK... you still need to test your bindings. Your patch was clearly not >> tested before sending. :( > > I did: yamllint, make dt_binding_check (with DT_SCHEMA_FILES specified), and > make dtbs_check (with DT_SCHEMA_FILES specified again). I have doubts. So if you say you did it, then you probably did not look at the results... or whatever other reason the test was not effective, because your binding cannot pass the dt_binding_check. > That's the proper > testing flow correct? In this case it's the pre-requisite that's causing > the issue as I see on a pristine master tree I'm warned about the missing > vendor prefix for anbernic. Should I wait for that to go upstream before > I submit this again? Not really. The testing fails on wrong compatible in example. Best regards, Krzysztof
On Wed, Sep 21, 2022 at 05:57:55PM +0200, Krzysztof Kozlowski wrote: > On 21/09/2022 17:50, Chris Morgan wrote: > > On Wed, Sep 21, 2022 at 05:21:19PM +0200, Krzysztof Kozlowski wrote: > >> On 21/09/2022 16:38, Chris Morgan wrote: > >>>>> + compatible: > >>>>> + items: > >>>>> + - enum: > >>>>> + - anbernic,rg353p-panel > >>>> > >>>> Are these vendor prefixs documented? > >>> > >>> Yes, they are in another patch series referenced in the cover letter. > >>> They were added for the Anbernic devicetrees and should (I believe) > >>> land in 6.1. > >> > >> OK... you still need to test your bindings. Your patch was clearly not > >> tested before sending. :( > > > > I did: yamllint, make dt_binding_check (with DT_SCHEMA_FILES specified), and > > make dtbs_check (with DT_SCHEMA_FILES specified again). > > I have doubts. So if you say you did it, then you probably did not look > at the results... or whatever other reason the test was not effective, > because your binding cannot pass the dt_binding_check. > > > That's the proper > > testing flow correct? In this case it's the pre-requisite that's causing > > the issue as I see on a pristine master tree I'm warned about the missing > > vendor prefix for anbernic. Should I wait for that to go upstream before > > I submit this again? > > Not really. The testing fails on wrong compatible in example. My mistake, I see what I did wrong and apologize for the trouble. I misinterpreted the error I did get (I expected an issue with a missing vendor string, but as you correctly point out the received error is not for that). I'll correct and resend. Would you be so kind as to confirm if you're okay with the "anbernic,rg353-panel", "newvision,nv3051d" strings? Thank you once again and I apologize for my mistake. > > Best regards, > Krzysztof >
On Tue, Sep 20, 2022 at 09:59:04AM -0500, 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. Name "anbernic,rg353p-panel" > is used because the hardware itself is known as "anbernic,rg353p". > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com> > --- > .../display/panel/newvision,nv3051d.yaml | 55 +++++++++++++++++++ > 1 file changed, 55 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..d90bca4171c2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml > @@ -0,0 +1,55 @@ > +# 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 LCD panel > + > +description: | > + The NewVision NV3051D is a driver chip used to drive DSI panels. For now, > + this driver only supports the 640x480 panels found in the Anbernic RG353 > + based devices. > + > +maintainers: > + - Chris Morgan <macromorgan@hotmail.com> > + > +allOf: > + - $ref: panel-common.yaml# > + > +properties: > + compatible: > + items: > + - enum: > + - anbernic,rg353p-panel > + - anbernic,rg353v-panel Is 'panel' redundant? IOW, could 'rg353v' identify something else other than the panel? Rob
On Sat, Sep 24, 2022 at 12:07:44PM -0500, Rob Herring wrote: > On Tue, Sep 20, 2022 at 09:59:04AM -0500, 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. Name "anbernic,rg353p-panel" > > is used because the hardware itself is known as "anbernic,rg353p". > > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com> > > --- > > .../display/panel/newvision,nv3051d.yaml | 55 +++++++++++++++++++ > > 1 file changed, 55 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..d90bca4171c2 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml > > @@ -0,0 +1,55 @@ > > +# 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&data=05%7C01%7C%7C4f204345128d4cb827ca08da9e4f4d06%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637996360672620588%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=9%2B66S0t1p9EqWBdmaLBj8pKte2fjzsmL%2FSbmmD8eNi0%3D&reserved=0 > > +$schema: https://nam12.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7C%7C4f204345128d4cb827ca08da9e4f4d06%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637996360672620588%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=r%2BzTHlte226t9fXktNC9k4NO%2FE2RomRIxuWBuRshIw0%3D&reserved=0 > > + > > +title: NewVision NV3051D based LCD panel > > + > > +description: | > > + The NewVision NV3051D is a driver chip used to drive DSI panels. For now, > > + this driver only supports the 640x480 panels found in the Anbernic RG353 > > + based devices. > > + > > +maintainers: > > + - Chris Morgan <macromorgan@hotmail.com> > > + > > +allOf: > > + - $ref: panel-common.yaml# > > + > > +properties: > > + compatible: > > + items: > > + - enum: > > + - anbernic,rg353p-panel > > + - anbernic,rg353v-panel > > Is 'panel' redundant? IOW, could 'rg353v' identify something else other > than the panel? It is not redundant, the device itself is identified as "anbernic,rg353v". I don't have a part number for the LCD panel itself, only the controller IC. Thank you. > > Rob
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..d90bca4171c2 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/newvision,nv3051d.yaml @@ -0,0 +1,55 @@ +# 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 LCD panel + +description: | + The NewVision NV3051D is a driver chip used to drive DSI panels. For now, + this driver only supports the 640x480 panels found in the Anbernic RG353 + based devices. + +maintainers: + - Chris Morgan <macromorgan@hotmail.com> + +allOf: + - $ref: panel-common.yaml# + +properties: + compatible: + items: + - enum: + - anbernic,rg353p-panel + - anbernic,rg353v-panel + - const: newvision,nv3051d + 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>; + }; + }; + +...