[PATCH 0/3] Correct WL-355608-A8 panel compatible

[PATCH 0/3] Correct WL-355608-A8 panel compatible

[Ryan Walklin]

The previous patch adding support for this panel [1] referred to previously by its serial number only. As discussed after the patch was committed, the preference is to use the integrating device vendor and name in this circumstance.

This series corrects the panel compatible to reflect the vendor (Anbernic, already in the vendor prefix table), updates the NV3052C panel driver with the new compatible, and lastly adds num-chipselects and sck-gpios to the DT binding example, identified by make dt_bindings_check as required for bit-banged SPI over GPIO lines.

Regards,

Ryan

[1] https://lore.kernel.org/dri-devel/20240530211415.44201-1-ryan@testtoast.com/

Ryan Walklin (3):
  dt-bindings: display: panel: Rename WL-355608-A8 panel
  drm: panel: nv3052c: Correct WL-355608-A8 panel compatible
  dt-bindings: display: panel: correct Anbernic RG35XX panel example

 .../{wl-355608-a8.yaml => anbernic,rg35xx-panel.yaml} | 11 +++++++----
 drivers/gpu/drm/panel/panel-newvision-nv3052c.c       |  2 +-
 2 files changed, 8 insertions(+), 5 deletions(-)
 rename Documentation/devicetree/bindings/display/panel/{wl-355608-a8.yaml => anbernic,rg35xx-panel.yaml} (76%)

-- 
2.45.2

[PATCH 1/3] dt-bindings: display: panel: Rename WL-355608-A8 panel

[Ryan Walklin]

The WL-355608-A8 is a 3.5" 640x480@60Hz RGB LCD display from an unknown
OEM used in a number of handheld gaming devices made by Anbernic.
Previously committed using the OEM serial without a vendor prefix,
however the preference is to use the integrating device vendor and name
where the OEM is unknown.

Alter the filename and compatible string to reflect the convention.

Fixes: f08aac40639c ("drm: panel: nv3052c: Add WL-355608-A8 panel")
Signed-off-by: Ryan Walklin <ryan@testtoast.com>
---
 .../{wl-355608-a8.yaml => anbernic,rg35xx-panel.yaml}     | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
 rename Documentation/devicetree/bindings/display/panel/{wl-355608-a8.yaml => anbernic,rg35xx-panel.yaml} (81%)

diff --git a/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
similarity index 81%
rename from Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml
rename to Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
index 397c26be9bda5..a7d5ad0f29389 100644
--- a/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml
+++ b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
@@ -1,10 +1,10 @@
 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/display/panel/wl-355608-a8.yaml#
+$id: http://devicetree.org/schemas/display/panel/anbernic,rg35xx-panel.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: WL-355608-A8 3.5" (640x480 pixels) 24-bit IPS LCD panel
+title: Anbernic RG35XX (WL-355608-A8) 3.5" 640x480 24-bit IPS LCD panel
 
 maintainers:
   - Ryan Walklin <ryan@testtoast.com>
@@ -15,7 +15,7 @@ allOf:
 
 properties:
   compatible:
-    const: wl-355608-a8
+    const: anbernic,rg35xx-panel
 
   reg:
     maxItems: 1
