2024-01-16 11:38:50

by Dharma Balasubiramani

[permalink] [raw]
Subject: [PATCH v2 0/3] Convert Microchip's HLCDC Text based DT bindings to JSON schema

Converted the text bindings to YAML and validated them individually using following commands

$ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/
$ make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/

changelogs are available in respective patches.

Dharma Balasubiramani (3):
dt-bindings: display: convert Atmel's HLCDC to DT schema
dt-bindings: atmel,hlcdc: convert pwm bindings to json-schema
dt-bindings: mfd: atmel,hlcdc: Convert to DT schema format

.../atmel/atmel,hlcdc-display-controller.yaml | 110 ++++++++++++++++++
.../bindings/display/atmel/hlcdc-dc.txt | 75 ------------
.../devicetree/bindings/mfd/atmel,hlcdc.yaml | 105 +++++++++++++++++
.../devicetree/bindings/mfd/atmel-hlcdc.txt | 56 ---------
.../bindings/pwm/atmel,hlcdc-pwm.yaml | 47 ++++++++
.../bindings/pwm/atmel-hlcdc-pwm.txt | 29 -----
6 files changed, 262 insertions(+), 160 deletions(-)
create mode 100644 Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
delete mode 100644 Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
create mode 100644 Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
create mode 100644 Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
delete mode 100644 Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt

--
2.25.1



2024-01-16 11:39:31

by Dharma Balasubiramani

[permalink] [raw]
Subject: [PATCH v2 1/3] dt-bindings: display: convert Atmel's HLCDC to DT schema

Convert the existing DT binding to DT schema of the Atmel's HLCDC display
controller.

Signed-off-by: Dharma Balasubiramani <[email protected]>
---
changelog
v1 -> v2
- Remove the explicit copyrights.
- Modify filename like compatible.
- Modify title (drop words like binding/driver).
- Modify description actually describing the hardware and not the driver.
- Remove pinctrl properties which aren't required.
- Ref endpoint and not endpoint-base.
- Drop redundant info about bus-width description and add ref to video-interfaces.
- Move 'additionalProperties' after 'Required'.
- Drop parent node and it's other sub-device node which are not related here.
- Add compatible to example 2 and add comments that bus-width is the diff between two examples.
---
.../atmel/atmel,hlcdc-display-controller.yaml | 110 ++++++++++++++++++
.../bindings/display/atmel/hlcdc-dc.txt | 75 ------------
2 files changed, 110 insertions(+), 75 deletions(-)
create mode 100644 Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
delete mode 100644 Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt

diff --git a/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
new file mode 100644
index 000000000000..f022c294cfbc
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
@@ -0,0 +1,110 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/atmel/atmel,hlcdc-display-controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel's High LCD Controller (HLCDC)
+
+maintainers:
+ - Nicolas Ferre <[email protected]>
+ - Alexandre Belloni <[email protected]>
+ - Claudiu Beznea <[email protected]>
+
+description: |
+ The LCD Controller (LCDC) consists of logic for transferring LCD image
+ data from an external display buffer to a TFT LCD panel. The LCDC has one
+ display input buffer per layer that fetches pixels through the single bus
+ host interface and a look-up table to allow palletized display
+ configurations.
+
+properties:
+ compatible:
+ const: atmel,hlcdc-display-controller
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ port@0:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description:
+ Output endpoint of the controller, connecting the LCD panel signals.
+
+ properties:
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ reg:
+ maxItems: 1
+
+ endpoint:
+ $ref: /schemas/graph.yaml#/$defs/endpoint
+ unevaluatedProperties: false
+ description:
+ Endpoint connecting the LCD panel signals.
+
+ properties:
+ bus-width:
+ description: Endpoint bus width.
+ $ref: /schemas/media/video-interfaces.yaml#
+ enum: [ 12, 16, 18, 24 ]
+
+required:
+ - '#address-cells'
+ - '#size-cells'
+ - compatible
+ - port@0
+
+additionalProperties: false
+
+examples:
+ - |
+ //Example 1
+
+ display-controller {
+ compatible = "atmel,hlcdc-display-controller";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb888>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ hlcdc_panel_output: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&panel_input>;
+ };
+ };
+ };
+
+ - |
+ //Example 2 With a video interface override to force rgb565, bus-width=16
+
+ display-controller {
+ compatible = "atmel,hlcdc-display-controller";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb565>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ hlcdc_panel_output2: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&panel_input>;
+ bus-width = <16>;
+ };
+ };
+ };
diff --git a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt b/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
deleted file mode 100644
index 923aea25344c..000000000000
--- a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
+++ /dev/null
@@ -1,75 +0,0 @@
-Device-Tree bindings for Atmel's HLCDC (High LCD Controller) DRM driver
-
-The Atmel HLCDC Display Controller is subdevice of the HLCDC MFD device.
-See ../../mfd/atmel-hlcdc.txt for more details.
-
-Required properties:
- - compatible: value should be "atmel,hlcdc-display-controller"
- - pinctrl-names: the pin control state names. Should contain "default".
- - pinctrl-0: should contain the default pinctrl states.
- - #address-cells: should be set to 1.
- - #size-cells: should be set to 0.
-
-Required children nodes:
- Children nodes are encoding available output ports and their connections
- to external devices using the OF graph representation (see ../graph.txt).
- At least one port node is required.
-
-Optional properties in grandchild nodes:
- Any endpoint grandchild node may specify a desired video interface
- according to ../../media/video-interfaces.txt, specifically
- - bus-width: recognized values are <12>, <16>, <18> and <24>, and
- override any output mode selection heuristic, forcing "rgb444",
- "rgb565", "rgb666" and "rgb888" respectively.
-
-Example:
-
- hlcdc: hlcdc@f0030000 {
- compatible = "atmel,sama5d3-hlcdc";
- reg = <0xf0030000 0x2000>;
- interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
- clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
- clock-names = "periph_clk","sys_clk", "slow_clk";
-
- hlcdc-display-controller {
- compatible = "atmel,hlcdc-display-controller";
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb888>;
- #address-cells = <1>;
- #size-cells = <0>;
-
- port@0 {
- #address-cells = <1>;
- #size-cells = <0>;
- reg = <0>;
-
- hlcdc_panel_output: endpoint@0 {
- reg = <0>;
- remote-endpoint = <&panel_input>;
- };
- };
- };
-
- hlcdc_pwm: hlcdc-pwm {
- compatible = "atmel,hlcdc-pwm";
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_lcd_pwm>;
- #pwm-cells = <3>;
- };
- };
-
-Example 2: With a video interface override to force rgb565; as above
-but with these changes/additions:
-
- &hlcdc {
- hlcdc-display-controller {
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb565>;
-
- port@0 {
- hlcdc_panel_output: endpoint@0 {
- bus-width = <16>;
- };
- };
- };
- };
--
2.25.1


