[PATCH 1/6] dt-bindings: display: panel: Add another panel for RG35XX Plus (Rev6)

Hironori KIKUCHI posted 6 patches 1 year, 2 months ago
[PATCH 1/6] dt-bindings: display: panel: Add another panel for RG35XX Plus (Rev6)
Posted by Hironori KIKUCHI 1 year, 2 months ago
This is a display panel used in the recent revision of the Anbernic
RG35XX Plus, a handheld gaming device from Anbernic.
It is 3.45 inches in size (diagonally) with a resolution of 640x480.

It has the same interface (pins and connector) as the panel of the former
revision of RG35XX Plus, but they differ in their init-sequence. So add
it as a new panel.

Signed-off-by: Hironori KIKUCHI <kikuchan98@gmail.com>
---
 .../anbernic,rg35xx-plus-rev6-panel.yaml      | 60 +++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-plus-rev6-panel.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-plus-rev6-panel.yaml b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-plus-rev6-panel.yaml
new file mode 100644
index 00000000000..b60a4cf00f8
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-plus-rev6-panel.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/anbernic,rg35xx-plus-rev6-panel.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Anbernic RG35XX series (YLM-LBV0345001H-V2) 3.45" 640x480 24-bit IPS LCD panel
+
+maintainers:
+  - Hironori KIKUCHI <kikuchan98@gmail.com>
+
+allOf:
+  - $ref: panel-common.yaml#
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    const: anbernic,rg35xx-plus-rev6-panel
+
+  reg:
+    maxItems: 1
+
+  spi-3wire: true
+
+required:
+  - compatible
+  - reg
+  - port
+  - power-supply
+  - reset-gpios
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        panel@0 {
+            compatible = "anbernic,rg35xx-plus-rev6-panel";
+            reg = <0>;
+
+            spi-3wire;
+            spi-max-frequency = <3125000>;
+
+            reset-gpios = <&pio 8 14 GPIO_ACTIVE_LOW>; // PI14
+
+            backlight = <&backlight>;
+            power-supply = <&reg_lcd>;
+
+            port {
+                endpoint {
+                    remote-endpoint = <&tcon_lcd0_out_lcd>;
+                };
+            };
+        };
+    };
-- 
2.47.0
Re: [PATCH 1/6] dt-bindings: display: panel: Add another panel for RG35XX Plus (Rev6)
Posted by Chris Morgan 1 year ago
On Sun, Nov 24, 2024 at 05:02:12PM +0900, Hironori KIKUCHI wrote:
> This is a display panel used in the recent revision of the Anbernic
> RG35XX Plus, a handheld gaming device from Anbernic.
> It is 3.45 inches in size (diagonally) with a resolution of 640x480.
> 
> It has the same interface (pins and connector) as the panel of the former
> revision of RG35XX Plus, but they differ in their init-sequence. So add
> it as a new panel.
> 
> Signed-off-by: Hironori KIKUCHI <kikuchan98@gmail.com>
> ---
>  .../anbernic,rg35xx-plus-rev6-panel.yaml      | 60 +++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-plus-rev6-panel.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-plus-rev6-panel.yaml b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-plus-rev6-panel.yaml
> new file mode 100644
> index 00000000000..b60a4cf00f8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-plus-rev6-panel.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/anbernic,rg35xx-plus-rev6-panel.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Anbernic RG35XX series (YLM-LBV0345001H-V2) 3.45" 640x480 24-bit IPS LCD panel
> +
> +maintainers:
> +  - Hironori KIKUCHI <kikuchan98@gmail.com>
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    const: anbernic,rg35xx-plus-rev6-panel
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-3wire: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - port
> +  - power-supply
> +  - reset-gpios
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        panel@0 {
> +            compatible = "anbernic,rg35xx-plus-rev6-panel";
> +            reg = <0>;
> +
> +            spi-3wire;
> +            spi-max-frequency = <3125000>;
> +
> +            reset-gpios = <&pio 8 14 GPIO_ACTIVE_LOW>; // PI14
> +
> +            backlight = <&backlight>;
> +            power-supply = <&reg_lcd>;
> +
> +            port {
> +                endpoint {
> +                    remote-endpoint = <&tcon_lcd0_out_lcd>;
> +                };
> +            };
> +        };
> +    };
> -- 
> 2.47.0
> 

