[Letux-kernel] [RFC v3 1/8] dt-bindings: display: convert ingenic, lcd.txt to ingenic, lcd.yaml

Sam Ravnborg sam at ravnborg.org
Thu Apr 9 09:25:59 CEST 2020


Hi Nikolaus


Some comments below that will result in a simplee binding that passes
the checks.
Thanks for pushing this.

	Sam

On Sun, Mar 29, 2020 at 07:35:47PM +0200, H. Nikolaus Schaller wrote:
> and add compatible: jz4780-lcd, including an example how to
> configure both lcd controllers.
> 
> Also fix the clock names and examples.
> 
> Based on work by Paul Cercueil <paul at crapouillou.net> and
> Sam Ravnborg <sam at ravnborg.org>
> 
> Signed-off-by: H. Nikolaus Schaller <hns at goldelico.com>
> Cc: Rob Herring <robh at kernel.org>
> Cc: devicetree at vger.kernel.org
> ---
>  .../bindings/display/ingenic,lcd.txt          |  45 ------
>  .../bindings/display/ingenic,lcd.yaml         | 128 ++++++++++++++++++
>  2 files changed, 128 insertions(+), 45 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/display/ingenic,lcd.txt
>  create mode 100644 Documentation/devicetree/bindings/display/ingenic,lcd.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/ingenic,lcd.txt b/Documentation/devicetree/bindings/display/ingenic,lcd.txt
> deleted file mode 100644
> index 01e3261defb6..000000000000
> --- a/Documentation/devicetree/bindings/display/ingenic,lcd.txt
> +++ /dev/null
> @@ -1,45 +0,0 @@
> -Ingenic JZ47xx LCD driver
> -
> -Required properties:
> -- compatible: one of:
> -  * ingenic,jz4740-lcd
> -  * ingenic,jz4725b-lcd
> -  * ingenic,jz4770-lcd
> -- reg: LCD registers location and length
> -- clocks: LCD pixclock and device clock specifiers.
> -	   The device clock is only required on the JZ4740.
> -- clock-names: "lcd_pclk" and "lcd"
> -- interrupts: Specifies the interrupt line the LCD controller is connected to.
> -
> -Example:
> -
> -panel {
> -	compatible = "sharp,ls020b1dd01d";
> -
> -	backlight = <&backlight>;
> -	power-supply = <&vcc>;
> -
> -	port {
> -		panel_input: endpoint {
> -			remote-endpoint = <&panel_output>;
> -		};
> -	};
> -};
> -
> -
> -lcd: lcd-controller at 13050000 {
> -	compatible = "ingenic,jz4725b-lcd";
> -	reg = <0x13050000 0x1000>;
> -
> -	interrupt-parent = <&intc>;
> -	interrupts = <31>;
> -
> -	clocks = <&cgu JZ4725B_CLK_LCD>;
> -	clock-names = "lcd";
> -
> -	port {
> -		panel_output: endpoint {
> -			remote-endpoint = <&panel_input>;
> -		};
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/display/ingenic,lcd.yaml b/Documentation/devicetree/bindings/display/ingenic,lcd.yaml
> new file mode 100644
> index 000000000000..8b6467cfc191
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/ingenic,lcd.yaml
> @@ -0,0 +1,128 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/ingenic,lcd.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Bindings for Ingenic JZ4780 LCD Controller
> +
> +maintainers:
> +  - Paul Cercueil <paul at crapouillou.net>
> +
> +description: |
> +  LCD Controller is the Display Controller for the Ingenic JZ47xx SoC
> +
> +properties:
> +  compatible:
> +    oneOf:
> +     - const: ingenic,jz4725b-lcd
> +     - const: ingenic,jz4740-lcd
> +     - const: ingenic,jz4770-lcd
> +     - const: ingenic,jz4780-lcd
> +
> +  reg:
> +    maxItems: 1
> +    description: LCD registers location and length
> +
> +  interrupts:
> +    maxItems: 1
> +    description: Specifies the interrupt provided by parent
> +
> +  clocks:
> +    maxItems: 2
> +    description: Clock specifiers for LCD pixclock and device clock.
> +      The device clock is only required on the JZ4740 and JZ4780
> +
> +  clock-names:
> +    items:
> +      - const: lcd
> +      - const: lcd_pclk
> +
> +  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/jz4725b-cgu.h>
> +
> +    panel {
> +      compatible = "sharp,ls020b1dd01d";
> +
> +      backlight = <&backlight>;
> +      power-supply = <&vcc>;
> +
> +      port {
> +        panel_input: endpoint {
> +          remote-endpoint = <&panel_output>;
> +          };
> +        };
> +      };
The panel part is not needed - better to drop it.


> +
> +    lcd: lcd-controller at 13050000 {
> +      compatible = "ingenic,jz4725b-lcd";
> +      reg = <0x13050000 0x1000>;
> +
> +      interrupt-parent = <&intc>;
> +      interrupts = <31>;
> +
> +      clocks = <&cgu JZ4725B_CLK_LCD>;
> +      clock-names = "lcd", "lcd_pclk";
> +
> +      port {
> +        panel_output: endpoint {
> +          remote-endpoint = <&panel_input>;
> +          };
> +        };
> +      };
We know this example will not pass the check, as there is only
one clock specified.
I suggest to drop this example.
If it later turns out that jz4725b only have one clock, then the binding
needs to be updated. But the best guess is that the example is wrong.

The example below for jz4780-lcd cover all relevant parts - so
just keep it as the only example.

> +
> +  - |
> +    #include <dt-bindings/clock/jz4780-cgu.h>
> +
> +    lcdc0: lcdc0 at 13050000 {
Name this lcdc
And drop "lcdc0 at 13050000" as this is not relevant for this example.

Remember - the examples exist to explain the binding. They are
just examples.

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

And drop this as it does not add anything extra.
> +    lcdc1: lcdc1 at 130a0000 {
> +        compatible = "ingenic,jz4780-lcd";
> +        reg = <0x130a0000 0x1800>;
> +
> +        clocks = <&cgu JZ4780_CLK_TVE>, <&cgu JZ4780_CLK_LCD1PIXCLK>;
> +        clock-names = "lcd", "lcd_pclk";
> +
> +        interrupt-parent = <&intc>;
> +        interrupts = <31>;
> +
> +        status = "disabled";
> +    };

	Sam


More information about the Letux-kernel mailing list