2024-01-16 11:40:28

by Dharma Balasubiramani

[permalink] [raw]
Subject: [PATCH v2 2/3] dt-bindings: atmel,hlcdc: convert pwm bindings to json-schema

Convert device tree bindings for Atmel's HLCDC PWM controller to YAML
format.

Signed-off-by: Dharma Balasubiramani <[email protected]>
---
changelog
v1 -> v2
- Remove the explicit copyrights.
- Modify title (not include words like binding/driver).
- Modify description actually describing the hardware and not the driver.
- Remove pinctrl properties which aren't required.
- Drop parent node and it's other sub-device node which are not related here.
---
.../bindings/pwm/atmel,hlcdc-pwm.yaml | 47 +++++++++++++++++++
.../bindings/pwm/atmel-hlcdc-pwm.txt | 29 ------------
2 files changed, 47 insertions(+), 29 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
delete mode 100644 Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt

diff --git a/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml b/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
new file mode 100644
index 000000000000..751122309fa9
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/atmel,hlcdc-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel's HLCDC's PWM controller
+
+maintainers:
+ - Nicolas Ferre <[email protected]>
+ - Alexandre Belloni <[email protected]>
+ - Claudiu Beznea <[email protected]>
+
+description: |
+ The LCDC integrates a Pulse Width Modulation (PWM) Controller. This block
+ generates the LCD contrast control signal (LCD_PWM) that controls the
+ display's contrast by software. LCDC_PWM is an 8-bit PWM signal that can be
+ converted to an analog voltage with a simple passive filter. LCD display
+ panels have different backlight specifications in terms of minimum/maximum
+ values for PWM frequency. If the LCDC PWM frequency range does not match the
+ LCD display panel, it is possible to use the standalone PWM Controller to
+ drive the backlight.
+
+properties:
+ compatible:
+ const: atmel,hlcdc-pwm
+
+ "#pwm-cells":
+ const: 3
+ description: |
+ This PWM chip uses the default 3 cells bindings defined in pwm.yaml in
+ this directory.
+
+required:
+ - compatible
+ - "#pwm-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ pwm: pwm {
+ compatible = "atmel,hlcdc-pwm";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_lcd_pwm>;
+ #pwm-cells = <3>;
+ };
diff --git a/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt b/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
deleted file mode 100644
index afa501bf7f94..000000000000
--- a/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
+++ /dev/null
@@ -1,29 +0,0 @@
-Device-Tree bindings for Atmel's HLCDC (High-end LCD Controller) PWM driver
-
-The Atmel HLCDC PWM is subdevice of the HLCDC MFD device.
-See ../mfd/atmel-hlcdc.txt for more details.
-
-Required properties:
- - compatible: value should be one of the following:
- "atmel,hlcdc-pwm"
- - pinctr-names: the pin control state names. Should contain "default".
- - pinctrl-0: should contain the pinctrl states described by pinctrl
- default.
- - #pwm-cells: should be set to 3. This PWM chip use the default 3 cells
- bindings defined in pwm.yaml in this directory.
-
-Example:
-
- hlcdc: hlcdc@f0030000 {
- compatible = "atmel,sama5d3-hlcdc";
- reg = <0xf0030000 0x2000>;
- clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
- clock-names = "periph_clk","sys_clk", "slow_clk";
-
- hlcdc_pwm: hlcdc-pwm {
- compatible = "atmel,hlcdc-pwm";
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_lcd_pwm>;
- #pwm-cells = <3>;
- };
- };
--
2.25.1


2024-01-16 17:56:00

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: display: convert Atmel's HLCDC to DT schema

Yo,

On Tue, Jan 16, 2024 at 05:07:58PM +0530, Dharma Balasubiramani wrote:
> Convert the existing DT binding to DT schema of the Atmel's HLCDC display
> controller.
>
> Signed-off-by: Dharma Balasubiramani <[email protected]>
> ---
> changelog
> v1 -> v2
> - Remove the explicit copyrights.
> - Modify filename like compatible.
> - Modify title (drop words like binding/driver).
> - Modify description actually describing the hardware and not the driver.
> - Remove pinctrl properties which aren't required.
> - Ref endpoint and not endpoint-base.
> - Drop redundant info about bus-width description and add ref to video-interfaces.
> - Move 'additionalProperties' after 'Required'.
> - Drop parent node and it's other sub-device node which are not related here.
> - Add compatible to example 2 and add comments that bus-width is the diff between two examples.
> ---
> .../atmel/atmel,hlcdc-display-controller.yaml | 110 ++++++++++++++++++
> .../bindings/display/atmel/hlcdc-dc.txt | 75 ------------
> 2 files changed, 110 insertions(+), 75 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
> delete mode 100644 Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
>
> diff --git a/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
> new file mode 100644
> index 000000000000..f022c294cfbc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/atmel/atmel,hlcdc-display-controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel's High LCD Controller (HLCDC)
> +
> +maintainers:
> + - Nicolas Ferre <[email protected]>
> + - Alexandre Belloni <[email protected]>
> + - Claudiu Beznea <[email protected]>
> +
> +description: |

This | is not needed as you have no formatting to preserve.

> + The LCD Controller (LCDC) consists of logic for transferring LCD image
> + data from an external display buffer to a TFT LCD panel. The LCDC has one
> + display input buffer per layer that fetches pixels through the single bus
> + host interface and a look-up table to allow palletized display
> + configurations.
> +
> +properties:
> + compatible:
> + const: atmel,hlcdc-display-controller
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + port@0:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + unevaluatedProperties: false
> + description:
> + Output endpoint of the controller, connecting the LCD panel signals.
> +
> + properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + reg:
> + maxItems: 1
> +
> + endpoint:
> + $ref: /schemas/graph.yaml#/$defs/endpoint

$ref: /schemas/media/video-interfaces.yaml#

to match approximately all other endpoints?

> + unevaluatedProperties: false
> + description:
> + Endpoint connecting the LCD panel signals.
> +
> + properties:
> + bus-width:
> + description: Endpoint bus width.
> + $ref: /schemas/media/video-interfaces.yaml#

and then bus-width's type is already defined for you, no?