Though the documentation file will likely change (to newvision,nv3052c.yaml
if I understand correctly) do we know if "anbernic,rg35xx-plus-rev6-panel" is
an acceptable compatible string for this panel? I'd like to add it to a fixup
in U-Boot but can't proceed until the string is defined.

Thank you,
Chris
Re: [PATCH 1/6] dt-bindings: display: panel: Add another panel for RG35XX Plus (Rev6)
Posted by Krzysztof Kozlowski 1 year, 2 months ago
On 24/11/2024 09:02, Hironori KIKUCHI wrote:
> +++ b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-plus-rev6-panel.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/anbernic,rg35xx-plus-rev6-panel.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Anbernic RG35XX series (YLM-LBV0345001H-V2) 3.45" 640x480 24-bit IPS LCD panel
> +
> +maintainers:
> +  - Hironori KIKUCHI <kikuchan98@gmail.com>
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    const: anbernic,rg35xx-plus-rev6-panel

Everything is the same here. Add new compatible to existing schema
respecting compatibility (fallback) or not (no fallback).


Best regards,
Krzysztof
Re: [PATCH 1/6] dt-bindings: display: panel: Add another panel for RG35XX Plus (Rev6)
Posted by Krzysztof Kozlowski 1 year, 2 months ago
On 24/11/2024 17:01, Krzysztof Kozlowski wrote:
> On 24/11/2024 09:02, Hironori KIKUCHI wrote:
>> +++ b/Documentation/devicetree/bindings/display/panel/anbernic,rg35xx-plus-rev6-panel.yaml
>> @@ -0,0 +1,60 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/panel/anbernic,rg35xx-plus-rev6-panel.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Anbernic RG35XX series (YLM-LBV0345001H-V2) 3.45" 640x480 24-bit IPS LCD panel
>> +
>> +maintainers:
>> +  - Hironori KIKUCHI <kikuchan98@gmail.com>
>> +
>> +allOf:
>> +  - $ref: panel-common.yaml#
>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: anbernic,rg35xx-plus-rev6-panel
> 
> Everything is the same here. Add new compatible to existing schema
> respecting compatibility (fallback) or not (no fallback).

BTW, isn't this v2? Where is the changelog and proper versioning of
patches? The changes against previous version are quite unexpected. You
removed specific compatibles and went with wildcard, so this needs
thorough explanation (why standard bindings rules do not apply here).

Anyway next version *will be v3*. Please start using b4 tool.


Best regards,
Krzysztof
Re: [PATCH 1/6] dt-bindings: display: panel: Add another panel for RG35XX Plus (Rev6)
Posted by Hironori KIKUCHI 1 year, 2 months ago
Hello Krzysztof,

Thank you for reviewing.

> no wildcards

Sorry, but I believe these are not wildcards.

As discussed previously, the integrating vendor and device name are
preferred instead of the OEM serial for unidentified OEM panels.
These compatible strings are based on the actual device names:
  "RG35XX Plus", "RG 40XXV", "RG40XX H", and "RG CubeXX"
You can refer to
https://anbernic.com/collections/handheld-game-console for the full
line-up.

Oh, regarding "rg40xx-panel", it might have been separated to
"rg40xx-v-panel" and "rg40xx-h-panel".


> don't duplicate schemas

The old schemas "leadtek,ltk035c5444t", "fascontek,fs035vg158", and
"anbernic,rg35xx-plus-panel" exist independently.
So I had to add new schemas since the new ones are not compatible with
the old ones.

Perhaps the compatibles should be like this:
  compatible = "anbernic,rg35xx-plus-panel", "newvision,nv3052c";
as some others do.

In this way, the schema files would be a single file and not be messed
up, but it would break the previously defined schemas.

