[RFC,v2,1/8] dt-bindings: display: add ingenic-jz4780-lcd DT Schema

Commit Message

H. Nikolaus Schaller Feb. 28, 2020, 6:19 p.m. UTC

From: Sam Ravnborg <[e-mail]>

Add DT bindings for the LCD controller on the jz4780 SoC
Based on .txt binding from Zubair Lutfullah Kakakhel

Signed-off-by: Sam Ravnborg <[e-mail]>
Cc: Zubair Lutfullah Kakakhel <[e-mail]>
Cc: H. Nikolaus Schaller <[e-mail]>
Cc: Rob Herring <[e-mail]>
Cc: [e-mail]
---
 .../bindings/display/ingenic-jz4780-lcd.yaml  | 78 +++++++++++++++++++
 1 file changed, 78 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml

Comments

H. Nikolaus Schaller Feb. 28, 2020, 6:51 p.m. UTC | #1

Hi Sam,

> Am 28.02.2020 um 19:42 schrieb Sam Ravnborg <[e-mail]>:
> 
> Hi Nikolaus.
> 
> On Fri, Feb 28, 2020 at 07:19:26PM +0100, H. Nikolaus Schaller wrote:
>> From: Sam Ravnborg <[e-mail]>
>> 
>> Add DT bindings for the LCD controller on the jz4780 SoC
>> Based on .txt binding from Zubair Lutfullah Kakakhel
>> 
>> Signed-off-by: Sam Ravnborg <[e-mail]>
>> Cc: Zubair Lutfullah Kakakhel <[e-mail]>
>> Cc: H. Nikolaus Schaller <[e-mail]>
>> Cc: Rob Herring <[e-mail]>
>> Cc: [e-mail]
> 
> As this patch was sent to you and you forward it you need to
> testify that this is OK.
> To do so follow the rules of the Developemnt Certificate of Origin
> as can be found in SubmittingPatches.rst.
> 
> In other words - you need to add your Signed-off-by: xxx <mail>
> to the patch.
> In the end we want to be able to see the patch the patch has taken
> reading the Signed-off-by: lines from top to bottom.

Ok, never someone explained this as precise as you did. Thanks!

> Please check other patches in this series for the same issue.

Ok.

BR and thanks,
Nikolaus

> 
> 	Sam
> 
>> ---
>> .../bindings/display/ingenic-jz4780-lcd.yaml  | 78 +++++++++++++++++++
>> 1 file changed, 78 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml b/Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml
>> new file mode 100644
>> index 000000000000..c71415a3a342
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml
>> @@ -0,0 +1,78 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/ingenic-jz4780-lcd.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Bindings for Ingenic JZ4780 LCD Controller
>> +
>> +maintainers:
>> +  - Zubair Lutfullah Kakakhel <[e-mail]>
>> +  - H. Nikolaus Schaller <[e-mail]>
>> +
>> +description: |
>> +  LCD Controller is the Display Controller for the Ingenic JZ4780 SoC
>> +
>> +properties:
>> +  compatible:
>> +    items:
>> +      - const: ingenic,jz4780-lcd
>> +
>> +  reg:
>> +    maxItems: 1
>> +    description: the address & size of the LCD controller registers
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +    description: Specifies the interrupt provided by parent
>> +
>> +  clocks:
>> +    maxItems: 2
>> +    description: Clock specifiers for the JZ4780_CLK_TVE JZ4780_CLK_LCD0PIXCLK
>> +
>> +  clock-names:
>> +    items:
>> +      - const: lcd_clk
>> +      - const: lcd_pixclk
>> +
>> +  port:
>> +    type: object
>> +    description: |
>> +      A port node with endpoint definitions as defined in
>> +      Documentation/devicetree/bindings/media/video-interfaces.txt
>> +
>> +required:
>> +    - compatible
>> +    - reg
>> +    - interrupts
>> +    - clocks
>> +    - clock-names
>> +    - port
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/jz4780-cgu.h>
>> +    lcd: jz4780-lcdk@0x13050000 {
>> +        compatible = "ingenic,jz4780-lcd";
>> +        reg = <0x13050000 0x1800>;
>> +
>> +        clocks = <&cgu JZ4780_CLK_TVE>, <&cgu JZ4780_CLK_LCD0PIXCLK>;
>> +        clock-names = "lcd_clk", "lcd_pixclk";
>> +
>> +        interrupt-parent = <&intc>;
>> +        interrupts = <31>;
>> +
>> +        jz4780_lcd_out: port {
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +
>> +            jz4780_out_hdmi: endpoint@0 {
>> +                reg = <0>;
>> +                remote-endpoint = <&hdmi_in_lcd>;
>> +            };
>> +        };
>> +    };
>> +
>> +...
>> -- 
>> 2.23.0