> + enum: [ 12, 16, 18, 24 ]
> +
> +required:
> + - '#address-cells'
> + - '#size-cells'
> + - compatible
> + - port@0
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + //Example 1
> +
> + display-controller {
> + compatible = "atmel,hlcdc-display-controller";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb888>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> +
> + hlcdc_panel_output: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&panel_input>;
> + };
> + };
> + };
> +
> + - |
> + //Example 2 With a video interface override to force rgb565, bus-width=16
> +
> + display-controller {
> + compatible = "atmel,hlcdc-display-controller";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb565>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;

Should be a newline here before the child node.

Cheers,
Conor.

> + hlcdc_panel_output2: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&panel_input>;
> + bus-width = <16>;
> + };
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt b/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
> deleted file mode 100644
> index 923aea25344c..000000000000
> --- a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
> +++ /dev/null
> @@ -1,75 +0,0 @@
> -Device-Tree bindings for Atmel's HLCDC (High LCD Controller) DRM driver
> -
> -The Atmel HLCDC Display Controller is subdevice of the HLCDC MFD device.
> -See ../../mfd/atmel-hlcdc.txt for more details.
> -
> -Required properties:
> - - compatible: value should be "atmel,hlcdc-display-controller"
> - - pinctrl-names: the pin control state names. Should contain "default".
> - - pinctrl-0: should contain the default pinctrl states.
> - - #address-cells: should be set to 1.
> - - #size-cells: should be set to 0.
> -
> -Required children nodes:
> - Children nodes are encoding available output ports and their connections
> - to external devices using the OF graph representation (see ../graph.txt).
> - At least one port node is required.
> -
> -Optional properties in grandchild nodes:
> - Any endpoint grandchild node may specify a desired video interface
> - according to ../../media/video-interfaces.txt, specifically
> - - bus-width: recognized values are <12>, <16>, <18> and <24>, and
> - override any output mode selection heuristic, forcing "rgb444",
> - "rgb565", "rgb666" and "rgb888" respectively.
> -
> -Example:
> -
> - hlcdc: hlcdc@f0030000 {
> - compatible = "atmel,sama5d3-hlcdc";
> - reg = <0xf0030000 0x2000>;
> - interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
> - clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
> - clock-names = "periph_clk","sys_clk", "slow_clk";
> -
> - hlcdc-display-controller {
> - compatible = "atmel,hlcdc-display-controller";
> - pinctrl-names = "default";
> - pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb888>;
> - #address-cells = <1>;
> - #size-cells = <0>;
> -
> - port@0 {
> - #address-cells = <1>;
> - #size-cells = <0>;
> - reg = <0>;
> -
> - hlcdc_panel_output: endpoint@0 {
> - reg = <0>;
> - remote-endpoint = <&panel_input>;
> - };
> - };
> - };
> -
> - hlcdc_pwm: hlcdc-pwm {
> - compatible = "atmel,hlcdc-pwm";
> - pinctrl-names = "default";
> - pinctrl-0 = <&pinctrl_lcd_pwm>;
> - #pwm-cells = <3>;
> - };
> - };
> -
> -Example 2: With a video interface override to force rgb565; as above
> -but with these changes/additions:
> -
> - &hlcdc {
> - hlcdc-display-controller {
> - pinctrl-names = "default";
> - pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb565>;
> -
> - port@0 {
> - hlcdc_panel_output: endpoint@0 {
> - bus-width = <16>;
> - };
> - };
> - };
> - };
> --
> 2.25.1
>


Attachments:
(No filename) (7.72 kB)
signature.asc (235.00 B)
Download all attachments

2024-01-16 18:03:36

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] dt-bindings: atmel,hlcdc: convert pwm bindings to json-schema

On Tue, Jan 16, 2024 at 05:07:59PM +0530, Dharma Balasubiramani wrote:
> Convert device tree bindings for Atmel's HLCDC PWM controller to YAML
> format.
>
> Signed-off-by: Dharma Balasubiramani <[email protected]>
> ---
> changelog
> v1 -> v2
> - Remove the explicit copyrights.
> - Modify title (not include words like binding/driver).
> - Modify description actually describing the hardware and not the driver.
> - Remove pinctrl properties which aren't required.
> - Drop parent node and it's other sub-device node which are not related here.
> ---
> .../bindings/pwm/atmel,hlcdc-pwm.yaml | 47 +++++++++++++++++++
> .../bindings/pwm/atmel-hlcdc-pwm.txt | 29 ------------
> 2 files changed, 47 insertions(+), 29 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
> delete mode 100644 Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
>
> diff --git a/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml b/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
> new file mode 100644
> index 000000000000..751122309fa9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

The original file has no license, but was originally written by a
free-electrons employee, so the relicensing here is fine.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pwm/atmel,hlcdc-pwm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel's HLCDC's PWM controller
> +
> +maintainers:
> + - Nicolas Ferre <[email protected]>
> + - Alexandre Belloni <[email protected]>
> + - Claudiu Beznea <[email protected]>
> +
> +description: |

Again, the | is not needed here.

> + The LCDC integrates a Pulse Width Modulation (PWM) Controller. This block
> + generates the LCD contrast control signal (LCD_PWM) that controls the
> + display's contrast by software. LCDC_PWM is an 8-bit PWM signal that can be
> + converted to an analog voltage with a simple passive filter. LCD display
> + panels have different backlight specifications in terms of minimum/maximum
> + values for PWM frequency. If the LCDC PWM frequency range does not match the
> + LCD display panel, it is possible to use the standalone PWM Controller to
> + drive the backlight.
> +
> +properties:
> + compatible:
> + const: atmel,hlcdc-pwm
> +
> + "#pwm-cells":
> + const: 3
> + description: |
> + This PWM chip uses the default 3 cells bindings defined in pwm.yaml in
> + this directory.

I would delete this description tbh.

> +
> +required:
> + - compatible
> + - "#pwm-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + pwm: pwm {
> + compatible = "atmel,hlcdc-pwm";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_lcd_pwm>;
> + #pwm-cells = <3>;
> + };

The label here is not used and can be dropped. Otherwise,
Reviewed-by: Conor Dooley <[email protected]>


Cheers,
Conor.