@@ -41,7 +41,7 @@ examples:
         #size-cells = <0>;
 
         panel@0 {
-            compatible = "wl-355608-a8";
+            compatible = "anbernic,rg35xx-panel";
             reg = <0>;
 
             spi-3wire;
-- 
2.45.2

Re: [PATCH 1/3] dt-bindings: display: panel: Rename WL-355608-A8 panel

[Neil Armstrong]

On 26/06/2024 13:17, Ryan Walklin wrote:
> The WL-355608-A8 is a 3.5" 640x480@60Hz RGB LCD display from an unknown
> OEM used in a number of handheld gaming devices made by Anbernic.
> Previously committed using the OEM serial without a vendor prefix,
> however the preference is to use the integrating device vendor and name
> where the OEM is unknown.
> 
> Alter the filename and compatible string to reflect the convention.
> 
> Fixes: f08aac40639c ("drm: panel: nv3052c: Add WL-355608-A8 panel")
> Signed-off-by: Ryan Walklin <ryan@testtoast.com>
> ---
>   .../{wl-355608-a8.yaml => anbernic,rg35xx-panel.yaml}     | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>   rename Documentation/devicetree/bindings/display/panel/{wl-355608-a8.yaml => anbernic,rg35xx-panel.yaml} (81%)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
> similarity index 81%
> rename from Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml
> rename to Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
> index 397c26be9bda5..a7d5ad0f29389 100644
> --- a/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
> @@ -1,10 +1,10 @@
>   # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>   %YAML 1.2
>   ---
> -$id: http://devicetree.org/schemas/display/panel/wl-355608-a8.yaml#
> +$id: http://devicetree.org/schemas/display/panel/anbernic,rg35xx-panel.yaml#
>   $schema: http://devicetree.org/meta-schemas/core.yaml#
>   
> -title: WL-355608-A8 3.5" (640x480 pixels) 24-bit IPS LCD panel
> +title: Anbernic RG35XX (WL-355608-A8) 3.5" 640x480 24-bit IPS LCD panel
>   
>   maintainers:
>     - Ryan Walklin <ryan@testtoast.com>
> @@ -15,7 +15,7 @@ allOf:
>   
>   properties:
>     compatible:
> -    const: wl-355608-a8
> +    const: anbernic,rg35xx-panel
>   
>     reg:
>       maxItems: 1
> @@ -41,7 +41,7 @@ examples:
>           #size-cells = <0>;
>   
>           panel@0 {
> -            compatible = "wl-355608-a8";
> +            compatible = "anbernic,rg35xx-panel";

Can it be more specific ? because there's a lot of rg35xx defined in bindings:
  anbernic,rg351m
  anbernic,rg351v
  anbernic,rg353p
  anbernic,rg353ps
  anbernic,rg353v
  anbernic,rg353vs
  anbernic,rg35xx-2024
  anbernic,rg35xx-plus
  anbernic,rg35xx-h

Neil

>               reg = <0>;
>   
>               spi-3wire;

Re: [PATCH 1/3] dt-bindings: display: panel: Rename WL-355608-A8 panel

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2859 bytes --]

On Wed, Jun 26, 2024 at 02:25:54PM GMT, Neil Armstrong wrote:
> On 26/06/2024 13:17, Ryan Walklin wrote:
> > The WL-355608-A8 is a 3.5" 640x480@60Hz RGB LCD display from an unknown
> > OEM used in a number of handheld gaming devices made by Anbernic.
> > Previously committed using the OEM serial without a vendor prefix,
> > however the preference is to use the integrating device vendor and name
> > where the OEM is unknown.
> > 
> > Alter the filename and compatible string to reflect the convention.
> > 
> > Fixes: f08aac40639c ("drm: panel: nv3052c: Add WL-355608-A8 panel")
> > Signed-off-by: Ryan Walklin <ryan@testtoast.com>
> > ---
> >   .../{wl-355608-a8.yaml => anbernic,rg35xx-panel.yaml}     | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >   rename Documentation/devicetree/bindings/display/panel/{wl-355608-a8.yaml => anbernic,rg35xx-panel.yaml} (81%)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
> > similarity index 81%
> > rename from Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml
> > rename to Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
> > index 397c26be9bda5..a7d5ad0f29389 100644
> > --- a/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml
> > +++ b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
> > @@ -1,10 +1,10 @@
> >   # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >   %YAML 1.2
> >   ---
> > -$id: http://devicetree.org/schemas/display/panel/wl-355608-a8.yaml#
> > +$id: http://devicetree.org/schemas/display/panel/anbernic,rg35xx-panel.yaml#
> >   $schema: http://devicetree.org/meta-schemas/core.yaml#
> > -title: WL-355608-A8 3.5" (640x480 pixels) 24-bit IPS LCD panel
> > +title: Anbernic RG35XX (WL-355608-A8) 3.5" 640x480 24-bit IPS LCD panel
> >   maintainers:
> >     - Ryan Walklin <ryan@testtoast.com>
> > @@ -15,7 +15,7 @@ allOf:
> >   properties:
> >     compatible:
> > -    const: wl-355608-a8
> > +    const: anbernic,rg35xx-panel
> >     reg:
> >       maxItems: 1
> > @@ -41,7 +41,7 @@ examples:
> >           #size-cells = <0>;
> >           panel@0 {
> > -            compatible = "wl-355608-a8";
> > +            compatible = "anbernic,rg35xx-panel";
> 
> Can it be more specific ? because there's a lot of rg35xx defined in bindings:
>  anbernic,rg351m
>  anbernic,rg351v
>  anbernic,rg353p
>  anbernic,rg353ps
>  anbernic,rg353v
>  anbernic,rg353vs
>  anbernic,rg35xx-2024
>  anbernic,rg35xx-plus
>  anbernic,rg35xx-h

Yeah, if we have an identified model name, we should probably use that,
with a comment that we couldn't figure out what the vendor was and thus
went for anbernic.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] dt-bindings: display: panel: Rename WL-355608-A8 panel

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3247 bytes --]

