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

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

[Dharma Balasubiramani]

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

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

[Dharma Balasubiramani]

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

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
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 <nicolas.ferre@microchip.com>
+  - Alexandre Belloni <alexandre.belloni@bootlin.com>
+  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
+
+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

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

[Dharma Balasubiramani]

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

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
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 <nicolas.ferre@microchip.com>
+  - Alexandre Belloni <alexandre.belloni@bootlin.com>
+  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
+
+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

[PATCH v2 3/3] dt-bindings: mfd: atmel,hlcdc: Convert to DT schema format

[Dharma Balasubiramani]

Convert the atmel,hlcdc binding to DT schema format.

Adjust the clock-names property to clarify that the LCD controller expects
one of these clocks (either sys_clk or lvds_pll_clk to be present but not
both) along with the slow_clk and periph_clk. This alignment with the actual
hardware requirements will enable accurate device tree configuration for
systems using the HLCDC IP.

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
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.
- Add details of lvds_pll addition in commit message.
- Ref endpoint and not endpoint-base.
- Fix coding style.

Note: Renaming hlcdc-display-controller, hlcdc-pwm to generic names throws
errors from the existing DTS files.
...
/home/dharma/Mainline/linux/arch/arm/boot/dts/microchip/at91sam9n12ek.dtb:
hlcdc@f8038000: 'hlcdc-display-controller' does not match any of the
regexes: 'pinctrl-[0-9]+'
---
 .../devicetree/bindings/mfd/atmel,hlcdc.yaml  | 105 ++++++++++++++++++
 .../devicetree/bindings/mfd/atmel-hlcdc.txt   |  56 ----------
 2 files changed, 105 insertions(+), 56 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
 delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt

diff --git a/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
new file mode 100644
index 000000000000..f624b60b76fb
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
@@ -0,0 +1,105 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/atmel,hlcdc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel's HLCD Controller
+
+maintainers:
+  - Nicolas Ferre <nicolas.ferre@microchip.com>
+  - Alexandre Belloni <alexandre.belloni@bootlin.com>
+  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
+
+description: |
+  The Atmel HLCDC (HLCD Controller) IP available on Atmel SoCs exposes two
+  subdevices
+    # a PWM chip:
+    # a Display Controller:
+
+properties:
+  compatible:
+    enum:
+      - atmel,at91sam9n12-hlcdc
+      - atmel,at91sam9x5-hlcdc
+      - atmel,sama5d2-hlcdc
+      - atmel,sama5d3-hlcdc
+      - atmel,sama5d4-hlcdc
+      - microchip,sam9x60-hlcdc
+      - microchip,sam9x75-xlcdc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 3
+
+  clock-names:
+    anyOf:
+      - items:
+          - enum:
+              - sys_clk
+              - lvds_pll_clk
+      - contains:
+          const: periph_clk
+      - contains:
+          const: slow_clk
+    maxItems: 3
+
+  hlcdc-display-controller:
+    $ref: /schemas/display/atmel/atmel,hlcdc-display-controller.yaml
+
+  hlcdc-pwm:
+    $ref: /schemas/pwm/atmel,hlcdc-pwm.yaml
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/at91.h>
+    #include <dt-bindings/dma/at91.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    lcd_controller: lcd-controller@f0030000 {
+      compatible = "atmel,sama5d3-hlcdc";
+      reg = <0xf0030000 0x2000>;
+      clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
+      clock-names = "periph_clk", "sys_clk", "slow_clk";
+      interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
+
+      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 {
+        compatible = "atmel,hlcdc-pwm";
+        pinctrl-names = "default";
+        pinctrl-0 = <&pinctrl_lcd_pwm>;
+        #pwm-cells = <3>;
+      };
+    };
diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
deleted file mode 100644
index 7de696eefaed..000000000000
--- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
+++ /dev/null
@@ -1,56 +0,0 @@
-Device-Tree bindings for Atmel's HLCDC (High LCD Controller) MFD driver
-
-Required properties:
- - compatible: value should be one of the following:
-   "atmel,at91sam9n12-hlcdc"
-   "atmel,at91sam9x5-hlcdc"
-   "atmel,sama5d2-hlcdc"
-   "atmel,sama5d3-hlcdc"
-   "atmel,sama5d4-hlcdc"
-   "microchip,sam9x60-hlcdc"
-   "microchip,sam9x75-xlcdc"
- - reg: base address and size of the HLCDC device registers.
- - clock-names: the name of the 3 clocks requested by the HLCDC device.
-   Should contain "periph_clk", "sys_clk" and "slow_clk".
- - clocks: should contain the 3 clocks requested by the HLCDC device.
- - interrupts: should contain the description of the HLCDC interrupt line
-
-The HLCDC IP exposes two subdevices:
- - a PWM chip: see ../pwm/atmel-hlcdc-pwm.txt
- - a Display Controller: see ../display/atmel/hlcdc-dc.txt
-
-Example:
-
-	hlcdc: hlcdc@f0030000 {
-		compatible = "atmel,sama5d3-hlcdc";
-		reg = <0xf0030000 0x2000>;
-		clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
-		clock-names = "periph_clk","sys_clk", "slow_clk";
-		interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
-
-		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>;
-		};
-	};
-- 
2.25.1

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 7904 bytes --]

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 <dharma.b@microchip.com>
> ---
> 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 <nicolas.ferre@microchip.com>
> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
> +  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
> +
> +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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4531 bytes --]

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 <dharma.b@microchip.com>
> ---
> 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 <nicolas.ferre@microchip.com>
> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
> +  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
> +
> +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 <conor.dooley@microchip.com>


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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 3/3] dt-bindings: mfd: atmel,hlcdc: Convert to DT schema format

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 7337 bytes --]

