[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e7693e9d-a46a-4dc2-9aee-36a2bbf74ade@linaro.org>
Date: Wed, 22 Nov 2023 20:07:56 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Andy Yan <andyshrk@....com>, heiko@...ech.de
Cc: hjc@...k-chips.com, dri-devel@...ts.freedesktop.org,
linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
krzysztof.kozlowski+dt@...aro.org, robh+dt@...nel.org,
devicetree@...r.kernel.org, sebastian.reichel@...labora.com,
kever.yang@...k-chips.com, chris.obbard@...labora.com,
s.hauer@...gutronix.de, Andy Yan <andy.yan@...k-chips.com>
Subject: Re: [PATCH v2 08/12] dt-bindings: display: vop2: Add rk3588 support
On 22/11/2023 13:55, Andy Yan wrote:
> From: Andy Yan <andy.yan@...k-chips.com>
>
> The vop2 on rk3588 is similar to which on rk356x
> but with 4 video ports and need to reference
> more grf modules.
>
> Signed-off-by: Andy Yan <andy.yan@...k-chips.com>
>
> ---
>
> Changes in v2:
> - fix errors when running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
>
> .../display/rockchip/rockchip-vop2.yaml | 27 +++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> index b60b90472d42..24148d9b3b14 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
> @@ -20,6 +20,7 @@ properties:
> enum:
> - rockchip,rk3566-vop
> - rockchip,rk3568-vop
> + - rockchip,rk3588-vop
>
> reg:
> items:
> @@ -42,26 +43,47 @@ properties:
> frame start (VSYNC), line flag and other status interrupts.
>
> clocks:
> + minItems: 3
> items:
> - description: Clock for ddr buffer transfer.
> - description: Clock for the ahb bus to R/W the phy regs.
> - description: Pixel clock for video port 0.
> - description: Pixel clock for video port 1.
> - description: Pixel clock for video port 2.
> + - description: Pixel clock for video port 4.
> + - description: Peripheral clock for vop on rk3588.
>
> clock-names:
> + minItems: 3
You relax requirements for all existing variants here which is not
explained in commit msg. I assume this was not intentional, so you need
to re-constrain them in allOf:if:then.
See for example:
https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L57
for some ideas.
> 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
>
> + rockchip,vo-grf:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to VO GRF regs used for misc control, required for rk3588
Drop last sentence, instead add it to required in allOf:if:then.
Is this valid for other variants? If not, should be disallowed in
allOf:if:then: for them.
> +
> + rockchip,vop-grf:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to VOP GRF regs used for misc control, required for rk3588
> +
> + rockchip,pmu:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to PMU regs used for misc control, required for rk3588
For all these three: what is "misc control"? Way too vague. Everything
is a misc and everything can be control. You must be here specific and
much more descriptive.
> +
> ports:
> $ref: /schemas/graph.yaml#/properties/ports
>
> @@ -81,6 +103,11 @@ properties:
> description:
> Output endpoint of VP2
>
> + port@3:
> + $ref: /schemas/graph.yaml#/properties/port
> + description:
> + Output endpoint of VP3
Valid for other variants?
Best regards,
Krzysztof
Powered by blists - more mailing lists