On Wed, Jun 26, 2024 at 05:04:19PM +0200, Maxime Ripard wrote:
> On Wed, Jun 26, 2024 at 02:25:54PM GMT, Neil Armstrong wrote:
> > On 26/06/2024 13:17, Ryan Walklin wrote:
> > > The WL-355608-A8 is a 3.5" 640x480@60Hz RGB LCD display from an unknown
> > > OEM used in a number of handheld gaming devices made by Anbernic.
> > > Previously committed using the OEM serial without a vendor prefix,
> > > however the preference is to use the integrating device vendor and name
> > > where the OEM is unknown.
> > > 
> > > Alter the filename and compatible string to reflect the convention.
> > > 
> > > Fixes: f08aac40639c ("drm: panel: nv3052c: Add WL-355608-A8 panel")
> > > Signed-off-by: Ryan Walklin <ryan@testtoast.com>
> > > ---
> > >   .../{wl-355608-a8.yaml => anbernic,rg35xx-panel.yaml}     | 8 ++++----
> > >   1 file changed, 4 insertions(+), 4 deletions(-)
> > >   rename Documentation/devicetree/bindings/display/panel/{wl-355608-a8.yaml => anbernic,rg35xx-panel.yaml} (81%)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
> > > similarity index 81%
> > > rename from Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml
> > > rename to Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
> > > index 397c26be9bda5..a7d5ad0f29389 100644
> > > --- a/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml
> > > +++ b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
> > > @@ -1,10 +1,10 @@
> > >   # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > >   %YAML 1.2
> > >   ---
> > > -$id: http://devicetree.org/schemas/display/panel/wl-355608-a8.yaml#
> > > +$id: http://devicetree.org/schemas/display/panel/anbernic,rg35xx-panel.yaml#
> > >   $schema: http://devicetree.org/meta-schemas/core.yaml#
> > > -title: WL-355608-A8 3.5" (640x480 pixels) 24-bit IPS LCD panel
> > > +title: Anbernic RG35XX (WL-355608-A8) 3.5" 640x480 24-bit IPS LCD panel
> > >   maintainers:
> > >     - Ryan Walklin <ryan@testtoast.com>
> > > @@ -15,7 +15,7 @@ allOf:
> > >   properties:
> > >     compatible:
> > > -    const: wl-355608-a8
> > > +    const: anbernic,rg35xx-panel
> > >     reg:
> > >       maxItems: 1
> > > @@ -41,7 +41,7 @@ examples:
> > >           #size-cells = <0>;
> > >           panel@0 {
> > > -            compatible = "wl-355608-a8";
> > > +            compatible = "anbernic,rg35xx-panel";
> > 
> > Can it be more specific ? because there's a lot of rg35xx defined in bindings:
> >  anbernic,rg351m
> >  anbernic,rg351v
> >  anbernic,rg353p
> >  anbernic,rg353ps
> >  anbernic,rg353v
> >  anbernic,rg353vs
> >  anbernic,rg35xx-2024
> >  anbernic,rg35xx-plus
> >  anbernic,rg35xx-h
> 
> Yeah, if we have an identified model name, we should probably use that,
> with a comment that we couldn't figure out what the vendor was and thus
> went for anbernic.

What's wrong with using the wl name that already exists as the model?
Using rg<whatever>-panel is total invention on our part, especially
seeing as the commit message says that multiple models can use it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] dt-bindings: display: panel: Rename WL-355608-A8 panel

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3499 bytes --]

