linux-kernel - Re: [PATCH] dt-bindings: display: atmel,lcdc: convert to dtschema

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]
Message-ID: <796652d5-af57-4aca-87c2-10a7b0b55959@linaro.org>
Date: Tue, 27 Feb 2024 13:03:45 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Dharma Balasubiramani <dharma.b@...rochip.com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
 Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Nicolas Ferre <nicolas.ferre@...rochip.com>,
 Alexandre Belloni <alexandre.belloni@...tlin.com>,
 Claudiu Beznea <claudiu.beznea@...on.dev>
Cc: dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dt-bindings: display: atmel,lcdc: convert to dtschema

On 23/02/2024 10:14, Dharma Balasubiramani wrote:
> Convert the atmel,lcdc bindings to DT schema.
> Changes during conversion: add missing clocks and clock-names properties.
> 
> Signed-off-by: Dharma Balasubiramani <dharma.b@...rochip.com>
> ---
>  .../devicetree/bindings/display/atmel,lcdc.txt     |  87 --------------
>  .../devicetree/bindings/display/atmel,lcdc.yaml    | 133 +++++++++++++++++++++
>  2 files changed, 133 insertions(+), 87 deletions(-)

You have several patch errors... check your git repo (git show). You
will easily spot them. Or just use decent text editor to clean it up.
Run checkpatch...

..

> diff --git a/Documentation/devicetree/bindings/display/atmel,lcdc.yaml b/Documentation/devicetree/bindings/display/atmel,lcdc.yaml
> new file mode 100644
> index 000000000000..4a1de5a8d64b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/atmel,lcdc.yaml
> @@ -0,0 +1,133 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/atmel,lcdc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip's LCDC Framebuffer
> +
> +maintainers:
> +  - Nicolas Ferre <nicolas.ferre@...rochip.com>
> +  - Dharma Balasubiramani <dharma.b@...rochip.com>
> +
> +description:
> +  The LCDC works with a framebuffer, which is a section of memory that contains
> +  a complete frame of data representing pixel values for the display. The LCDC
> +  reads the pixel data from the framebuffer and sends it to the LCD panel to
> +  render the image.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - atmel,at91sam9261-lcdc 
> +      - atmel,at91sam9263-lcdc
> +      - atmel,at91sam9g10-lcdc
> +      - atmel,at91sam9g45-lcdc
> +      - atmel,at91sam9g45es-lcdc
> +      - atmel,at91sam9rl-lcdc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks: 
> +    maxItems: 2
> + 
> +  clock-names:
> +    items:
> +      - const: hclk
> +      - const: lcdc_clk
> +
> +  display:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: A phandle pointing to the display node.

Phandle does not have properties. Didn't you want object?

This cannot work - just test it. Change the properties in the example,
remove or add something. Do you see errors? No, because it does not work
at all.

I don't know what's this exactly, but if embedded display then maybe
could be part of this device node. If some other display, then maybe you
need another schema, with compatible? But first I would check how others
are doing this.


> +
> +    properties:
> +      atmel,dmacon:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: dma controller configuration
> +
> +      atmel,lcdcon2:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: lcd controller configuration
> +
> +      atmel,guard-time:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: lcd guard time (Delay in frame periods)
> +
> +      bits-per-pixel:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: lcd panel bit-depth.
> +
> +      atmel,lcdcon-backlight: 
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: enable backlight
> +
> +      atmel,lcdcon-backlight-inverted:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: invert backlight PWM polarity
> +
> +      atmel,lcd-wiring-mode:
> +        $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> +        description: lcd wiring mode "RGB" or "BRG"
> +
> +      atmel,power-control-gpio:
> +        description: gpio to power on or off the LCD (as many as needed)
> +
> +    required:
> +      - atmel,dmacon
> +      - atmel,lcdcon2
> +      - atmel,guard-time
> +      - bits-per-pixel 
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - display
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/at91.h>
> +    fb@...000 {
> +      compatible = "atmel,at91sam9g45-lcdc";
> +      reg = <0x00500000 0x1000>;
> +      interrupts = <23 3 0>;

Aren't here some standard interrupt flags?

> +      pinctrl-names = "default";
> +      pinctrl-0 = <&pinctrl_fb>;
> +      clocks = <&pmc PMC_TYPE_PERIPHERAL 23>, <&pmc PMC_TYPE_PERIPHERAL 23>;
> +      clock-names = "hclk", "lcdc_clk";
> +      display = <&display0>;
> +    };
> +


Best regards,
Krzysztof