| Message ID | 20240110102535.246177-2-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 |
Hi Krzysztof, On 10/01/24 4:09 pm, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 10/01/2024 11:25, 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> >> --- >> .../display/atmel/atmel,hlcdc-dc.yaml | 133 ++++++++++++++++++ >> .../bindings/display/atmel/hlcdc-dc.txt | 75 ---------- >> 2 files changed, 133 insertions(+), 75 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-dc.yaml >> delete mode 100644 Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt >> >> diff --git a/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-dc.yaml b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-dc.yaml >> new file mode 100644 >> index 000000000000..49ef28646c48 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-dc.yaml >> @@ -0,0 +1,133 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +# Copyright (C) 2024 Microchip Technology, Inc. and its subsidiaries > > What about original copyrights from TXT file? Although conversion is > quite independent, I could imagine some lawyer would call it a > derivative work of original TXT. > > If you decide to add explicit copyrights (which anyway I do not > understand why), then please make it signed off by some of your lawyers > to be sure that you really claim that, in respect of other people > copyrights. I omitted it in v2, but given the approval from Conor and Alexandre to include the copyrights, I will reintroduce it in v3. > >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/display/atmel/atmel,hlcdc-dc.yaml# > > Filename like compatible I modified it in v2. > >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Atmel's HLCDC (High LCD Controller) DRM driver > > Driver as Linux driver? Not suitable for bindings, so please drop. Done. > >> + >> +maintainers: >> + - Nicolas Ferre <nicolas.ferre@microchip.com> >> + - Alexandre Belloni <alexandre.belloni@bootlin.com> >> + - Claudiu Beznea <claudiu.beznea@tuxon.dev> >> + >> +description: | >> + Device-Tree bindings for Atmel's HLCDC DRM driver. The Atmel HLCDC Display > > Drop entire first sentence and instead describe hardware. Done. > >> + Controller is a subdevice of the HLCDC MFD device. >> + # See ../../mfd/atmel,hlcdc.yaml for more details. > > Full paths please. Considering that the MFD's YAML comes after this patch, I have decided to remove it here. > >> + >> +properties: >> + compatible: >> + const: atmel,hlcdc-display-controller >> + >> + pinctrl-names: >> + const: default >> + >> + pinctrl-0: true > > Why do you need these two? Are they really required? No, I removed it in v2. > >> + >> + '#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-base > > Hm, why do you reference endpoint-base? This looks oddly different than > all other bindings for such devices, so please explain why. My apologies, it should be '$ref: /schemas/media/video-interfaces.yaml#' I will correct this in v3. > >> + unevaluatedProperties: false >> + description: >> + Endpoint connecting the LCD panel signals. >> + >> + properties: >> + bus-width: >> + description: | >> + Any endpoint grandchild node may specify a desired video interface according to >> + ../../media/video-interfaces.yaml, specifically "bus-width" whose recognized > > Drop redundant information. Don't you miss some $ref? Sure, I will drop the description in v3. > > >> + values are <12>, <16>, <18> and <24>, and override any output mode selection >> + heuristic, forcing "rgb444","rgb565", "rgb666" and "rgb888" respectively. >> + enum: [ 12, 16, 18, 24 ] >> + >> +additionalProperties: false > > This goes after required: Done. > >> + >> +required: >> + - '#address-cells' >> + - '#size-cells' >> + - compatible >> + - pinctrl-names >> + - pinctrl-0 >> + - port@0 >> + >> +examples: >> + - | >> + #include <dt-bindings/clock/at91.h> >> + #include <dt-bindings/dma/at91.h> >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + //Example 1 > > Drop Sure. > >> + hlcdc: hlcdc@f0030000 { > > Node names should be generic. See also an explanation and list of > examples (not exhaustive) in DT specification: > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > >> + 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"; > > This part does not look related... If this is part of other device, > usually it is enough to have just one complete example. Sure, I will have one complete example in mfd binding and drop the other example here. > > Also, fix coding style - space after , Sure. > >> + >> + 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"; > > How is this related? Not related here, thanks, removed it in v2. > >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_lcd_pwm>; >> + #pwm-cells = <3>; >> + }; >> + }; >> + - | >> + //Example 2 With a video interface override to force rgb565 >> + hlcdc-display-controller { >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb565>; > > And how is this? Where is the compatible? Maybe just drop second > example, what are the differences? bus-width is the only property that is different between the examples, I will drop the example 2 in v3. > > Are you sure your Microchip folks reviewed it before? I acknowledge my oversight; I should have sought their review beforehand. I appreciate your understanding.
Hi Rob, On 10/01/24 5:27 pm, Rob Herring wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Wed, 10 Jan 2024 15:55:33 +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> >> --- >> .../display/atmel/atmel,hlcdc-dc.yaml | 133 ++++++++++++++++++ >> .../bindings/display/atmel/hlcdc-dc.txt | 75 ---------- >> 2 files changed, 133 insertions(+), 75 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-dc.yaml >> delete mode 100644 Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt >> > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > I conducted the tests after applying all three of them, so I didn't notice these errors. I have addressed this in v2. Thank you.
diff --git a/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-dc.yaml b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-dc.yaml new file mode 100644 index 000000000000..49ef28646c48 --- /dev/null +++ b/Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-dc.yaml @@ -0,0 +1,133 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright (C) 2024 Microchip Technology, Inc. and its subsidiaries +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/atmel/atmel,hlcdc-dc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Atmel's HLCDC (High LCD Controller) DRM driver + +maintainers: + - Nicolas Ferre <nicolas.ferre@microchip.com> + - Alexandre Belloni <alexandre.belloni@bootlin.com> + - Claudiu Beznea <claudiu.beznea@tuxon.dev> + +description: | + Device-Tree bindings for Atmel's HLCDC DRM driver. The Atmel HLCDC Display + Controller is a subdevice of the HLCDC MFD device. + # See ../../mfd/atmel,hlcdc.yaml for more details. + +properties: + compatible: + const: atmel,hlcdc-display-controller + + pinctrl-names: + const: default + + pinctrl-0: true + + '#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-base + unevaluatedProperties: false + description: + Endpoint connecting the LCD panel signals. + + properties: + bus-width: + description: | + Any endpoint grandchild node may specify a desired video interface according to + ../../media/video-interfaces.yaml, specifically "bus-width" whose recognized + values are <12>, <16>, <18> and <24>, and override any output mode selection + heuristic, forcing "rgb444","rgb565", "rgb666" and "rgb888" respectively. + enum: [ 12, 16, 18, 24 ] + +additionalProperties: false + +required: + - '#address-cells' + - '#size-cells' + - compatible + - pinctrl-names + - pinctrl-0 + - port@0 + +examples: + - | + #include <dt-bindings/clock/at91.h> + #include <dt-bindings/dma/at91.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + //Example 1 + 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 + 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>; - }; - }; - }; - };
Convert the existing DT binding to DT schema of the Atmel's HLCDC display controller. Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com> --- .../display/atmel/atmel,hlcdc-dc.yaml | 133 ++++++++++++++++++ .../bindings/display/atmel/hlcdc-dc.txt | 75 ---------- 2 files changed, 133 insertions(+), 75 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/atmel/atmel,hlcdc-dc.yaml delete mode 100644 Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt