Re: [PATCH v3 09/14] dt-bindings: display: vop2: Add rk3588 support

Krzysztof Kozlowski
2023-12-01T08:17:44Z

On 30/11/2023 13:24, Andy Yan wrote:
> clock-names:
> + minItems: 5
> items:
> - const: aclk
> - const: hclk
> - const: dclk_vp0
> - const: dclk_vp1
> - const: dclk_vp2
> + - const: dclk_vp3
> + - const: pclk_vop
>
> rockchip,grf:
> $ref: /schemas/types.yaml#/definitions/phandle
> description:
> - Phandle to GRF regs used for misc control
> + Phandle to GRF regs used for control the polarity of dclk/hsync/vsync of DPI,
> + also used for query vop memory bisr enable status, etc.
> +
> + rockchip,vo1-grf:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to VO GRF regs used for control the polarity of dclk/hsync/vsync of hdmi
> + on rk3588
> +
> + rockchip,vop-grf:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to VOP GRF regs used for control data path between vopr and hdmi/edp.
> +
> + rockchip,pmu:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to PMU GRF used for query vop memory bisr status on rk3588

Some of your above descriptions end with full stop, some not. Keep one
style.

>
> ports:
> $ref: /schemas/graph.yaml#/properties/ports
> -
> - properties:
> - port@0:
> + description: |
> + The connections to the output video ports are modeled using the OF
> + graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> + The number of ports and their assignment are model-dependent. Each port
> + shall have a single endpoint.

The description is redundant, drop.

> +
> + patternProperties:
> + "^port@[0-3]$":
> $ref: /schemas/graph.yaml#/properties/port
> - description:
> - Output endpoint of VP0
> + description: Output endpoint of VP0/1/2/3
> + unevaluatedProperties: false

Why did you add this line? It should not be needed, if I read diff
correctly.

>
> - port@1:
> - $ref: /schemas/graph.yaml#/properties/port
> - description:
> - Output endpoint of VP1
> + required:
> + - port@0
>
> - port@2:
> - $ref: /schemas/graph.yaml#/properties/port
> - description:
> - Output endpoint of VP2
> + unevaluatedProperties: false
>
> iommus:
> maxItems: 1
> @@ -96,6 +121,61 @@ required:
> - clock-names
> - ports
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: rockchip,rk3588-vop
> + then:
> + properties:
> + clocks:
> + minItems: 7
> + clock-names:
> + items:
> + - const: aclk
> + - const: hclk
> + - const: dclk_vp0
> + - const: dclk_vp1
> + - const: dclk_vp2
> + - const: dclk_vp3
> + - const: pclk_vop

These look the same as in top-level, so just minItems: 7

> +
> + ports:
> + required:
> + - port@0
> + - port@1
> + - port@2
> + - port@3
> +
> + required:
> + - rockchip,grf
> + - rockchip,vo1-grf
> + - rockchip,vop-grf
> + - rockchip,pmu
> +
> + else:
> + properties:
> + rockchip,vo1-grf: false
> + rockchip,vop-grf: false
> + rockchip,pmu: false
> +
> + clocks:
> + minItems: 5

maxItems instead

> + clock-names:

maxItems

Best regards,
Krzysztof