On Tue, Jan 16, 2024 at 05:08:00PM +0530, Dharma Balasubiramani wrote:
> Convert the atmel,hlcdc binding to DT schema format.
> 
> Adjust the clock-names property to clarify that the LCD controller expects
> one of these clocks (either sys_clk or lvds_pll_clk to be present but not
> both) along with the slow_clk and periph_clk. This alignment with the actual
> hardware requirements will enable accurate device tree configuration for
> systems using the HLCDC IP.
> 
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> ---
> 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.
> - Add details of lvds_pll addition in commit message.
> - Ref endpoint and not endpoint-base.
> - Fix coding style.
> 
> Note: Renaming hlcdc-display-controller, hlcdc-pwm to generic names throws
> errors from the existing DTS files.

I don't think that is important. If there is no code that depends on the
node names (and there is not in the mainline kernel, not sure about
anywhere else) the binding and the devicetree could easily adopt generic
node names.

> ...
> /home/dharma/Mainline/linux/arch/arm/boot/dts/microchip/at91sam9n12ek.dtb:
> hlcdc@f8038000: 'hlcdc-display-controller' does not match any of the
> regexes: 'pinctrl-[0-9]+'
> ---
>  .../devicetree/bindings/mfd/atmel,hlcdc.yaml  | 105 ++++++++++++++++++
>  .../devicetree/bindings/mfd/atmel-hlcdc.txt   |  56 ----------
>  2 files changed, 105 insertions(+), 56 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>  delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
> new file mode 100644
> index 000000000000..f624b60b76fb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/atmel,hlcdc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel's HLCD Controller
> +
> +maintainers:
> +  - Nicolas Ferre <nicolas.ferre@microchip.com>
> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
> +  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
> +
> +description: |
> +  The Atmel HLCDC (HLCD Controller) IP available on Atmel SoCs exposes two
> +  subdevices
> +    # a PWM chip:
> +    # a Display Controller:

The formatting here is a bit odd. I'd truncate this to
"subdevices: a PWM chip and a display controller." & drop the |.

> +properties:
> +  compatible:
> +    enum:
> +      - atmel,at91sam9n12-hlcdc
> +      - atmel,at91sam9x5-hlcdc
> +      - atmel,sama5d2-hlcdc
> +      - atmel,sama5d3-hlcdc
> +      - atmel,sama5d4-hlcdc
> +      - microchip,sam9x60-hlcdc
> +      - microchip,sam9x75-xlcdc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    anyOf:
> +      - items:
> +          - enum:
> +              - sys_clk
> +              - lvds_pll_clk
> +      - contains:
> +          const: periph_clk
> +      - contains:
> +          const: slow_clk
> +    maxItems: 3