> diff --git a/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt b/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
> deleted file mode 100644
> index afa501bf7f94..000000000000
> --- a/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
> +++ /dev/null
> @@ -1,29 +0,0 @@
> -Device-Tree bindings for Atmel's HLCDC (High-end LCD Controller) PWM driver
> -
> -The Atmel HLCDC PWM is subdevice of the HLCDC MFD device.
> -See ../mfd/atmel-hlcdc.txt for more details.
> -
> -Required properties:
> - - compatible: value should be one of the following:
> - "atmel,hlcdc-pwm"
> - - pinctr-names: the pin control state names. Should contain "default".
> - - pinctrl-0: should contain the pinctrl states described by pinctrl
> - default.
> - - #pwm-cells: should be set to 3. This PWM chip use the default 3 cells
> - bindings defined in pwm.yaml in this directory.
> -
> -Example:
> -
> - hlcdc: hlcdc@f0030000 {
> - compatible = "atmel,sama5d3-hlcdc";
> - reg = <0xf0030000 0x2000>;
> - clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
> - clock-names = "periph_clk","sys_clk", "slow_clk";
> -
> - hlcdc_pwm: hlcdc-pwm {
> - compatible = "atmel,hlcdc-pwm";
> - pinctrl-names = "default";
> - pinctrl-0 = <&pinctrl_lcd_pwm>;
> - #pwm-cells = <3>;
> - };
> - };
> --
> 2.25.1
>


Attachments:
(No filename) (4.42 kB)
signature.asc (235.00 B)
Download all attachments

2024-01-16 20:35:44

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: display: convert Atmel's HLCDC to DT schema

On Tue, Jan 16, 2024 at 05:07:58PM +0530, Dharma Balasubiramani wrote:
> Convert the existing DT binding to DT schema of the Atmel's HLCDC display
> controller.
>
> Signed-off-by: Dharma Balasubiramani <[email protected]>
> ---
> changelog
> v1 -> v2
> - Remove the explicit copyrights.
> - Modify filename like compatible.
> - Modify title (drop words like binding/driver).
> - Modify description actually describing the hardware and not the driver.
> - Remove pinctrl properties which aren't required.
> - Ref endpoint and not endpoint-base.
> - Drop redundant info about bus-width description and add ref to video-interfaces.
> - Move 'additionalProperties' after 'Required'.
> - Drop parent node and it's other sub-device node which are not related here.
> - Add compatible to example 2 and add comments that bus-width is the diff between two examples.
> ---
> .../atmel/atmel,hlcdc-display-controller.yaml | 110 ++++++++++++++++++
> .../bindings/display/atmel/hlcdc-dc.txt | 75 ------------
> 2 files changed, 110 insertions(+), 75 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
> delete mode 100644 Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
>
> diff --git a/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
> new file mode 100644
> index 000000000000..f022c294cfbc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
> @@ -0,0 +1,110 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/atmel/atmel,hlcdc-display-controller.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel's High LCD Controller (HLCDC)
> +
> +maintainers:
> + - Nicolas Ferre <[email protected]>
> + - Alexandre Belloni <[email protected]>
> + - Claudiu Beznea <[email protected]>
> +
> +description: |
> + The LCD Controller (LCDC) consists of logic for transferring LCD image
> + data from an external display buffer to a TFT LCD panel. The LCDC has one
> + display input buffer per layer that fetches pixels through the single bus
> + host interface and a look-up table to allow palletized display
> + configurations.
> +
> +properties:
> + compatible:
> + const: atmel,hlcdc-display-controller
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + port@0:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + unevaluatedProperties: false
> + description:
> + Output endpoint of the controller, connecting the LCD panel signals.
> +
> + properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + reg:
> + maxItems: 1
> +
> + endpoint:
> + $ref: /schemas/graph.yaml#/$defs/endpoint
> + unevaluatedProperties: false
> + description:
> + Endpoint connecting the LCD panel signals.
> +
> + properties:
> + bus-width:
> + description: Endpoint bus width.
> + $ref: /schemas/media/video-interfaces.yaml#
> + enum: [ 12, 16, 18, 24 ]
> +
> +required:
> + - '#address-cells'
> + - '#size-cells'
> + - compatible
> + - port@0
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + //Example 1
> +
> + display-controller {
> + compatible = "atmel,hlcdc-display-controller";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb888>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> +
> + hlcdc_panel_output: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&panel_input>;
> + };
> + };
> + };
> +
> + - |
> + //Example 2 With a video interface override to force rgb565, bus-width=16
> +
> + display-controller {
> + compatible = "atmel,hlcdc-display-controller";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb565>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> + hlcdc_panel_output2: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&panel_input>;
> + bus-width = <16>;
> + };
> + };
> + };

Just 1 extra property doesn't justify 2 examples.

In any case, drop the partial examples and just have 1 complete example
in the MFD binding schema.

Rob

2024-01-16 22:09:29

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] dt-bindings: atmel,hlcdc: convert pwm bindings to json-schema

On 16/01/2024 18:03:19+0000, Conor Dooley wrote:
> On Tue, Jan 16, 2024 at 05:07:59PM +0530, Dharma Balasubiramani wrote:
> > Convert device tree bindings for Atmel's HLCDC PWM controller to YAML
> > format.
> >
> > Signed-off-by: Dharma Balasubiramani <[email protected]>
> > ---
> > changelog
> > v1 -> v2
> > - Remove the explicit copyrights.
> > - Modify title (not include words like binding/driver).
> > - Modify description actually describing the hardware and not the driver.
> > - Remove pinctrl properties which aren't required.
> > - Drop parent node and it's other sub-device node which are not related here.
> > ---
> > .../bindings/pwm/atmel,hlcdc-pwm.yaml | 47 +++++++++++++++++++
> > .../bindings/pwm/atmel-hlcdc-pwm.txt | 29 ------------
> > 2 files changed, 47 insertions(+), 29 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
> > delete mode 100644 Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml b/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
> > new file mode 100644
> > index 000000000000..751122309fa9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
> > @@ -0,0 +1,47 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>
> The original file has no license, but was originally written by a
> free-electrons employee, so the relicensing here is fine.
>

I confirm relicensing is fine, even assigning the copyright to
Microchip (note that Bootlin is legally the same entity as
free-electrons)

> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pwm/atmel,hlcdc-pwm.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Atmel's HLCDC's PWM controller
> > +
> > +maintainers:
> > + - Nicolas Ferre <[email protected]>
> > + - Alexandre Belloni <[email protected]>
> > + - Claudiu Beznea <[email protected]>
> > +
> > +description: |
>
> Again, the | is not needed here.
>
> > + The LCDC integrates a Pulse Width Modulation (PWM) Controller. This block
> > + generates the LCD contrast control signal (LCD_PWM) that controls the
> > + display's contrast by software. LCDC_PWM is an 8-bit PWM signal that can be
> > + converted to an analog voltage with a simple passive filter. LCD display
> > + panels have different backlight specifications in terms of minimum/maximum
> > + values for PWM frequency. If the LCDC PWM frequency range does not match the
> > + LCD display panel, it is possible to use the standalone PWM Controller to
> > + drive the backlight.
> > +
> > +properties:
> > + compatible:
> > + const: atmel,hlcdc-pwm
> > +
> > + "#pwm-cells":
> > + const: 3
> > + description: |
> > + This PWM chip uses the default 3 cells bindings defined in pwm.yaml in
> > + this directory.
>
> I would delete this description tbh.
>
> > +
> > +required:
> > + - compatible
> > + - "#pwm-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + pwm: pwm {
> > + compatible = "atmel,hlcdc-pwm";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_lcd_pwm>;
> > + #pwm-cells = <3>;
> > + };
>
> The label here is not used and can be dropped. Otherwise,
> Reviewed-by: Conor Dooley <[email protected]>
>
>
> Cheers,
> Conor.
>
> > diff --git a/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt b/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
> > deleted file mode 100644
> > index afa501bf7f94..000000000000
> > --- a/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
> > +++ /dev/null
> > @@ -1,29 +0,0 @@
> > -Device-Tree bindings for Atmel's HLCDC (High-end LCD Controller) PWM driver
> > -
> > -The Atmel HLCDC PWM is subdevice of the HLCDC MFD device.
> > -See ../mfd/atmel-hlcdc.txt for more details.
> > -
> > -Required properties:
> > - - compatible: value should be one of the following:
> > - "atmel,hlcdc-pwm"
> > - - pinctr-names: the pin control state names. Should contain "default".
> > - - pinctrl-0: should contain the pinctrl states described by pinctrl
> > - default.
> > - - #pwm-cells: should be set to 3. This PWM chip use the default 3 cells
> > - bindings defined in pwm.yaml in this directory.
> > -
> > -Example:
> > -
> > - hlcdc: hlcdc@f0030000 {
> > - compatible = "atmel,sama5d3-hlcdc";
> > - reg = <0xf0030000 0x2000>;
> > - clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
> > - clock-names = "periph_clk","sys_clk", "slow_clk";
> > -
> > - hlcdc_pwm: hlcdc-pwm {
> > - compatible = "atmel,hlcdc-pwm";
> > - pinctrl-names = "default";
> > - pinctrl-0 = <&pinctrl_lcd_pwm>;
> > - #pwm-cells = <3>;
> > - };
> > - };
> > --
> > 2.25.1
> >



--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2024-01-16 22:17:32

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: display: convert Atmel's HLCDC to DT schema

On 16/01/2024 12:37, Dharma Balasubiramani wrote:
> Convert the existing DT binding to DT schema of the Atmel's HLCDC display
> controller.
>
> Signed-off-by: Dharma Balasubiramani <[email protected]>
> ---
> changelog
> v1 -> v2
> - Remove the explicit copyrights.
> - Modify filename like compatible.
> - Modify title (drop words like binding/driver).
> - Modify description actually describing the hardware and not the driver.
> - Remove pinctrl properties which aren't required.
> - Ref endpoint and not endpoint-base.
> - Drop redundant info about bus-width description and add ref to video-interfaces.
> - Move 'additionalProperties' after 'Required'.
> - Drop parent node and it's other sub-device node which are not related here.
> - Add compatible to example 2 and add comments that bus-width is the diff between two examples.
> ---

Please respond to review comments and acknowledge you implement each of
them. I don't think you did here everything I pointed out. The next
patch for sure misses things.

Best regards,
Krzysztof


2024-01-17 02:36:18

by Dharma Balasubiramani

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: display: convert Atmel's HLCDC to DT schema

