[PATCH v4 3/5] dt-bindings: display: ti, j721e-dss: Add dt-schema yaml binding

[Maxime Ripard]

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 at ti.com>
> ---
>  .../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 at ti.com>
> +  - Tomi Valkeinen <tomi.valkeinen at ti.com>
> +
> +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

...

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20191219/9e9d096b/attachment-0001.sig>