On Wed, Jun 26, 2024 at 04:34:11PM GMT, Conor Dooley wrote:
> On Wed, Jun 26, 2024 at 05:04:19PM +0200, Maxime Ripard wrote:
> > On Wed, Jun 26, 2024 at 02:25:54PM GMT, Neil Armstrong wrote:
> > > On 26/06/2024 13:17, Ryan Walklin wrote:
> > > > The WL-355608-A8 is a 3.5" 640x480@60Hz RGB LCD display from an unknown
> > > > OEM used in a number of handheld gaming devices made by Anbernic.
> > > > Previously committed using the OEM serial without a vendor prefix,
> > > > however the preference is to use the integrating device vendor and name
> > > > where the OEM is unknown.
> > > > 
> > > > Alter the filename and compatible string to reflect the convention.
> > > > 
> > > > Fixes: f08aac40639c ("drm: panel: nv3052c: Add WL-355608-A8 panel")
> > > > Signed-off-by: Ryan Walklin <ryan@testtoast.com>
> > > > ---
> > > >   .../{wl-355608-a8.yaml => anbernic,rg35xx-panel.yaml}     | 8 ++++----
> > > >   1 file changed, 4 insertions(+), 4 deletions(-)
> > > >   rename Documentation/devicetree/bindings/display/panel/{wl-355608-a8.yaml => anbernic,rg35xx-panel.yaml} (81%)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
> > > > similarity index 81%
> > > > rename from Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml
> > > > rename to Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
> > > > index 397c26be9bda5..a7d5ad0f29389 100644
> > > > --- a/Documentation/devicetree/bindings/display/panel/wl-355608-a8.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
> > > > @@ -1,10 +1,10 @@
> > > >   # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > >   %YAML 1.2
> > > >   ---
> > > > -$id: http://devicetree.org/schemas/display/panel/wl-355608-a8.yaml#
> > > > +$id: http://devicetree.org/schemas/display/panel/anbernic,rg35xx-panel.yaml#
> > > >   $schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > -title: WL-355608-A8 3.5" (640x480 pixels) 24-bit IPS LCD panel
> > > > +title: Anbernic RG35XX (WL-355608-A8) 3.5" 640x480 24-bit IPS LCD panel
> > > >   maintainers:
> > > >     - Ryan Walklin <ryan@testtoast.com>
> > > > @@ -15,7 +15,7 @@ allOf:
> > > >   properties:
> > > >     compatible:
> > > > -    const: wl-355608-a8
> > > > +    const: anbernic,rg35xx-panel
> > > >     reg:
> > > >       maxItems: 1
> > > > @@ -41,7 +41,7 @@ examples:
> > > >           #size-cells = <0>;
> > > >           panel@0 {
> > > > -            compatible = "wl-355608-a8";
> > > > +            compatible = "anbernic,rg35xx-panel";
> > > 
> > > Can it be more specific ? because there's a lot of rg35xx defined in bindings:
> > >  anbernic,rg351m
> > >  anbernic,rg351v
> > >  anbernic,rg353p
> > >  anbernic,rg353ps
> > >  anbernic,rg353v
> > >  anbernic,rg353vs
> > >  anbernic,rg35xx-2024
> > >  anbernic,rg35xx-plus
> > >  anbernic,rg35xx-h
> > 
> > Yeah, if we have an identified model name, we should probably use that,
> > with a comment that we couldn't figure out what the vendor was and thus
> > went for anbernic.
> 
> What's wrong with using the wl name that already exists as the model?
> Using rg<whatever>-panel is total invention on our part, especially
> seeing as the commit message says that multiple models can use it.

Yeah, that makes sense, sorry for the noise

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] dt-bindings: display: panel: Rename WL-355608-A8 panel