On 16/01/24 11:25 pm, Conor Dooley wrote:
> Yo,
>
> On Tue, Jan 16, 2024 at 05:07:58PM +0530, Dharma Balasubiramani wrote:
>> Convert the existing DT binding to DT schema of the Atmel's HLCDC display
>> controller.
>>
>> Signed-off-by: Dharma Balasubiramani<[email protected]>
>> ---
>> changelog
>> v1 -> v2
>> - Remove the explicit copyrights.
>> - Modify filename like compatible.
>> - Modify title (drop words like binding/driver).
>> - Modify description actually describing the hardware and not the driver.
>> - Remove pinctrl properties which aren't required.
>> - Ref endpoint and not endpoint-base.
>> - Drop redundant info about bus-width description and add ref to video-interfaces.
>> - Move 'additionalProperties' after 'Required'.
>> - Drop parent node and it's other sub-device node which are not related here.
>> - Add compatible to example 2 and add comments that bus-width is the diff between two examples.
>> ---
>> .../atmel/atmel,hlcdc-display-controller.yaml | 110 ++++++++++++++++++
>> .../bindings/display/atmel/hlcdc-dc.txt | 75 ------------
>> 2 files changed, 110 insertions(+), 75 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
>> delete mode 100644 Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
>> new file mode 100644
>> index 000000000000..f022c294cfbc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
>> @@ -0,0 +1,110 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id:http://devicetree.org/schemas/display/atmel/atmel,hlcdc-display-controller.yaml#
>> +$schema:http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Atmel's High LCD Controller (HLCDC)
>> +
>> +maintainers:
>> + - Nicolas Ferre<[email protected]>
>> + - Alexandre Belloni<[email protected]>
>> + - Claudiu Beznea<[email protected]>
>> +
>> +description: |
> This | is not needed as you have no formatting to preserve.
Sure, I will drop this '|'.
>
>> + The LCD Controller (LCDC) consists of logic for transferring LCD image
>> + data from an external display buffer to a TFT LCD panel. The LCDC has one
>> + display input buffer per layer that fetches pixels through the single bus
>> + host interface and a look-up table to allow palletized display
>> + configurations.
>> +
>> +properties:
>> + compatible:
>> + const: atmel,hlcdc-display-controller
>> +
>> + '#address-cells':
>> + const: 1
>> +
>> + '#size-cells':
>> + const: 0
>> +
>> + port@0:
>> + $ref: /schemas/graph.yaml#/$defs/port-base
>> + unevaluatedProperties: false
>> + description:
>> + Output endpoint of the controller, connecting the LCD panel signals.
>> +
>> + properties:
>> + '#address-cells':
>> + const: 1
>> +
>> + '#size-cells':
>> + const: 0
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + endpoint:
>> + $ref: /schemas/graph.yaml#/$defs/endpoint
> $ref: /schemas/media/video-interfaces.yaml#
I will replace this.
>
> to match approximately all other endpoints?
I'm not sure; some of the referenced devices, like this one, were
pointing to 'endpoint.' I will go with 'video-interfaces.'
>
>> + unevaluatedProperties: false
>> + description:
>> + Endpoint connecting the LCD panel signals.
>> +
>> + properties:
>> + bus-width:
>> + description: Endpoint bus width.
>> + $ref: /schemas/media/video-interfaces.yaml#
> and then bus-width's type is already defined for you, no?
I will remove this $ref here.
>
>> + enum: [ 12, 16, 18, 24 ]
>> +
>> +required:
>> + - '#address-cells'
>> + - '#size-cells'
>> + - compatible
>> + - port@0
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + //Example 1
>> +
>> + display-controller {
>> + compatible = "atmel,hlcdc-display-controller";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb888>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0>;
>> +
>> + hlcdc_panel_output: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint = <&panel_input>;
>> + };
>> + };
>> + };
>> +
>> + - |
>> + //Example 2 With a video interface override to force rgb565, bus-width=16
>> +
>> + display-controller {
>> + compatible = "atmel,hlcdc-display-controller";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb565>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0>;
> Should be a newline here before the child node.
Sure, I will take care of this in v3.

--
With Best Regards,
Dharma B.
>
> Cheers,
> Conor.
>
>> + hlcdc_panel_output2: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint = <&panel_input>;
>> + bus-width = <16>;
>> + };
>> + };
>> + };
>> diff --git a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt b/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
>> deleted file mode 100644
>> index 923aea25344c..000000000000
>> --- a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
>> +++ /dev/null
>> @@ -1,75 +0,0 @@
>> -Device-Tree bindings for Atmel's HLCDC (High LCD Controller) DRM driver
>> -
>> -The Atmel HLCDC Display Controller is subdevice of the HLCDC MFD device.
>> -See ../../mfd/atmel-hlcdc.txt for more details.
>> -
>> -Required properties:
>> - - compatible: value should be "atmel,hlcdc-display-controller"
>> - - pinctrl-names: the pin control state names. Should contain "default".
>> - - pinctrl-0: should contain the default pinctrl states.
>> - - #address-cells: should be set to 1.
>> - - #size-cells: should be set to 0.
>> -
>> -Required children nodes:
>> - Children nodes are encoding available output ports and their connections
>> - to external devices using the OF graph representation (see ../graph.txt).
>> - At least one port node is required.
>> -
>> -Optional properties in grandchild nodes:
>> - Any endpoint grandchild node may specify a desired video interface
>> - according to ../../media/video-interfaces.txt, specifically
>> - - bus-width: recognized values are <12>, <16>, <18> and <24>, and
>> - override any output mode selection heuristic, forcing "rgb444",
>> - "rgb565", "rgb666" and "rgb888" respectively.
>> -
>> -Example:
>> -
>> - hlcdc: hlcdc@f0030000 {
>> - compatible = "atmel,sama5d3-hlcdc";
>> - reg = <0xf0030000 0x2000>;
>> - interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
>> - clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
>> - clock-names = "periph_clk","sys_clk", "slow_clk";
>> -
>> - hlcdc-display-controller {
>> - compatible = "atmel,hlcdc-display-controller";
>> - pinctrl-names = "default";
>> - pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb888>;
>> - #address-cells = <1>;
>> - #size-cells = <0>;
>> -
>> - port@0 {
>> - #address-cells = <1>;
>> - #size-cells = <0>;
>> - reg = <0>;
>> -
>> - hlcdc_panel_output: endpoint@0 {
>> - reg = <0>;
>> - remote-endpoint = <&panel_input>;
>> - };
>> - };
>> - };
>> -
>> - hlcdc_pwm: hlcdc-pwm {
>> - compatible = "atmel,hlcdc-pwm";
>> - pinctrl-names = "default";
>> - pinctrl-0 = <&pinctrl_lcd_pwm>;
>> - #pwm-cells = <3>;
>> - };
>> - };
>> -
>> -Example 2: With a video interface override to force rgb565; as above
>> -but with these changes/additions:
>> -
>> - &hlcdc {
>> - hlcdc-display-controller {
>> - pinctrl-names = "default";
>> - pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb565>;
>> -
>> - port@0 {
>> - hlcdc_panel_output: endpoint@0 {
>> - bus-width = <16>;
>> - };
>> - };
>> - };
>> - };
>> --
>> 2.25.1
>>

2024-01-17 02:37:11

by Dharma Balasubiramani

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: display: convert Atmel's HLCDC to DT schema

On 17/01/24 1:19 am, Rob Herring wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Tue, Jan 16, 2024 at 05:07:58PM +0530, Dharma Balasubiramani wrote:
>> Convert the existing DT binding to DT schema of the Atmel's HLCDC display
>> controller.
>>
>> Signed-off-by: Dharma Balasubiramani <[email protected]>
>> ---
>> changelog
>> v1 -> v2
>> - Remove the explicit copyrights.
>> - Modify filename like compatible.
>> - Modify title (drop words like binding/driver).
>> - Modify description actually describing the hardware and not the driver.
>> - Remove pinctrl properties which aren't required.
>> - Ref endpoint and not endpoint-base.
>> - Drop redundant info about bus-width description and add ref to video-interfaces.
>> - Move 'additionalProperties' after 'Required'.
>> - Drop parent node and it's other sub-device node which are not related here.
>> - Add compatible to example 2 and add comments that bus-width is the diff between two examples.
>> ---
>> .../atmel/atmel,hlcdc-display-controller.yaml | 110 ++++++++++++++++++
>> .../bindings/display/atmel/hlcdc-dc.txt | 75 ------------
>> 2 files changed, 110 insertions(+), 75 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
>> delete mode 100644 Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
>> new file mode 100644
>> index 000000000000..f022c294cfbc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-display-controller.yaml
>> @@ -0,0 +1,110 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/atmel/atmel,hlcdc-display-controller.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Atmel's High LCD Controller (HLCDC)
>> +
>> +maintainers:
>> + - Nicolas Ferre <[email protected]>
>> + - Alexandre Belloni <[email protected]>
>> + - Claudiu Beznea <[email protected]>
>> +
>> +description: |
>> + The LCD Controller (LCDC) consists of logic for transferring LCD image
>> + data from an external display buffer to a TFT LCD panel. The LCDC has one
>> + display input buffer per layer that fetches pixels through the single bus
>> + host interface and a look-up table to allow palletized display
>> + configurations.
>> +
>> +properties:
>> + compatible:
>> + const: atmel,hlcdc-display-controller
>> +
>> + '#address-cells':
>> + const: 1
>> +
>> + '#size-cells':
>> + const: 0
>> +
>> + port@0:
>> + $ref: /schemas/graph.yaml#/$defs/port-base
>> + unevaluatedProperties: false
>> + description:
>> + Output endpoint of the controller, connecting the LCD panel signals.
>> +
>> + properties:
>> + '#address-cells':
>> + const: 1
>> +
>> + '#size-cells':
>> + const: 0
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + endpoint:
>> + $ref: /schemas/graph.yaml#/$defs/endpoint
>> + unevaluatedProperties: false
>> + description:
>> + Endpoint connecting the LCD panel signals.
>> +
>> + properties:
>> + bus-width:
>> + description: Endpoint bus width.
>> + $ref: /schemas/media/video-interfaces.yaml#
>> + enum: [ 12, 16, 18, 24 ]
>> +
>> +required:
>> + - '#address-cells'
>> + - '#size-cells'
>> + - compatible
>> + - port@0
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + //Example 1
>> +
>> + display-controller {
>> + compatible = "atmel,hlcdc-display-controller";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb888>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0>;
>> +
>> + hlcdc_panel_output: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint = <&panel_input>;
>> + };
>> + };
>> + };
>> +
>> + - |
>> + //Example 2 With a video interface override to force rgb565, bus-width=16
>> +
>> + display-controller {
>> + compatible = "atmel,hlcdc-display-controller";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb565>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0>;
>> + hlcdc_panel_output2: endpoint@0 {
>> + reg = <0>;
>> + remote-endpoint = <&panel_input>;
>> + bus-width = <16>;
>> + };
>> + };
>> + };
>
> Just 1 extra property doesn't justify 2 examples.
>
> In any case, drop the partial examples and just have 1 complete example
> in the MFD binding schema.
Sure, I will drop the Example 2 in v3.