Why not just:
  clock-names:
    items:
      - const: periph_clk
      - enum: [sys_clk, lvds_pll_clk]
      - const: slow_clk

Cheers,
Conor.

> +
> +  hlcdc-display-controller:
> +    $ref: /schemas/display/atmel/atmel,hlcdc-display-controller.yaml
> +
> +  hlcdc-pwm:
> +    $ref: /schemas/pwm/atmel,hlcdc-pwm.yaml
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/at91.h>
> +    #include <dt-bindings/dma/at91.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    lcd_controller: lcd-controller@f0030000 {
> +      compatible = "atmel,sama5d3-hlcdc";
> +      reg = <0xf0030000 0x2000>;
> +      clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
> +      clock-names = "periph_clk", "sys_clk", "slow_clk";
> +      interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
> +
> +      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 {
> +        compatible = "atmel,hlcdc-pwm";
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&pinctrl_lcd_pwm>;
> +        #pwm-cells = <3>;
> +      };
> +    };
> diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> deleted file mode 100644
> index 7de696eefaed..000000000000
> --- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> +++ /dev/null
> @@ -1,56 +0,0 @@
> -Device-Tree bindings for Atmel's HLCDC (High LCD Controller) MFD driver
> -
> -Required properties:
> - - compatible: value should be one of the following:
> -   "atmel,at91sam9n12-hlcdc"
> -   "atmel,at91sam9x5-hlcdc"
> -   "atmel,sama5d2-hlcdc"
> -   "atmel,sama5d3-hlcdc"
> -   "atmel,sama5d4-hlcdc"
> -   "microchip,sam9x60-hlcdc"
> -   "microchip,sam9x75-xlcdc"
> - - reg: base address and size of the HLCDC device registers.
> - - clock-names: the name of the 3 clocks requested by the HLCDC device.
> -   Should contain "periph_clk", "sys_clk" and "slow_clk".
> - - clocks: should contain the 3 clocks requested by the HLCDC device.
> - - interrupts: should contain the description of the HLCDC interrupt line
> -
> -The HLCDC IP exposes two subdevices:
> - - a PWM chip: see ../pwm/atmel-hlcdc-pwm.txt
> - - a Display Controller: see ../display/atmel/hlcdc-dc.txt
> -
> -Example:
> -
> -	hlcdc: hlcdc@f0030000 {
> -		compatible = "atmel,sama5d3-hlcdc";
> -		reg = <0xf0030000 0x2000>;
> -		clocks = <&lcdc_clk>, <&lcdck>, <&clk32k>;
> -		clock-names = "periph_clk","sys_clk", "slow_clk";
> -		interrupts = <36 IRQ_TYPE_LEVEL_HIGH 0>;
> -
> -		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>;
> -		};
> -	};
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Rob Herring]

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 <dharma.b@microchip.com>
> ---
> 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 <nicolas.ferre@microchip.com>
> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
> +  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
> +
> +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

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

[Alexandre Belloni]

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 <dharma.b@microchip.com>
> > ---
> > 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 <nicolas.ferre@microchip.com>
> > +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
> > +  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
> > +
> > +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 <conor.dooley@microchip.com>
> 
> 
> 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

Re: [PATCH v2 3/3] dt-bindings: mfd: atmel,hlcdc: Convert to DT schema format

[Krzysztof Kozlowski]

