diff mbox series

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

Message ID 20241124080220.1657238-2-kikuchan98@gmail.com (mailing list archive)
State New
Headers show
Series drm/panel: nv3052c: Add support for new Anbernic panels | expand

Commit Message

Hironori KIKUCHI Nov. 24, 2024, 8:02 a.m. UTC
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

Comments

Krzysztof Kozlowski Nov. 24, 2024, 4:01 p.m. UTC | #1
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
Krzysztof Kozlowski Nov. 24, 2024, 4:05 p.m. UTC | #2
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
Hironori KIKUCHI Nov. 24, 2024, 7:14 p.m. UTC | #3
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
Krzysztof Kozlowski Nov. 25, 2024, 7:38 a.m. UTC | #4
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
Hironori KIKUCHI Nov. 27, 2024, 3:32 a.m. UTC | #5
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
Krzysztof Kozlowski Nov. 27, 2024, 7:22 a.m. UTC | #6
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
diff mbox series

Patch

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>;
+                };
+            };
+        };
+    };