--
With Best Regards,
Dharma B.
>
> Rob



2024-01-17 02:41:36

by Dharma Balasubiramani

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] dt-bindings: display: convert Atmel's HLCDC to DT schema

On 17/01/24 2:15 am, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 16/01/2024 12:37, Dharma Balasubiramani wrote:
>> Convert the existing DT binding to DT schema of the Atmel's HLCDC display
>> controller.
>>
>> Signed-off-by: Dharma Balasubiramani <[email protected]>
>> ---
>> changelog
>> v1 -> v2
>> - Remove the explicit copyrights.
>> - Modify filename like compatible.
>> - Modify title (drop words like binding/driver).
>> - Modify description actually describing the hardware and not the driver.
>> - Remove pinctrl properties which aren't required.
>> - Ref endpoint and not endpoint-base.
>> - Drop redundant info about bus-width description and add ref to video-interfaces.
>> - Move 'additionalProperties' after 'Required'.
>> - Drop parent node and it's other sub-device node which are not related here.
>> - Add compatible to example 2 and add comments that bus-width is the diff between two examples.
>> ---
>
> Please respond to review comments and acknowledge you implement each of
> them. I don't think you did here everything I pointed out. The next
> patch for sure misses things.
My apologies, I will ensure that I address all the review comments in v3.

--
With Best Regards,
Dharma B.
>
> Best regards,
> Krzysztof
>


2024-01-17 02:43:44

by Dharma Balasubiramani

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] dt-bindings: atmel,hlcdc: convert pwm bindings to json-schema

On 17/01/24 1:40 am, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 16/01/2024 18:03:19+0000, Conor Dooley wrote:
>> On Tue, Jan 16, 2024 at 05:07:59PM +0530, Dharma Balasubiramani wrote:
>>> Convert device tree bindings for Atmel's HLCDC PWM controller to YAML
>>> format.
>>>
>>> Signed-off-by: Dharma Balasubiramani <[email protected]>
>>> ---
>>> changelog
>>> v1 -> v2
>>> - Remove the explicit copyrights.
>>> - Modify title (not include words like binding/driver).
>>> - Modify description actually describing the hardware and not the driver.
>>> - Remove pinctrl properties which aren't required.
>>> - Drop parent node and it's other sub-device node which are not related here.
>>> ---
>>> .../bindings/pwm/atmel,hlcdc-pwm.yaml | 47 +++++++++++++++++++
>>> .../bindings/pwm/atmel-hlcdc-pwm.txt | 29 ------------
>>> 2 files changed, 47 insertions(+), 29 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
>>> delete mode 100644 Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml b/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
>>> new file mode 100644
>>> index 000000000000..751122309fa9
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
>>> @@ -0,0 +1,47 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>
>> The original file has no license, but was originally written by a
>> free-electrons employee, so the relicensing here is fine.
>>
>
> I confirm relicensing is fine, even assigning the copyright to
> Microchip (note that Bootlin is legally the same entity as
> free-electrons)
Thanks Conor and Alexandre.
I will add the copyrights back in v3.
>
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/pwm/atmel,hlcdc-pwm.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Atmel's HLCDC's PWM controller
>>> +
>>> +maintainers:
>>> + - Nicolas Ferre <[email protected]>
>>> + - Alexandre Belloni <[email protected]>
>>> + - Claudiu Beznea <[email protected]>
>>> +
>>> +description: |
>>
>> Again, the | is not needed here.
Sure, I will drop it.
>>
>>> + The LCDC integrates a Pulse Width Modulation (PWM) Controller. This block
>>> + generates the LCD contrast control signal (LCD_PWM) that controls the
>>> + display's contrast by software. LCDC_PWM is an 8-bit PWM signal that can be
>>> + converted to an analog voltage with a simple passive filter. LCD display
>>> + panels have different backlight specifications in terms of minimum/maximum
>>> + values for PWM frequency. If the LCDC PWM frequency range does not match the
>>> + LCD display panel, it is possible to use the standalone PWM Controller to
>>> + drive the backlight.
>>> +
>>> +properties:
>>> + compatible:
>>> + const: atmel,hlcdc-pwm
>>> +
>>> + "#pwm-cells":
>>> + const: 3
>>> + description: |
>>> + This PWM chip uses the default 3 cells bindings defined in pwm.yaml in
>>> + this directory.
>>
>> I would delete this description tbh.
Sure, I will remove it.
>>
>>> +
>>> +required:
>>> + - compatible
>>> + - "#pwm-cells"
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + pwm: pwm {
>>> + compatible = "atmel,hlcdc-pwm";
>>> + pinctrl-names = "default";
>>> + pinctrl-0 = <&pinctrl_lcd_pwm>;
>>> + #pwm-cells = <3>;
>>> + };
>>
>> The label here is not used and can be dropped. Otherwise,
>> Reviewed-by: Conor Dooley <[email protected]>
Sure, I will remove the label.

--
With Best Regards,
Dharma B.
>>
>>
>> Cheers,
>> Conor.
>>
>>> diff --git a/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt b/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
>>> deleted file mode 100644
>>> index afa501bf7f94..000000000000
>>> --- a/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
>>> +++ /dev/null
>>> @@ -1,29 +0,0 @@
>>> -Device-Tree bindings for Atmel's HLCDC (High-end LCD Controller) PWM driver
>>> -
>>> -The Atmel HLCDC PWM is subdevice of the HLCDC MFD device.
>>> -See ../mfd/atmel-hlcdc.txt for more details.
>>> -
>>> -Required properties:
>>> - - compatible: value should be one of the following:
>>> - "atmel,hlcdc-pwm"
>>> - - pinctr-names: the pin control state names. Should contain "default".
>>> - - pinctrl-0: should contain the pinctrl states described by pinctrl
>>> - default.
>>> - - #pwm-cells: should be set to 3. This PWM chip use the default 3 cells
>>> - bindings defined in pwm.yaml in this directory.
>>> -
>>> -Example:
>>> -
>>> - hlcdc: hlcdc@f0030000 {
>>> - compatible = "atmel,sama5d3-hlcdc";
>>> - reg = <0xf0030000 0x2000>;
>>> - clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
>>> - clock-names = "periph_clk","sys_clk", "slow_clk";
>>> -
>>> - hlcdc_pwm: hlcdc-pwm {
>>> - compatible = "atmel,hlcdc-pwm";
>>> - pinctrl-names = "default";
>>> - pinctrl-0 = <&pinctrl_lcd_pwm>;
>>> - #pwm-cells = <3>;
>>> - };
>>> - };
>>> --
>>> 2.25.1
>>>
>
>
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com



2024-01-17 15:33:54

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] dt-bindings: atmel,hlcdc: convert pwm bindings to json-schema

On Wed, Jan 17, 2024 at 02:43:00AM +0000, [email protected] wrote:
> On 17/01/24 1:40 am, Alexandre Belloni wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > On 16/01/2024 18:03:19+0000, Conor Dooley wrote:
> >> On Tue, Jan 16, 2024 at 05:07:59PM +0530, Dharma Balasubiramani wrote:
> >>> Convert device tree bindings for Atmel's HLCDC PWM controller to YAML
> >>> format.
> >>>
> >>> Signed-off-by: Dharma Balasubiramani <[email protected]>
> >>> ---
> >>> changelog
> >>> v1 -> v2
> >>> - Remove the explicit copyrights.
> >>> - Modify title (not include words like binding/driver).
> >>> - Modify description actually describing the hardware and not the driver.
> >>> - Remove pinctrl properties which aren't required.
> >>> - Drop parent node and it's other sub-device node which are not related here.
> >>> ---
> >>> .../bindings/pwm/atmel,hlcdc-pwm.yaml | 47 +++++++++++++++++++
> >>> .../bindings/pwm/atmel-hlcdc-pwm.txt | 29 ------------
> >>> 2 files changed, 47 insertions(+), 29 deletions(-)
> >>> create mode 100644 Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
> >>> delete mode 100644 Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml b/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
> >>> new file mode 100644
> >>> index 000000000000..751122309fa9
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
> >>> @@ -0,0 +1,47 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>
> >> The original file has no license, but was originally written by a
> >> free-electrons employee, so the relicensing here is fine.
> >>
> >
> > I confirm relicensing is fine, even assigning the copyright to
> > Microchip (note that Bootlin is legally the same entity as
> > free-electrons)
> Thanks Conor and Alexandre.
> I will add the copyrights back in v3.

Just to note, in case you misunderstood my original comment here:
What I said had nothing to do with adding a Microchip copyright assignment
to the file, but rather about the fact that your patch relicenses the
binding from GPL v2 to GPL v2 OR BSD 2 Clause.


Attachments:
(No filename) (2.28 kB)
signature.asc (235.00 B)
Download all attachments

2024-01-18 09:09:58

by Dharma Balasubiramani

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] dt-bindings: atmel,hlcdc: convert pwm bindings to json-schema

On 17/01/24 9:03 pm, Conor Dooley wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
>
> ForwardedMessage.eml
>
> Subject:
> Re: [PATCH v2 2/3] dt-bindings: atmel,hlcdc: convert pwm bindings to
> json-schema
> From:
> Conor Dooley <[email protected]>
> Date:
> 17/01/24, 9:03 pm
>
> To:
> [email protected]
> CC:
> [email protected], [email protected],
> [email protected], [email protected],
> [email protected], [email protected],
> [email protected], [email protected], [email protected],
> [email protected], [email protected],
> [email protected], [email protected],
> [email protected], [email protected],
> [email protected], [email protected],
> [email protected], [email protected], [email protected],
> [email protected], [email protected],
> [email protected]
>
>
> On Wed, Jan 17, 2024 at 02:43:00AM +0000,[email protected] wrote:
>> On 17/01/24 1:40 am, Alexandre Belloni wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 16/01/2024 18:03:19+0000, Conor Dooley wrote:
>>>> On Tue, Jan 16, 2024 at 05:07:59PM +0530, Dharma Balasubiramani wrote:
>>>>> Convert device tree bindings for Atmel's HLCDC PWM controller to YAML
>>>>> format.
>>>>>
>>>>> Signed-off-by: Dharma Balasubiramani<[email protected]>
>>>>> ---
>>>>> changelog
>>>>> v1 -> v2
>>>>> - Remove the explicit copyrights.
>>>>> - Modify title (not include words like binding/driver).
>>>>> - Modify description actually describing the hardware and not the driver.
>>>>> - Remove pinctrl properties which aren't required.
>>>>> - Drop parent node and it's other sub-device node which are not related here.
>>>>> ---
>>>>> .../bindings/pwm/atmel,hlcdc-pwm.yaml | 47 +++++++++++++++++++
>>>>> .../bindings/pwm/atmel-hlcdc-pwm.txt | 29 ------------
>>>>> 2 files changed, 47 insertions(+), 29 deletions(-)
>>>>> create mode 100644 Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
>>>>> delete mode 100644 Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml b/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..751122309fa9
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/pwm/atmel,hlcdc-pwm.yaml
>>>>> @@ -0,0 +1,47 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> The original file has no license, but was originally written by a
>>>> free-electrons employee, so the relicensing here is fine.
>>>>
>>> I confirm relicensing is fine, even assigning the copyright to
>>> Microchip (note that Bootlin is legally the same entity as
>>> free-electrons)
>> Thanks Conor and Alexandre.
>> I will add the copyrights back in v3.
> Just to note, in case you misunderstood my original comment here:
> What I said had nothing to do with adding a Microchip copyright assignment
> to the file, but rather about the fact that your patch relicenses the
> binding from GPL v2 to GPL v2 OR BSD 2 Clause.
I appreciate the clarification; my initial understanding was not
accurate. Thanks
>

--
With Best Regards,
Dharma B.