| Message ID | 20240116113800.82529-1-dharma.b@microchip.com |
|---|---|
| Headers | show |
| Series | Convert Microchip's HLCDC Text based DT bindings to JSON schema | expand |
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.
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.