Dr. Nikolaus Schaller
Geschäftsführer / Managing Director

Modiblast Pharma GmbH
Buchenstraße 3
82041 Oberhaching

Tel. [phone] 
Mail     [e-mail]

Der Inhalt dieser E-Mail (einschliesslich etwaiger beigefuegter Dateien) ist vertraulich und nur für den Empfaenger bestimmt. Sollten Sie nicht der bestimmungsgemaesse Empfaenger sein, ist Ihnen jegliche Offenlegung, Vervielfaeltigung, Weitergabe oder Nutzung des Inhalts untersagt. Bitte informieren Sie in diesem Fall unverzueglich den Absender und loeschen Sie die E-Mail (einschliesslich etwaiger beigefuegter Dateien) von Ihrem System.    
Vielen Dank.

The contents of this e-mail (including any attachments) are confidential and may be legally privileged. If you are not the intended recipient of this e-mail, any disclosure, copying, distribution or use of its contents is strictly prohibited, and you should please notify the sender immediately and then delete it (including any attachments) from your system.
Thank you.

HRB 254854 (Amtsgericht München)
Geschäftsführer: Prof. Dr. Helga Schmetzer, Dr. Octavian Schatz, Dr. Nikolaus Schaller
www.modiblast.com

Rob Herring (Arm) March 2, 2020, 6:55 p.m. UTC | #2

On Fri, 28 Feb 2020 19:19:26 +0100, "H. Nikolaus Schaller" wrote:
> From: Sam Ravnborg <[e-mail]>
> 
> Add DT bindings for the LCD controller on the jz4780 SoC
> Based on .txt binding from Zubair Lutfullah Kakakhel
> 
> Signed-off-by: Sam Ravnborg <[e-mail]>
> Cc: Zubair Lutfullah Kakakhel <[e-mail]>
> Cc: H. Nikolaus Schaller <[e-mail]>
> Cc: Rob Herring <[e-mail]>
> Cc: [e-mail]
> ---
>  .../bindings/display/ingenic-jz4780-lcd.yaml  | 78 +++++++++++++++++++
>  1 file changed, 78 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

Documentation/devicetree/bindings/display/simple-framebuffer.example.dts:21.16-37.11: Warning (chosen_node_is_root): /example-0/chosen: chosen node must be at root node
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.example.dt.yaml: example-0: 'jz4780-lcdk@0x13050000' does not match any of the regexes: '.*-names$', '.*-supply$', '^#.*-cells$', '^#[a-zA-Z0-9,+\\-._]{0,63}$', '^[a-zA-Z][a-zA-Z0-9,+\\-._]{0,63}$', '^[a-zA-Z][a-zA-Z0-9,+\\-._]{0,63}@[0-9a-fA-F]+(,[0-9a-fA-F]+)*$', '^__.*__$', 'pinctrl-[0-9]+'

See https://patchwork.ozlabs.org/patch/1246780
Please check and re-submit.

Paul Cercueil March 2, 2020, 7:10 p.m. UTC | #3

Hi Nikolaus,


Le ven., févr. 28, 2020 at 19:19, H. Nikolaus Schaller 
<[e-mail]> a écrit :
> From: Sam Ravnborg <[e-mail]>
> 
> Add DT bindings for the LCD controller on the jz4780 SoC
> Based on .txt binding from Zubair Lutfullah Kakakhel

If you mean Documentation/devicetree/bindings/display/ingenic,lcd.txt 
then it was written by me.

> Signed-off-by: Sam Ravnborg <[e-mail]>
> Cc: Zubair Lutfullah Kakakhel <[e-mail]>
> Cc: H. Nikolaus Schaller <[e-mail]>
> Cc: Rob Herring <[e-mail]>
> Cc: [e-mail]
> ---
>  .../bindings/display/ingenic-jz4780-lcd.yaml  | 78 
> +++++++++++++++++++
>  1 file changed, 78 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml 
> b/Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml
> new file mode 100644
> index 000000000000..c71415a3a342
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml
> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/ingenic-jz4780-lcd.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Bindings for Ingenic JZ4780 LCD Controller
> +
> +maintainers:
> +  - Zubair Lutfullah Kakakhel <[e-mail]>
> +  - H. Nikolaus Schaller <[e-mail]>