[Ryan Walklin]

On Fri, 28 Jun 2024, at 5:59 AM, Maxime Ripard wrote:
> On Wed, Jun 26, 2024 at 04:34:11PM GMT, Conor Dooley wrote:
>> On Wed, Jun 26, 2024 at 05:04:19PM +0200, Maxime Ripard wrote:
>> > On Wed, Jun 26, 2024 at 02:25:54PM GMT, Neil Armstrong wrote:
>> > > Can it be more specific ? because there's a lot of rg35xx defined in bindings:
>> > >  anbernic,rg351m
>> > >  anbernic,rg351v
>> > >  anbernic,rg353p
>> > >  anbernic,rg353ps
>> > >  anbernic,rg353v
>> > >  anbernic,rg353vs
>> > >  anbernic,rg35xx-2024
>> > >  anbernic,rg35xx-plus
>> > >  anbernic,rg35xx-h

Just to note only the three rg35xx-* devices use this particular panel.

>> > 
>> > Yeah, if we have an identified model name, we should probably use that,
>> > with a comment that we couldn't figure out what the vendor was and thus
>> > went for anbernic.
>> 
>> What's wrong with using the wl name that already exists as the model?
>> Using rg<whatever>-panel is total invention on our part, especially
>> seeing as the commit message says that multiple models can use it.
>
> Yeah, that makes sense, sorry for the noise
>
Thanks both for the further feedback, agreed logical to use the device vendor and panel serial number, ie "anbernic,wl-355608-a8". Will post a V2 with a comment to that effect.

Regards,

Ryan

Re: [PATCH 1/3] dt-bindings: display: panel: Rename WL-355608-A8 panel

[Neil Armstrong]

On 30/06/2024 10:16, Ryan Walklin wrote:
> On Fri, 28 Jun 2024, at 5:59 AM, Maxime Ripard wrote:
>> On Wed, Jun 26, 2024 at 04:34:11PM GMT, Conor Dooley wrote:
>>> On Wed, Jun 26, 2024 at 05:04:19PM +0200, Maxime Ripard wrote:
>>>> On Wed, Jun 26, 2024 at 02:25:54PM GMT, Neil Armstrong wrote:
>>>>> Can it be more specific ? because there's a lot of rg35xx defined in bindings:
>>>>>   anbernic,rg351m
>>>>>   anbernic,rg351v
>>>>>   anbernic,rg353p
>>>>>   anbernic,rg353ps
>>>>>   anbernic,rg353v
>>>>>   anbernic,rg353vs
>>>>>   anbernic,rg35xx-2024
>>>>>   anbernic,rg35xx-plus
>>>>>   anbernic,rg35xx-h
> 
> Just to note only the three rg35xx-* devices use this particular panel.
> 
>>>>
>>>> Yeah, if we have an identified model name, we should probably use that,
>>>> with a comment that we couldn't figure out what the vendor was and thus
>>>> went for anbernic.
>>>
>>> What's wrong with using the wl name that already exists as the model?
>>> Using rg<whatever>-panel is total invention on our part, especially
>>> seeing as the commit message says that multiple models can use it.
>>
>> Yeah, that makes sense, sorry for the noise
>>
> Thanks both for the further feedback, agreed logical to use the device vendor and panel serial number, ie "anbernic,wl-355608-a8". Will post a V2 with a comment to that effect.

Well in this case we can keep "wl-355608-a8", because the panel vendor _is not_ anbernic.

Neil

> 
> Regards,
> 
> Ryan

Re: [PATCH 1/3] dt-bindings: display: panel: Rename WL-355608-A8 panel

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1787 bytes --]

