[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <11c545e2-45df-4587-a5c7-12b05c2f01e0@microchip.com>
Date: Wed, 28 Feb 2024 06:59:59 +0000
From: <Dharma.B@...rochip.com>
To: <krzysztof.kozlowski@...aro.org>, <maarten.lankhorst@...ux.intel.com>,
<mripard@...nel.org>, <tzimmermann@...e.de>, <airlied@...il.com>,
<daniel@...ll.ch>, <robh+dt@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>,
<conor+dt@...nel.org>, <Nicolas.Ferre@...rochip.com>,
<alexandre.belloni@...tlin.com>, <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
Hi Krzysztof,
On 27/02/24 5:33 pm, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> 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...
>
There seems to be an issue with my git hooks, Thanks for letting me know
these errors, I will fix it.
> ...
>
>> 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.
Yes Indeed, I thought its working as expected as I had all required
property.
>
> 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.
Okay, then I think the driver also needs to be modified, currently the
driver parses the phandle and looks for these properties. Also the
corresponding dts files.
>
>
>> +
>> + 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?
I will update the flags as well in the next revision.
>
>> + 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
>
--
With Best Regards,
Dharma B.
Powered by blists - more mailing lists