How should I deal with this?
Any suggestions or advice would be greatly appreciated.


> BTW, isn't this v2? Where is the changelog and proper versioning of
> patches?

Sorry, I thought the previous version was completely rejected due to
renaming back to "wl-355608-a8".
https://lore.kernel.org/dri-devel/20241105-maybe-chamomile-7505214f737e@spud/

But yes, these are somewhat relevant, I'll post the next version as v3
with changelogs. Thanks.

Best regards,
kikuchan
Re: [PATCH 1/6] dt-bindings: display: panel: Add another panel for RG35XX Plus (Rev6)
Posted by Krzysztof Kozlowski 1 year, 2 months ago
On 24/11/2024 20:14, Hironori KIKUCHI wrote:
> Hello Krzysztof,
> 
> Thank you for reviewing.
> 
>> no wildcards
> 
> Sorry, but I believe these are not wildcards.
> 
> As discussed previously, the integrating vendor and device name are
> preferred instead of the OEM serial for unidentified OEM panels.
> These compatible strings are based on the actual device names:
>   "RG35XX Plus", "RG 40XXV", "RG40XX H", and "RG CubeXX"
> You can refer to
> https://anbernic.com/collections/handheld-game-console for the full


Then explain this in commit msg.

> line-up.
> 
> Oh, regarding "rg40xx-panel", it might have been separated to
> "rg40xx-v-panel" and "rg40xx-h-panel".
> 
> 
>> don't duplicate schemas
> 
> The old schemas "leadtek,ltk035c5444t", "fascontek,fs035vg158", and
> "anbernic,rg35xx-plus-panel" exist independently.


So you duplicate them. I wrote: Don't duplicate.

> So I had to add new schemas since the new ones are not compatible with
> the old ones.

No, you do not have to. There is no such thing as schema compatible with
schema.


> 
> Perhaps the compatibles should be like this:
>   compatible = "anbernic,rg35xx-plus-panel", "newvision,nv3052c";
> as some others do.

We do not talk about compatibles.

> 
> In this way, the schema files would be a single file and not be messed
> up, but it would break the previously defined schemas.


What? No, it would not  break. Don't touch existing compatibles.

> 
> How should I deal with this?
> Any suggestions or advice would be greatly appreciated.

The same as with all other changes: add to existing files.

Best regards,
Krzysztof
Re: [PATCH 1/6] dt-bindings: display: panel: Add another panel for RG35XX Plus (Rev6)
Posted by Hironori KIKUCHI 1 year, 2 months ago
Hello Krzysztof,

Thank you for your reply.

> > The old schemas "leadtek,ltk035c5444t", "fascontek,fs035vg158", and
> > "anbernic,rg35xx-plus-panel" exist independently.
> So you duplicate them. I wrote: Don't duplicate.

Ok, thanks. I won't duplicate.

They are already duplicated in the tree with their own file names.
The panels I want to add are not directly relevant to them, so there
is no single file suitable for the panels.

Should I merge these files into a single file with a file name such as
`newvision,nv3052c.yaml`, taken from the driver name?

Best regards,
kikuchan
Re: [PATCH 1/6] dt-bindings: display: panel: Add another panel for RG35XX Plus (Rev6)
Posted by Krzysztof Kozlowski 1 year, 2 months ago
On 27/11/2024 04:32, Hironori KIKUCHI wrote:
> Hello Krzysztof,
> 
> Thank you for your reply.
> 
>>> The old schemas "leadtek,ltk035c5444t", "fascontek,fs035vg158", and
>>> "anbernic,rg35xx-plus-panel" exist independently.
>> So you duplicate them. I wrote: Don't duplicate.
> 
> Ok, thanks. I won't duplicate.
> 
> They are already duplicated in the tree with their own file names.
> The panels I want to add are not directly relevant to them, so there
> is no single file suitable for the panels.
> 
> Should I merge these files into a single file with a file name such as
> `newvision,nv3052c.yaml`, taken from the driver name?
Add it to the existing anbernic.

Best regards,
Krzysztof