On Tue, Aug 27, 2024 at 06:28:21PM GMT, Neil Armstrong wrote:
> On 30/06/2024 10:16, Ryan Walklin wrote:
> > On Fri, 28 Jun 2024, at 5:59 AM, Maxime Ripard wrote:
> > > On Wed, Jun 26, 2024 at 04:34:11PM GMT, Conor Dooley wrote:
> > > > On Wed, Jun 26, 2024 at 05:04:19PM +0200, Maxime Ripard wrote:
> > > > > On Wed, Jun 26, 2024 at 02:25:54PM GMT, Neil Armstrong wrote:
> > > > > > Can it be more specific ? because there's a lot of rg35xx defined in bindings:
> > > > > >   anbernic,rg351m
> > > > > >   anbernic,rg351v
> > > > > >   anbernic,rg353p
> > > > > >   anbernic,rg353ps
> > > > > >   anbernic,rg353v
> > > > > >   anbernic,rg353vs
> > > > > >   anbernic,rg35xx-2024
> > > > > >   anbernic,rg35xx-plus
> > > > > >   anbernic,rg35xx-h
> > 
> > Just to note only the three rg35xx-* devices use this particular panel.
> > 
> > > > > 
> > > > > Yeah, if we have an identified model name, we should probably use that,
> > > > > with a comment that we couldn't figure out what the vendor was and thus
> > > > > went for anbernic.
> > > > 
> > > > What's wrong with using the wl name that already exists as the model?
> > > > Using rg<whatever>-panel is total invention on our part, especially
> > > > seeing as the commit message says that multiple models can use it.
> > > 
> > > Yeah, that makes sense, sorry for the noise
> > > 
> > Thanks both for the further feedback, agreed logical to use the
> > device vendor and panel serial number, ie "anbernic,wl-355608-a8".
> > Will post a V2 with a comment to that effect.
> 
> Well in this case we can keep "wl-355608-a8", because the panel vendor
> _is not_ anbernic.

And it's not a generic or ubiquitous device either. We've been over
this already, anbernic is the best we have.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH 1/3] dt-bindings: display: panel: Rename WL-355608-A8 panel

[Ryan Walklin]

> On 28 Aug 2024, at 7:07 PM, Maxime Ripard <mripard@kernel.org> wrote:
> 
> On Tue, Aug 27, 2024 at 06:28:21PM GMT, Neil Armstrong wrote:
>>> Thanks both for the further feedback, agreed logical to use the
>>> device vendor and panel serial number, ie "anbernic,wl-355608-a8".
>>> Will post a V2 with a comment to that effect.
>> 
>> Well in this case we can keep "wl-355608-a8", because the panel vendor
>> _is not_ anbernic.
> 
> And it's not a generic or ubiquitous device either. We've been over
> this already, anbernic is the best we have.

I don’t have a strong preference either way but agree the anbernic vendor string is the best compromise.

Regards,

Ryan

Re: [PATCH 1/3] dt-bindings: display: panel: Rename WL-355608-A8 panel

[Neil Armstrong]

On 30/08/2024 03:43, Ryan Walklin wrote:
> 
> 
>> On 28 Aug 2024, at 7:07 PM, Maxime Ripard <mripard@kernel.org> wrote:
>>
>> On Tue, Aug 27, 2024 at 06:28:21PM GMT, Neil Armstrong wrote:
>>>> Thanks both for the further feedback, agreed logical to use the
>>>> device vendor and panel serial number, ie "anbernic,wl-355608-a8".
>>>> Will post a V2 with a comment to that effect.
>>>
>>> Well in this case we can keep "wl-355608-a8", because the panel vendor
>>> _is not_ anbernic.
>>
>> And it's not a generic or ubiquitous device either. We've been over
>> this already, anbernic is the best we have.
> 
> I don’t have a strong preference either way but agree the anbernic vendor string is the best compromise.

The only valid compatible with anbernic would be to use the exact device in use and not a wildcard,
so you said the 3 devices using this panel are:
  anbernic,rg35xx-2024
  anbernic,rg35xx-plus
  anbernic,rg35xx-h
