Re: [PATCH v2 06/11] dt-bindings: display/msm: move qcom,sc7180-mdss schema to mdss.yaml

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 10/07/2022 12:00, Dmitry Baryshkov wrote:
> Move schema for qcom,sc7180-mdss from dpu-sc7180.yaml to mdss.yaml so
> that the dpu file describes only the DPU schema.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../bindings/display/msm/dpu-sc7180.yaml      | 149 +++++-------------
>  .../devicetree/bindings/display/msm/mdss.yaml |  45 +++++-
>  2 files changed, 80 insertions(+), 114 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml
> index d3c3e4b07897..9d4ec0b60c25 100644
> --- a/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml
> @@ -10,151 +10,78 @@ maintainers:
>    - Krishna Manikandan <quic_mkrishn@quicinc.com>
>  
>  description: |
> -  Device tree bindings for MSM Mobile Display Subsystem(MDSS) that encapsulates
> -  sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree
> -  bindings of MDSS and DPU are mentioned for SC7180 target.
> +  Device tree bindings for the DPU display controller for SC7180 target.
>  
>  properties:
>    compatible:
>      items:
> -      - const: qcom,sc7180-mdss
> +      - const: qcom,sc7180-dpu
>  
>    reg:
> -    maxItems: 1
> +    items:
> +      - description: Address offset and size for mdp register set
> +      - description: Address offset and size for vbif register set
>  
>    reg-names:
> -    const: mdss
> -
> -  power-domains:
> -    maxItems: 1
> +    items:
> +      - const: mdp
> +      - const: vbif
>  
>    clocks:
>      items:
> -      - description: Display AHB clock from gcc
> -      - description: Display AHB clock from dispcc
> +      - description: Display hf axi clock
> +      - description: Display ahb clock
> +      - description: Display rotator clock
> +      - description: Display lut clock
>        - description: Display core clock
> +      - description: Display vsync clock
>  
>    clock-names:
>      items:
> +      - const: bus
>        - const: iface
> -      - const: ahb
> +      - const: rot
> +      - const: lut
>        - const: core
> +      - const: vsync
>  
>    interrupts:
>      maxItems: 1
>  
> -  interrupt-controller: true
> -
> -  "#address-cells": true
> -
> -  "#size-cells": true
> -
> -  "#interrupt-cells":
> -    const: 1
> -
> -  iommus:
> -    items:
> -      - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0
> -
> -  ranges: true
> -
> -  interconnects:
> -    items:
> -      - description: Interconnect path specifying the port ids for data bus
> -
> -  interconnect-names:
> -    const: mdp0-mem
> +  power-domains:
> +    maxItems: 1
>  
> -  resets:
> -    items:
> -      - description: MDSS_CORE reset
> +  operating-points-v2: true
>  
> -patternProperties:
> -  "^display-controller@[0-9a-f]+$":
> -    type: object
> -    description: Node containing the properties of DPU.
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +    description: |
> +      Contains the list of output ports from DPU device. These ports
> +      connect to interfaces that are external to the DPU hardware,
> +      such as DSI, DP etc. Each output port contains an endpoint that
> +      describes how it is connected to an external interface.
>  
>      properties:
> -      compatible:
> -        items:
> -          - const: qcom,sc7180-dpu
> -
> -      reg:
> -        items:
> -          - description: Address offset and size for mdp register set
> -          - description: Address offset and size for vbif register set
> -
> -      reg-names:
> -        items:
> -          - const: mdp
> -          - const: vbif
> -
> -      clocks:
> -        items:
> -          - description: Display hf axi clock
> -          - description: Display ahb clock
> -          - description: Display rotator clock
> -          - description: Display lut clock
> -          - description: Display core clock
> -          - description: Display vsync clock
> -
> -      clock-names:
> -        items:
> -          - const: bus
> -          - const: iface
> -          - const: rot
> -          - const: lut
> -          - const: core
> -          - const: vsync
> -
> -      interrupts:
> -        maxItems: 1
> -
> -      power-domains:
> -        maxItems: 1
> -
> -      operating-points-v2: true
> -
> -      ports:
> -        $ref: /schemas/graph.yaml#/properties/ports
> -        description: |
> -          Contains the list of output ports from DPU device. These ports
> -          connect to interfaces that are external to the DPU hardware,
> -          such as DSI, DP etc. Each output port contains an endpoint that
> -          describes how it is connected to an external interface.
> -
> -        properties:
> -          port@0:
> -            $ref: /schemas/graph.yaml#/properties/port
> -            description: DPU_INTF1 (DSI1)
> -
> -          port@2:
> -            $ref: /schemas/graph.yaml#/properties/port
> -            description: DPU_INTF0 (DP)
> -
> -        required:
> -          - port@0
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: DPU_INTF1 (DSI1)
> +
> +      port@2:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: DPU_INTF0 (DP)
>  
>      required:
> -      - compatible
> -      - reg
> -      - reg-names
> -      - clocks
> -      - interrupts
> -      - power-domains
> -      - operating-points-v2
> -      - ports
> +      - port@0
>  
>  required:
>    - compatible
>    - reg
>    - reg-names
> -  - power-domains
>    - clocks
>    - interrupts
> -  - interrupt-controller
> -  - iommus
> -  - ranges
> +  - power-domains
> +  - operating-points-v2
> +  - ports
>  
>  additionalProperties: false
>  
> diff --git a/Documentation/devicetree/bindings/display/msm/mdss.yaml b/Documentation/devicetree/bindings/display/msm/mdss.yaml
> index 7d4ab3d71d2d..98f1f2501291 100644
> --- a/Documentation/devicetree/bindings/display/msm/mdss.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/mdss.yaml
> @@ -17,6 +17,7 @@ description:
>  properties:
>    compatible:
>      enum:
> +      - qcom,sc7180-mdss
>        - qcom,sdm845-mdss
>        - qcom,mdss
>  
> @@ -64,20 +65,21 @@ properties:
>        - description: MDSS_CORE reset
>  
>    interconnects:
> -    minItems: 2
> +    minItems: 1
>      items:
>        - description: MDP port 0
>        - description: MDP port 1
>        - description: Rotator
>  
>    interconnect-names:
> -    minItems: 2
> +    minItems: 1
>      items:
>        - const: mdp0-mem
>        - const: mdp1-mem
>        - const: rotator-mem
>  
>    iommus:
> +    minItems: 1
>      items:
>        - description: Phandle to apps_smmu node with SID mask for Hard-Fail port0
>        - description: Phandle to apps_smmu node with SID mask for Hard-Fail port1
> @@ -129,9 +131,11 @@ allOf:
>              - const: mdss
>  
>          interconnects:
> +          minItems: 1
>            maxItems: 2
>  
>          interconnect-names:
> +          minItems: 1
>            maxItems: 2
>  
>        required:
> @@ -157,6 +161,29 @@ allOf:
>          iommus:
>            minItems: 2
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,sc7180-mdss
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: Display AHB clock from gcc
> +            - description: Display AHB clock from dispcc
> +            - description: Display core clock
> +
> +        clock-names:
> +          items:
> +            - const: iface
> +            - const: ahb
> +            - const: core
> +
> +        iommus:
> +          maxItems: 1
> +
>  required:
>    - compatible
>    - reg
> @@ -177,7 +204,19 @@ patternProperties:
>      # TODO: add reference once the mdp5 is converted
>  
>    "^display-controller@(0|[1-9a-f][0-9a-f]*)$":
> -    $ref: dpu-sdm845.yaml
> +    oneOf:
> +      - $ref: dpu-sc7180.yaml
> +      - $ref: dpu-sdm845.yaml

I don't think this is good approach. These are strictly tied, so you
should have rather per SoC MDSS schema pulling in:
1. common MDSS
2. specific children schemas

This makes it quite obvious and specific. Your setup now allows putting
sc7180-dpu under sdm845-dpu.

Best regards,
Krzysztof