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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 02 Mar 2020 16:10:47 -0300
From:   Paul Cercueil <paul@...pouillou.net>
To:     "H. Nikolaus Schaller" <hns@...delico.com>
Cc:     Paul Boddie <paul@...die.org.uk>, David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Paul Burton <paulburton@...nel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Andi Kleen <ak@...ux.intel.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-mips@...r.kernel.org,
        linux-gpio@...r.kernel.org, letux-kernel@...nphoenux.org,
        kernel@...a-handheld.com, Sam Ravnborg <sam@...nborg.org>,
        Zubair Lutfullah Kakakhel <Zubair.Kakakhel@...tec.com>,
        Rob Herring <robh@...nel.org>
Subject: Re: [RFC v2 1/8] dt-bindings: display: add ingenic-jz4780-lcd DT
 Schema

Hi Nikolaus,


Le ven., févr. 28, 2020 at 19:19, H. Nikolaus Schaller 
<hns@...delico.com> a écrit :
> From: Sam Ravnborg <sam@...nborg.org>
> 
> 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 <sam@...nborg.org>
> Cc: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@...tec.com>
> Cc: H. Nikolaus Schaller <hns@...delico.com>
> Cc: Rob Herring <robh@...nel.org>
> Cc: devicetree@...r.kernel.org
> ---
>  .../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 <Zubair.Kakakhel@...tec.com>
> +  - H. Nikolaus Schaller <hns@...delico.com>

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@...3050000 {

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
>