you should introduce 3 compatibles:
  anbernic,rg35xx-2024-panel
  anbernic,rg35xx-plus-panel
  anbernic,rg35xx-h-panel

but it's duplicating for nothing, to you should use fallbacks for 2 of them to have in DT :

  anbernic,rg35xx-2024-panel
  anbernic,rg35xx-plus-panel, anbernic,rg35xx-2024-panel
  anbernic,rg35xx-h-panel, anbernic,rg35xx-2024-panel

and only use anbernic,rg35xx-2024-panel in the driver.

In this case bindings should be like:
   properties:
     compatible:
       oneOf:
         - const: anbernic,rg35xx-2024-panel
         - items:
             - enum:
                 - anbernic,rg35xx-plus-panel
                 - anbernic,rg35xx-h-panel
             - const: anbernic,rg35xx-2024-panel

(of course I selected rg35xx-2024 as the primary one, it could be another, usually the older one)

Neil

> 
> Regards,
> 
> Ryan

Re: [PATCH 1/3] dt-bindings: display: panel: Rename WL-355608-A8 panel

[Ryan Walklin]

On Fri, 30 Aug 2024, at 7:56 PM, Neil Armstrong wrote:

Thanks Neil,

>
> The only valid compatible with anbernic would be to use the exact 
> device in use and not a wildcard,

> so you said the 3 devices using this panel are:
>   anbernic,rg35xx-2024
>   anbernic,rg35xx-plus
>   anbernic,rg35xx-h
> you should introduce 3 compatibles:
>   anbernic,rg35xx-2024-panel
>   anbernic,rg35xx-plus-panel
>   anbernic,rg35xx-h-panel
>
> but it's duplicating for nothing, to you should use fallbacks for 2 of 
> them to have in DT :
>
>   anbernic,rg35xx-2024-panel
>   anbernic,rg35xx-plus-panel, anbernic,rg35xx-2024-panel
>   anbernic,rg35xx-h-panel, anbernic,rg35xx-2024-panel
>
> and only use anbernic,rg35xx-2024-panel in the driver.
>
> In this case bindings should be like:
>    properties:
>      compatible:
>        oneOf:
>          - const: anbernic,rg35xx-2024-panel
>          - items:
>              - enum:
>                  - anbernic,rg35xx-plus-panel
>                  - anbernic,rg35xx-h-panel
>              - const: anbernic,rg35xx-2024-panel
>
> (of course I selected rg35xx-2024 as the primary one, it could be 
> another, usually the older one)
>

If all are happy with this approach I will resubmit a V2 with that change this evening.

Regards,

Ryan

[PATCH 2/3] drm: panel: nv3052c: Correct WL-355608-A8 panel compatible

[Ryan Walklin]

As per the previous dt-binding commit, update the WL-355608-A8 panel
compatible to reflect the the integrating device vendor and name.

Signed-off-by: Ryan Walklin <ryan@testtoast.com>
---
 drivers/gpu/drm/panel/panel-newvision-nv3052c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
index ee0ce271205e3..cee40269d9bb8 100644
--- a/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
+++ b/drivers/gpu/drm/panel/panel-newvision-nv3052c.c
@@ -925,7 +925,7 @@ MODULE_DEVICE_TABLE(spi, nv3052c_ids);
 static const struct of_device_id nv3052c_of_match[] = {
 	{ .compatible = "leadtek,ltk035c5444t", .data = &ltk035c5444t_panel_info },
 	{ .compatible = "fascontek,fs035vg158", .data = &fs035vg158_panel_info },
-	{ .compatible = "wl-355608-a8", .data = &wl_355608_a8_panel_info },
+	{ .compatible = "anbernic,rg35xx-panel", .data = &wl_355608_a8_panel_info },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, nv3052c_of_match);
-- 
2.45.2

[PATCH 3/3] dt-bindings: display: panel: correct Anbernic RG35XX panel example

[Ryan Walklin]

