| Message ID | 20240116113800.82529-4-dharma.b@microchip.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Convert Microchip's HLCDC Text based DT bindings to JSON schema | expand |
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 >
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
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>; - }; - };
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