On 16/01/2024 12:38, Dharma Balasubiramani wrote:
> Convert the atmel,hlcdc binding to DT schema format.
> 
> Adjust the clock-names property to clarify that the LCD controller expects
> one of these clocks (either sys_clk or lvds_pll_clk to be present but not
> both) along with the slow_clk and periph_clk. This alignment with the actual
> hardware requirements will enable accurate device tree configuration for
> systems using the HLCDC IP.
> 
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
> ---
> 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.
> - Add details of lvds_pll addition in commit message.
> - Ref endpoint and not endpoint-base.
> - Fix coding style.
> 
> Note: Renaming hlcdc-display-controller, hlcdc-pwm to generic names throws
> errors from the existing DTS files.
> ...
> /home/dharma/Mainline/linux/arch/arm/boot/dts/microchip/at91sam9n12ek.dtb:
> hlcdc@f8038000: 'hlcdc-display-controller' does not match any of the
> regexes: 'pinctrl-[0-9]+'
> ---
>  .../devicetree/bindings/mfd/atmel,hlcdc.yaml  | 105 ++++++++++++++++++
>  .../devicetree/bindings/mfd/atmel-hlcdc.txt   |  56 ----------
>  2 files changed, 105 insertions(+), 56 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
>  delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
> new file mode 100644
> index 000000000000..f624b60b76fb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/atmel,hlcdc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel's HLCD Controller
> +
> +maintainers:
> +  - Nicolas Ferre <nicolas.ferre@microchip.com>
> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
> +  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
> +
> +description: |
> +  The Atmel HLCDC (HLCD Controller) IP available on Atmel SoCs exposes two
> +  subdevices
> +    # a PWM chip:
> +    # a Display Controller:

This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.

Thank you.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - atmel,at91sam9n12-hlcdc
> +      - atmel,at91sam9x5-hlcdc
> +      - atmel,sama5d2-hlcdc
> +      - atmel,sama5d3-hlcdc
> +      - atmel,sama5d4-hlcdc
> +      - microchip,sam9x60-hlcdc
> +      - microchip,sam9x75-xlcdc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    anyOf:
> +      - items:
> +          - enum:
> +              - sys_clk
> +              - lvds_pll_clk
> +      - contains:
> +          const: periph_clk
> +      - contains:
> +          const: slow_clk

NAK. You just ignored entire review.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

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 <dharma.b@microchip.com>
> ---
> 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

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

[Dharma.B]

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<dharma.b@microchip.com>
>> ---
>> 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<nicolas.ferre@microchip.com>
>> +  - Alexandre Belloni<alexandre.belloni@bootlin.com>
>> +  - Claudiu Beznea<claudiu.beznea@tuxon.dev>
>> +
>> +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
>>

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

[Dharma.B]

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 <dharma.b@microchip.com>
>> ---
>> 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 <nicolas.ferre@microchip.com>
>> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
>> +  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
>> +
>> +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

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

[Dharma.B]

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 <dharma.b@microchip.com>
>> ---
>> 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
> 

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

[Dharma.B]

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 <dharma.b@microchip.com>
>>> ---
>>> 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 <nicolas.ferre@microchip.com>
>>> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
>>> +  - Claudiu Beznea <claudiu.beznea@tuxon.dev>
>>> +
>>> +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 <conor.dooley@microchip.com>
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

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2333 bytes --]

On Wed, Jan 17, 2024 at 02:43:00AM +0000, Dharma.B@microchip.com 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 <dharma.b@microchip.com>
> >>> ---
> >>> 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Dharma.B]

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 <conor@kernel.org>
> Date:
> 17/01/24, 9:03 pm
> 
> To:
> Dharma.B@microchip.com
> CC:
> alexandre.belloni@bootlin.com, Conor.Dooley@microchip.com, 
> sam@ravnborg.org, bbrezillon@kernel.org, 
> maarten.lankhorst@linux.intel.com, mripard@kernel.org, 
> tzimmermann@suse.de, airlied@gmail.com, daniel@ffwll.ch, 
> robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, 
> conor+dt@kernel.org, Nicolas.Ferre@microchip.com, 
> claudiu.beznea@tuxon.dev, dri-devel@lists.freedesktop.org, 
> devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, 
> linux-kernel@vger.kernel.org, lee@kernel.org, thierry.reding@gmail.com, 
> u.kleine-koenig@pengutronix.de, linux-pwm@vger.kernel.org, 
> Linux4Microchip@microchip.com
> 
> 
> On Wed, Jan 17, 2024 at 02:43:00AM +0000,Dharma.B@microchip.com  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<dharma.b@microchip.com>
>>>>> ---
>>>>> 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.