On 19/12/2019 10:38, Maxime Ripard wrote: > Hi, > > On Thu, Dec 19, 2019 at 10:23:17AM +0200, Jyri Sarha wrote: >> Add dt-schema yaml bindig for J721E DSS, J721E version TI Keystone >> Display SubSystem. >> >> Version history: >> >> v2: no change >> >> v3: - reg-names: "wp" -> "wb" >> - Add ports node >> - Add includes to dts example >> - reindent dts example >> >> v4: - Add descriptions to reg, clocks, and interrups properties >> - Remove minItems when its value is the same as maxItems value >> >> Signed-off-by: Jyri Sarha <jsarha@xxxxxx> >> --- >> .../bindings/display/ti/ti,j721e-dss.yaml | 209 ++++++++++++++++++ >> 1 file changed, 209 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/display/ti/ti,j721e-dss.yaml >> >> diff --git a/Documentation/devicetree/bindings/display/ti/ti,j721e-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,j721e-dss.yaml >> new file mode 100644 >> index 000000000000..cd68c4294f9a >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/display/ti/ti,j721e-dss.yaml >> @@ -0,0 +1,209 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +# Copyright 2019 Texas Instruments Incorporated >> +%YAML 1.2 >> +--- >> +$id: "http://devicetree.org/schemas/display/ti/ti,j721e-dss.yaml#" >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >> + >> +title: Texas Instruments J721E Display Subsystem >> + >> +maintainers: >> + - Jyri Sarha <jsarha@xxxxxx> >> + - Tomi Valkeinen <tomi.valkeinen@xxxxxx> >> + >> +description: | >> + The J721E TI Keystone Display SubSystem with four output ports and >> + four video planes. There is two full video planes and two "lite >> + planes" without scaling support. The video ports can be connected to >> + the SoC's DPI pins or to integrated display bridges on the SoC. >> + >> +properties: >> + compatible: >> + const: ti,j721e-dss >> + >> + reg: >> + maxItems: 17 >> + description: | >> + Addresses to each DSS memory region described in the SoC's TRM. >> + The reg-names refer to memory regions as follows: >> + reg-names: Region Name in TRM: Description: >> + common_m DSS0_DISPC_0_COMMON_M DSS Master common register area >> + common_s0 DSS0_DISPC_0_COMMON_SO DSS Shared common register area 0 >> + common_s1 DSS0_DISPC_0_COMMON_S1 DSS Shared common register area 1 >> + common_s2 DSS0_DISPC_0_COMMON_S2 DSS Shared common register area 2 >> + vidl1 DSS0_VIDL1 VIDL1 light video plane 1 >> + vidl2 DSS0_VIDL2 VIDL2 light video plane 2 >> + vid1 DSS0_VID1 VID1 video plane 1 >> + vid2 DSS0_VID2 VID1 video plane 2 >> + ovr1 DSS0_OVR1 OVR1 overlay manager for vp1 >> + ovr2 DSS0_OVR2 OVR2 overlay manager for vp2 >> + ovr3 DSS0_OVR3 OVR1 overlay manager for vp3 >> + ovr4 DSS0_OVR4 OVR2 overlay manager for vp4 >> + vp1 DSS0_VP1 VP1 video port 1 >> + vp2 DSS0_VP2 VP1 video port 2 >> + vp3 DSS0_VP3 VP1 video port 3 >> + vp4 DSS0_VP4 VP1 video port 4 >> + wp DSS0_WB Write Back registers > > I guess it applies to all your schemas in that patch series, but you > could just do something like > > reg: > items: > - description: DSS Master common register area > - description: DSS Shared common register area 0 > - description: DSS Shared common register area 1 > Ok, thanks. I was not sure if you can do that (still a newbie with yaml). What do you think about Peter Ujfalusi's suggestion of putting the descriptions to reg-names (and clock-names and interrupt-names)? e.g. something like this: reg-names: items: - const: common_m - description: DSS Master common register area - const: common_s0 - description: DSS Master common register area ... Or is that even allowed? > ... > > That way, you wouldn't have to worry about the maxItems, and you end > up doing pretty much that already in the description > >> + reg-names: >> + items: >> + - const: common_m >> + - const: common_s0 >> + - const: common_s1 >> + - const: common_s2 >> + - const: vidl1 >> + - const: vidl2 >> + - const: vid1 >> + - const: vid2 >> + - const: ovr1 >> + - const: ovr2 >> + - const: ovr3 >> + - const: ovr4 >> + - const: vp1 >> + - const: vp2 >> + - const: vp3 >> + - const: vp4 >> + - const: wb >> + >> + clocks: >> + maxItems: 5 >> + description: >> + phandles to clock nodes for DSS functional clock (fck) and video >> + port 1, 2, 3 and 4 pixel clocks (vp1, vp2, vp3, vp4). >> + >> + clock-names: >> + items: >> + - const: fck >> + - const: vp1 >> + - const: vp2 >> + - const: vp3 >> + - const: vp4 >> + >> + interrupts: >> + maxItems: 4 >> + description: >> + Interrupt descriptions for common irq registers in common_m, >> + common_m0, common_m1, and common_m2, sections. > > Same story here, but the names don't match interrupt-names. I guess > describing what those interrupts actually are would be great: you just > define how the driver calls them, but not what they are actually doing > or representing. > > I'm guessing that would end up in something like that: > > interrupts: > items: > - description: DSS Master interrupt > - description: DSS Shared 0 interrupt > - description: DSS Shared 1 interrupt > - description: DSS Shared 2 interrupt > > Maxime > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Attachment:
signature.asc
Description: OpenPGP digital signature