2024-02-23 09:16:23

by Dharma Balasubiramani

[permalink] [raw]
Subject: [PATCH] dt-bindings: display: atmel,lcdc: convert to dtschema

Convert the atmel,lcdc bindings to DT schema.
Changes during conversion: add missing clocks and clock-names properties.

Signed-off-by: Dharma Balasubiramani <[email protected]>
---
.../devicetree/bindings/display/atmel,lcdc.txt | 87 --------------
.../devicetree/bindings/display/atmel,lcdc.yaml | 133 +++++++++++++++++++++
2 files changed, 133 insertions(+), 87 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/atmel,lcdc.txt b/Documentation/devicetree/bindings/display/atmel,lcdc.txt
deleted file mode 100644
index b5e355ada2fa..000000000000
--- a/Documentation/devicetree/bindings/display/atmel,lcdc.txt
+++ /dev/null
@@ -1,87 +0,0 @@
-Atmel LCDC Framebuffer
------------------------------------------------------
-
-Required properties:
-- compatible :
- "atmel,at91sam9261-lcdc" ,
- "atmel,at91sam9263-lcdc" ,
- "atmel,at91sam9g10-lcdc" ,
- "atmel,at91sam9g45-lcdc" ,
- "atmel,at91sam9g45es-lcdc" ,
- "atmel,at91sam9rl-lcdc" ,
-- reg : Should contain 1 register ranges(address and length).
- Can contain an additional register range(address and length)
- for fixed framebuffer memory. Useful for dedicated memories.
-- interrupts : framebuffer controller interrupt
-- display: a phandle pointing to the display node
-
-Required nodes:
-- display: a display node is required to initialize the lcd panel
- This should be in the board dts.
-- default-mode: a videomode within the display with timing parameters
- as specified below.
-
-Optional properties:
-- lcd-supply: Regulator for LCD supply voltage.
-
-Example:
-
- fb0: fb@00500000 {
- compatible = "atmel,at91sam9g45-lcdc";
- reg = <0x00500000 0x1000>;
- interrupts = <23 3 0>;
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_fb>;
- display = <&display0>;
- #address-cells = <1>;
- #size-cells = <1>;
-
- };
-
-Example for fixed framebuffer memory:
-
- fb0: fb@00500000 {
- compatible = "atmel,at91sam9263-lcdc";
- reg = <0x00700000 0x1000 0x70000000 0x200000>;
- [...]
- };
-
-Atmel LCDC Display
------------------------------------------------------
-Required properties (as per of_videomode_helper):
-
- - atmel,dmacon: dma controller configuration
- - atmel,lcdcon2: lcd controller configuration
- - atmel,guard-time: lcd guard time (Delay in frame periods)
- - bits-per-pixel: lcd panel bit-depth.
-
-Optional properties (as per of_videomode_helper):
- - atmel,lcdcon-backlight: enable backlight
- - atmel,lcdcon-backlight-inverted: invert backlight PWM polarity
- - atmel,lcd-wiring-mode: lcd wiring mode "RGB" or "BRG"
- - atmel,power-control-gpio: gpio to power on or off the LCD (as many as needed)
-
-Example:
- display0: display {
- bits-per-pixel = <32>;
- atmel,lcdcon-backlight;
- atmel,dmacon = <0x1>;
- atmel,lcdcon2 = <0x80008002>;
- atmel,guard-time = <9>;
- atmel,lcd-wiring-mode = <1>;
-
- display-timings {
- native-mode = <&timing0>;
- timing0: timing0 {
- clock-frequency = <9000000>;
- hactive = <480>;
- vactive = <272>;
- hback-porch = <1>;
- hfront-porch = <1>;
- vback-porch = <40>;
- vfront-porch = <1>;
- hsync-len = <45>;
- vsync-len = <1>;
- };
- };
- };
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 <[email protected]>
+ - Dharma Balasubiramani <[email protected]>
+
+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.
+
+ 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@500000 {
+ compatible = "atmel,at91sam9g45-lcdc";
+ reg = <0x00500000 0x1000>;
+ interrupts = <23 3 0>;
+ 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>;
+ };
+
+ display {
+ bits-per-pixel = <32>;
+ atmel,lcdcon-backlight;
+ atmel,dmacon = <0x1>;
+ atmel,lcdcon2 = <0x80008002>;
+ atmel,guard-time = <9>;
+ atmel,lcd-wiring-mode = <1>;
+
+ display-timings {
+ native-mode = <&timing0>;
+ timing0: timing0 {
+ clock-frequency = <9000000>;
+ hactive = <480>;
+ vactive = <272>;
+ hback-porch = <1>;
+ hfront-porch = <1>;
+ vback-porch = <40>;
+ vfront-porch = <1>;
+ hsync-len = <45>;
+ vsync-len = <1>;
+ };
+ };
+ };

