Re: [PATCH v5 1/3] media: dt-bindings: media: renesas,vsp1: Document RZ/{G2L,V2L} VSPD bindings

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Biju,

Thank you for the patch.

On Sat, Mar 12, 2022 at 08:42:03AM +0000, Biju Das wrote:
> Document VSPD found in RZ/G2L and RZ/V2L family SoC's. VSPD block is
> similar to VSP2-D found on R-Car SoC's, but it does not have a version
> register and it has 3 clocks compared to 1 clock on vsp1 and vsp2.
> 
> This patch introduces a new compatible 'renesas,rzg2l-vsp2' to handle
> these differences.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> ---
> v4->v5:
>  * No change
> v3->v4:
>  * No change
> v2->v3:
>  * Added Rb tag from Krzysztof.
> v1->v2:
>  * Changed compatible from vsp2-rzg2l->rzg2l-vsp2
> RFC->v1:
>  * Updated commit description
>  * Changed compatible from vsp2-r9a07g044->vsp2-rzg2l
>  * Defined the clocks
>  * Clock max Items is based on SoC Compatible string
> RFC:
>  * https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220112174612.10773-20-biju.das.jz@bp.renesas.com/
> ---
>  .../bindings/media/renesas,vsp1.yaml          | 52 ++++++++++++++-----
>  1 file changed, 39 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,vsp1.yaml b/Documentation/devicetree/bindings/media/renesas,vsp1.yaml
> index 990e9c1dbc43..2696a4582251 100644
> --- a/Documentation/devicetree/bindings/media/renesas,vsp1.yaml
> +++ b/Documentation/devicetree/bindings/media/renesas,vsp1.yaml
> @@ -19,6 +19,7 @@ properties:
>      enum:
>        - renesas,vsp1 # R-Car Gen2 and RZ/G1
>        - renesas,vsp2 # R-Car Gen3 and RZ/G2
> +      - renesas,rzg2l-vsp2 # RZ/G2L and RZ/V2L
>  
>    reg:
>      maxItems: 1
> @@ -26,8 +27,8 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> -  clocks:
> -    maxItems: 1
> +  clocks: true
> +  clock-names: true
>  
>    power-domains:
>      maxItems: 1
> @@ -50,17 +51,42 @@ required:
>  
>  additionalProperties: false
>  
> -if:
> -  properties:
> -    compatible:
> -      items:
> -        - const: renesas,vsp1
> -then:
> -  properties:
> -    renesas,fcp: false
> -else:
> -  required:
> -    - renesas,fcp
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,vsp1
> +    then:
> +      properties:
> +        renesas,fcp: false
> +    else:
> +      required:
> +        - renesas,fcp
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,rzg2l-vsp2
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: LCDC Main clock
> +            - description: LCDC Register Access Clock
> +            - description: LCDC Video Clock

Shouldn't we avoid referring to LCDC in the VSP bindings ?

> +        clock-names:
> +          items:
> +            - const: du.0

Similarly, I'm not sure this is a good name from the point of view of
the VSP.

> +            - const: pclk
> +            - const: vclk

I couldn't find those names in the documentation, where do they come
from ? Could you maybe share a DT integration example ?

> +      required:
> +        - clock-names
> +    else:
> +      properties:
> +        clocks:
> +          maxItems: 1
>  
>  examples:
>    # R8A7790 (R-Car H2) VSP1-S

-- 
Regards,

Laurent Pinchart