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@xxxxxxxxxxxxx> >> --- >> 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@xxxxxxxxxxxxx> >> + - Alexandre Belloni<alexandre.belloni@xxxxxxxxxxx> >> + - Claudiu Beznea<claudiu.beznea@xxxxxxxxx> >> + >> +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 >>