I'm the author of the driver, please put me here; and remove Zubair, 
which 1. didn't touch the DRM driver at all, and 2. isn't working at 
ImgTec anymore. Also, no need to put yourself here, unless you maintain 
the Ingenic DRM/KMS driver.

> +
> +description: |
> +  LCD Controller is the Display Controller for the Ingenic JZ4780 SoC
> +
> +properties:

You should add a '$nodename' property.

> +  compatible:
> +    items:
> +      - const: ingenic,jz4780-lcd

The .txt lists more compatible strings. Please add them all.

> +
> +  reg:
> +    maxItems: 1
> +    description: the address & size of the LCD controller registers

Drop the description here,

> +
> +  interrupts:
> +    maxItems: 1
> +    description: Specifies the interrupt provided by parent

and here.

> +
> +  clocks:
> +    maxItems: 2
> +    description: Clock specifiers for the JZ4780_CLK_TVE 
> JZ4780_CLK_LCD0PIXCLK

Add one 'description:' per item.

> +
> +  clock-names:
> +    items:
> +      - const: lcd_clk
> +      - const: lcd_pixclk
> +
> +  port:
> +    type: object
> +    description: |
> +      A port node with endpoint definitions as defined in
> +      Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +required:
> +    - compatible
> +    - reg
> +    - interrupts
> +    - clocks
> +    - clock-names
> +    - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/jz4780-cgu.h>
> +    lcd: jz4780-lcdk@0x13050000 {

The node name does not comply with the DT spec, it should be 
'lcd-controller'.

Cheers,
-Paul

> +        compatible = "ingenic,jz4780-lcd";
> +        reg = <0x13050000 0x1800>;
> +
> +        clocks = <&cgu JZ4780_CLK_TVE>, <&cgu JZ4780_CLK_LCD0PIXCLK>;
> +        clock-names = "lcd_clk", "lcd_pixclk";
> +
> +        interrupt-parent = <&intc>;
> +        interrupts = <31>;
> +
> +        jz4780_lcd_out: port {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            jz4780_out_hdmi: endpoint@0 {
> +                reg = <0>;
> +                remote-endpoint = <&hdmi_in_lcd>;
> +            };
> +        };
> +    };
> +
> +...
> --
> 2.23.0
>

H. Nikolaus Schaller March 3, 2020, 7:21 a.m. UTC | #4

Hi Paul,

> Am 02.03.2020 um 20:10 schrieb Paul Cercueil <[e-mail]>:
> 
> Hi Nikolaus,
> 
> 
> Le ven., févr. 28, 2020 at 19:19, H. Nikolaus Schaller <[e-mail]> a écrit :
>> From: Sam Ravnborg <[e-mail]>
>> Add DT bindings for the LCD controller on the jz4780 SoC
>> Based on .txt binding from Zubair Lutfullah Kakakhel
> 
> If you mean Documentation/devicetree/bindings/display/ingenic,lcd.txt then it was written by me.

Ah, ok. We didn't recognise this before. 6 eyes see more than 4...

I just did cherry-pick this old 4.0 patch from 2015 by Zubair
and it created a ingenic-jz4780-lcd.txt:

https://lore.kernel.org/patchwork/patch/547872/

and Sam was so kind to convert it to yaml.

> 
>> Signed-off-by: Sam Ravnborg <[e-mail]>
>> Cc: Zubair Lutfullah Kakakhel <[e-mail]>
>> Cc: H. Nikolaus Schaller <[e-mail]>
>> Cc: Rob Herring <[e-mail]>
>> Cc: [e-mail]
>> ---
>> .../bindings/display/ingenic-jz4780-lcd.yaml  | 78 +++++++++++++++++++
>> 1 file changed, 78 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml
>> diff --git a/Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml b/Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml
>> new file mode 100644
>> index 000000000000..c71415a3a342
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml
>> @@ -0,0 +1,78 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/ingenic-jz4780-lcd.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Bindings for Ingenic JZ4780 LCD Controller
>> +
>> +maintainers:
>> +  - Zubair Lutfullah Kakakhel <[e-mail]>
>> +  - H. Nikolaus Schaller <[e-mail]>
> 
> I'm the author of the driver, please put me here; and remove Zubair, which 1. didn't touch the DRM driver at all, and 2. isn't working at ImgTec anymore.

Yes that is true.

> Also, no need to put yourself here, unless you maintain the Ingenic DRM/KMS driver.

Agreed. That was suggested by Sam.

> 
>> +
>> +description: |
>> +  LCD Controller is the Display Controller for the Ingenic JZ4780 SoC
>> +
>> +properties:
> 
> You should add a '$nodename' property.
> 
>> +  compatible:
>> +    items:
>> +      - const: ingenic,jz4780-lcd
> 
> The .txt lists more compatible strings. Please add them all.
> 
>> +
>> +  reg:
>> +    maxItems: 1
>> +    description: the address & size of the LCD controller registers
> 
> Drop the description here,
> 
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +    description: Specifies the interrupt provided by parent
> 
> and here.
> 
>> +
>> +  clocks:
>> +    maxItems: 2
>> +    description: Clock specifiers for the JZ4780_CLK_TVE JZ4780_CLK_LCD0PIXCLK
> 
> Add one 'description:' per item.
> 
>> +
>> +  clock-names:
>> +    items:
>> +      - const: lcd_clk
>> +      - const: lcd_pixclk
>> +
>> +  port:
>> +    type: object
>> +    description: |
>> +      A port node with endpoint definitions as defined in
>> +      Documentation/devicetree/bindings/media/video-interfaces.txt
>> +
>> +required:
>> +    - compatible
>> +    - reg
>> +    - interrupts
>> +    - clocks
>> +    - clock-names
>> +    - port
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/jz4780-cgu.h>
>> +    lcd: jz4780-lcdk@0x13050000 {
> 
> The node name does not comply with the DT spec, it should be 'lcd-controller'.

Ok, I think I'll review all so that it does match/replace
Documentation/devicetree/bindings/display/ingenic,lcd.txt
and no information is lost.
 
> 
> Cheers,
> -Paul

BR and thanks,
Nikolaus

Patch

diff --git a/Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml b/Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml
new file mode 100644
index 000000000000..c71415a3a342
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/ingenic-jz4780-lcd.yaml
@@ -0,0 +1,78 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/ingenic-jz4780-lcd.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bindings for Ingenic JZ4780 LCD Controller
+
+maintainers:
+  - Zubair Lutfullah Kakakhel <[e-mail]>
+  - H. Nikolaus Schaller <[e-mail]>
+
+description: |
+  LCD Controller is the Display Controller for the Ingenic JZ4780 SoC
+
+properties:
+  compatible:
+    items:
+      - const: ingenic,jz4780-lcd
+
+  reg:
+    maxItems: 1
+    description: the address & size of the LCD controller registers
+
+  interrupts:
+    maxItems: 1
+    description: Specifies the interrupt provided by parent
+
+  clocks:
+    maxItems: 2
+    description: Clock specifiers for the JZ4780_CLK_TVE JZ4780_CLK_LCD0PIXCLK
+
+  clock-names:
+    items:
+      - const: lcd_clk
+      - const: lcd_pixclk
+
+  port:
+    type: object
+    description: |
+      A port node with endpoint definitions as defined in
+      Documentation/devicetree/bindings/media/video-interfaces.txt
+
+required:
+    - compatible
+    - reg
+    - interrupts
+    - clocks
+    - clock-names
+    - port
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/jz4780-cgu.h>
+    lcd: jz4780-lcdk@0x13050000 {
+        compatible = "ingenic,jz4780-lcd";
+        reg = <0x13050000 0x1800>;
+
+        clocks = <&cgu JZ4780_CLK_TVE>, <&cgu JZ4780_CLK_LCD0PIXCLK>;
+        clock-names = "lcd_clk", "lcd_pixclk";
+
+        interrupt-parent = <&intc>;
+        interrupts = <31>;
+
+        jz4780_lcd_out: port {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            jz4780_out_hdmi: endpoint@0 {
+                reg = <0>;
+                remote-endpoint = <&hdmi_in_lcd>;
+            };
+        };
+    };
+
+...