---
base-commit: ffd2cb6b718e189e7e2d5d0c19c25611f92e061a
change-id: 20240223-lcdc-fb-b8d2e2f3c914

Best regards,
--
Dharma Balasubiramani <[email protected]>



2024-02-27 12:13:49

by Krzysztof Kozlowski

[permalink] [raw]
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 <[email protected]>
> ---
> .../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 <[email protected]>
> + - Dharma Balasubiramani <[email protected]>
> +
> +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@500000 {
> + 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


2024-02-28 07:03:22

by Dharma Balasubiramani

[permalink] [raw]
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 <[email protected]>
>> ---
>> .../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 <[email protected]>
>> + - Dharma Balasubiramani <[email protected]>
>> +
>> +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@500000 {
>> + 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.

2024-02-28 07:13:21

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: display: atmel,lcdc: convert to dtschema

On 28/02/2024 07:59, [email protected] wrote:
>
>>
>> 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.

Driver does not have to be modified in my proposal. You would still have
phandle.


Best regards,
Krzysztof


2024-02-28 10:20:10

by Dharma Balasubiramani

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: display: atmel,lcdc: convert to dtschema

On 28/02/24 12:43 pm, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 28/02/2024 07:59, [email protected] wrote:
>>
>>>
>>> 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.
>
> Driver does not have to be modified in my proposal. You would still have
> phandle.

If I understand correctly, I could define the dt bindings as below

display:
$ref: /schemas/types.yaml#/definitions/phandle
description: A phandle pointing to the display node.

panel:
$ref: panel/panel-common.yaml#
properties:

I will add those properties under panel node and modify the example as below

fb@500000 {
compatible = "atmel,at91sam9g45-lcdc";
reg = <0x00500000 0x1000>;
interrupts = <23 IRQ_TYPE_LEVEL_HIGH 0>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_fb>;
clocks = <&pmc PMC_TYPE_PERIPHERAL 23>, <&pmc PMC_TYPE_PERIPHERAL
23>;
clock-names = "hclk", "lcdc_clk";
display = <&display>;

display: panel {
bits-per-pixel = <32>;
atmel,lcdcon-backlight;
atmel,dmacon = <0x1>;
atmel,lcdcon2 = <0x80008002>;
atmel,guard-time = <9>;
atmel,lcd-wiring-mode = <1>;

display-timings {
native-mode = <&timing0>;

timing0: timing0 {
clock-frequency = <9000000>;
hactive = <480>;
vactive = <272>;
hback-porch = <1>;
hfront-porch = <1>;
vback-porch = <40>;
vfront-porch = <1>;
hsync-len = <45>;
vsync-len = <1>;
};
};
};
};

This structure is now in sync with the existing DTS files.

>
>
> Best regards,
> Krzysztof
>

--
With Best Regards,
Dharma B.

2024-02-28 10:36:10

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: display: atmel,lcdc: convert to dtschema

On 28/02/2024 11:18, [email protected] wrote:
> On 28/02/24 12:43 pm, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 28/02/2024 07:59, [email protected] wrote:
>>>
>>>>
>>>> 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.
>>
>> Driver does not have to be modified in my proposal. You would still have
>> phandle.
>
> If I understand correctly, I could define the dt bindings as below
>
> display:
> $ref: /schemas/types.yaml#/definitions/phandle
> description: A phandle pointing to the display node.
>
> panel:
> $ref: panel/panel-common.yaml#
> properties:
>

So these are standard panel bindings? Then the node should live outside
of lcdc. If current driver needs to poke inside panel and panel could be
anything, then probably you need peripheral-props-like approach. :/

Best regards,
Krzysztof


2024-02-29 06:27:06

by Dharma Balasubiramani

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: display: atmel,lcdc: convert to dtschema

On 28/02/24 3:53 pm, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 28/02/2024 11:18, [email protected] wrote:
>> On 28/02/24 12:43 pm, Krzysztof Kozlowski wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 28/02/2024 07:59, [email protected] wrote:
>>>>
>>>>>
>>>>> 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.
>>>
>>> Driver does not have to be modified in my proposal. You would still have
>>> phandle.
>>
>> If I understand correctly, I could define the dt bindings as below
>>
>> display:
>> $ref: /schemas/types.yaml#/definitions/phandle
>> description: A phandle pointing to the display node.
>>
>> panel:
>> $ref: panel/panel-common.yaml#
>> properties:
>>
>
> So these are standard panel bindings? Then the node should live outside
> of lcdc. If current driver needs to poke inside panel and panel could be
> anything, then probably you need peripheral-props-like approach. :/

Thank you so much, so can I use something like this

display:
$ref: /schemas/types.yaml#/definitions/phandle
description: A phandle pointing to the display node.

patternProperties:
"^panel":
type: object
properties:
atmel,dmacon:
atmel,lcdcon2:
:
:
required:
- atmel,dmacon
- atmel,lcdcon2
- atmel,guard-time
- bits-per-pixel

additionalProperties: false

I tested it by removing the required property in the panel node and it
flagged it as an issue.

>
> Best regards,
> Krzysztof
>

--
With Best Regards,
Dharma B.

2024-03-01 18:10:23

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: display: atmel,lcdc: convert to dtschema

On Thu, Feb 29, 2024 at 06:25:56AM +0000, [email protected] wrote:
> On 28/02/24 3:53 pm, Krzysztof Kozlowski wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > On 28/02/2024 11:18, [email protected] wrote:
> >> On 28/02/24 12:43 pm, Krzysztof Kozlowski wrote:
> >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >>>
> >>> On 28/02/2024 07:59, [email protected] wrote:
> >>>>
> >>>>>
> >>>>> 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.
> >>>
> >>> Driver does not have to be modified in my proposal. You would still have
> >>> phandle.
> >>
> >> If I understand correctly, I could define the dt bindings as below
> >>
> >> display:
> >> $ref: /schemas/types.yaml#/definitions/phandle
> >> description: A phandle pointing to the display node.
> >>
> >> panel:
> >> $ref: panel/panel-common.yaml#
> >> properties:
> >>
> >
> > So these are standard panel bindings? Then the node should live outside
> > of lcdc. If current driver needs to poke inside panel and panel could be
> > anything, then probably you need peripheral-props-like approach. :/
>
> Thank you so much, so can I use something like this
>
> display:
> $ref: /schemas/types.yaml#/definitions/phandle
> description: A phandle pointing to the display node.
>
> patternProperties:
> "^panel":

Just 'panel' and not a pattern.

However, that's not what the original binding had. It was a separate
node. If you want to preserve that, then you'll need a separate
schema file and a special 'select'. Something like:

select:
anyOf:
- required: [ atmel,dmacon ]
- required: [ atmel,lcdcon2 ]
- required: [ atmel,guard-time ]

Up to you and at91 maintainers if you want to have to update your dts
files or not.

Rob

2024-03-04 04:15:27

by Dharma Balasubiramani

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: display: atmel,lcdc: convert to dtschema

On 01/03/24 11:40 pm, Rob Herring wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Thu, Feb 29, 2024 at 06:25:56AM +0000, [email protected] wrote:
>> On 28/02/24 3:53 pm, Krzysztof Kozlowski wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 28/02/2024 11:18, [email protected] wrote:
>>>> On 28/02/24 12:43 pm, Krzysztof Kozlowski wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>
>>>>> On 28/02/2024 07:59, [email protected] wrote:
>>>>>>
>>>>>>>
>>>>>>> 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.
>>>>>
>>>>> Driver does not have to be modified in my proposal. You would still have
>>>>> phandle.
>>>>
>>>> If I understand correctly, I could define the dt bindings as below
>>>>
>>>> display:
>>>> $ref: /schemas/types.yaml#/definitions/phandle
>>>> description: A phandle pointing to the display node.
>>>>
>>>> panel:
>>>> $ref: panel/panel-common.yaml#
>>>> properties:
>>>>
>>>
>>> So these are standard panel bindings? Then the node should live outside
>>> of lcdc. If current driver needs to poke inside panel and panel could be
>>> anything, then probably you need peripheral-props-like approach. :/
>>
>> Thank you so much, so can I use something like this
>>
>> display:
>> $ref: /schemas/types.yaml#/definitions/phandle
>> description: A phandle pointing to the display node.
>>
>> patternProperties:
>> "^panel":
>
> Just 'panel' and not a pattern.
>
> However, that's not what the original binding had. It was a separate
> node. If you want to preserve that, then you'll need a separate
> schema file and a special 'select'. Something like:
>
> select:
> anyOf:
> - required: [ atmel,dmacon ]
> - required: [ atmel,lcdcon2 ]
> - required: [ atmel,guard-time ]
>
> Up to you and at91 maintainers if you want to have to update your dts
> files or not.
>
> Rob

Thank you so much Rob, I will introduce a new binding for lcdc-display
with a special select as you suggested.

I will send a v2 with the following changes
- Run checkpatch and remove whitespace errors.
- Add the standard interrupt flags.
- Split the binding into two namely lcdc.yaml and lcdc-display.yaml.
for your kind perusal.

--
With Best Regards,
Dharma B.