make dt_bindings_check reports that sck-gpios and num-chipselects are
required for spi nodes, therefore add these to the example.

Signed-off-by: Ryan Walklin <ryan@testtoast.com>
---
 .../bindings/display/panel/anbernic,rg35xx-panel.yaml          | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
index a7d5ad0f29389..610601c1594f3 100644
--- a/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
+++ b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
@@ -40,6 +40,9 @@ examples:
         #address-cells = <1>;
         #size-cells = <0>;

+        sck-gpios = <&pio 8 9 GPIO_ACTIVE_HIGH>; // PI9
+        num-chipselects = <1>;
+
         panel@0 {
             compatible = "anbernic,rg35xx-panel";
             reg = <0>;
-- 
2.45.2

Re: [PATCH 3/3] dt-bindings: display: panel: correct Anbernic RG35XX panel example

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]

On Wed, Jun 26, 2024 at 11:17:50PM +1200, Ryan Walklin wrote:
> make dt_bindings_check reports that sck-gpios and num-chipselects are
> required for spi nodes, therefore add these to the example.
> 
> Signed-off-by: Ryan Walklin <ryan@testtoast.com>

huh, are you sure it does? It doesn't for me...

> ---
>  .../bindings/display/panel/anbernic,rg35xx-panel.yaml          | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
> index a7d5ad0f29389..610601c1594f3 100644
> --- a/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-panel.yaml
> @@ -40,6 +40,9 @@ examples:
>          #address-cells = <1>;
>          #size-cells = <0>;
> 
> +        sck-gpios = <&pio 8 9 GPIO_ACTIVE_HIGH>; // PI9
> +        num-chipselects = <1>;
> +
>          panel@0 {
>              compatible = "anbernic,rg35xx-panel";
>              reg = <0>;
> -- 
> 2.45.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/3] dt-bindings: display: panel: correct Anbernic RG35XX panel example

[Ryan Walklin]

Hi Conor,

On Thu, 27 Jun 2024, at 3:38 AM, Conor Dooley wrote:
> On Wed, Jun 26, 2024 at 11:17:50PM +1200, Ryan Walklin wrote:
>> make dt_bindings_check reports that sck-gpios and num-chipselects are
>> required for spi nodes, therefore add these to the example.


> huh, are you sure it does? It doesn't for me...

With:
make dt_binding_check -j32 -s -W 2

I get the following output:

/home/ryan/Projects/Develop/RG35XX+/kernel/linux/Documentation/devicetree/bindings/display/panel/anbernic,wl-355608-a8.example.dtb: spi: 'num-chipselects' is a required property
	from schema $id: http://devicetree.org/schemas/spi/spi-gpio.yaml#
/home/ryan/Projects/Develop/RG35XX+/kernel/linux/Documentation/devicetree/bindings/display/panel/anbernic,wl-355608-a8.example.dtb: spi: 'sck-gpios' is a required property
	from schema $id: http://devicetree.org/schemas/spi/spi-gpio.yaml#

The relevant section from spi-gpio.yaml seems to be:

required:
  - compatible
  - num-chipselects
  - sck-gpios

I am happy to leave this out if a simpler example is desired, but alternately if the chipselects/sck-gpios should stay then I would suggest a better example would include the mosi/miso/cs GPIOs as well which would make the example functional.

Regards,

Ryan

Re: [PATCH 0/3] Correct WL-355608-A8 panel compatible

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 723 bytes --]

Hi,

On Wed, Jun 26, 2024 at 11:17:47PM GMT, Ryan Walklin wrote:
> The previous patch adding support for this panel [1] referred to previously by its serial number only. As discussed after the patch was committed, the preference is to use the integrating device vendor and name in this circumstance.
> 
> This series corrects the panel compatible to reflect the vendor (Anbernic, already in the vendor prefix table), updates the NV3052C panel driver with the new compatible, and lastly adds num-chipselects and sck-gpios to the DT binding example, identified by make dt_bindings_check as required for bit-banged SPI over GPIO lines.

Where are we on this?

The 6.11 release comes dangerously close now.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]