Re: [PATCH v7 1/3] media: dt-bindings: media: renesas,vsp1: Document RZ/{G2L,V2L} VSPD bindings To: Mauro Carvalho Chehab <mchehab@kernel.org>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzk+dt@kernel.org>

[Rob Herring <robh@kernel.org>]

On Tue, Apr 19, 2022 at 11:29:39AM +0300, Laurent Pinchart wrote:
> Hi Biju,
> 
> On Mon, Apr 18, 2022 at 07:34:19PM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH v7 1/3] media: dt-bindings: media: renesas,vsp1:
> > > Document RZ/{G2L,V2L} VSPD bindings 
> > > 
> > > Hi Biju,
> > > 
> > > Thank you for the patch.
> > > 
> > > On Thu, Apr 14, 2022 at 03:26:03PM +0100, 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>
> > > > ---
> > > > v6->v7:
> > > >  * No change
> > > > v5->v6:
> > > >  * Removed LCDC reference clock description
> > > >  * Changed the clock name from du.0->aclk
> > > > 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:
> > > >  *
> > > 
> > > > ---
> > > >  .../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..a236b266fa4b 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
> > > 
> > > clock-names shouldn't be true here, as it should only be set on rzg2l-vsp2.
> > > I think you can actually drop both clocks and clock-names here.
> > 
> > If I drop clocks, then I get below dt_binding_check error
> > 
> > biju@biju-VirtualBox:~/rzg2l-linux$ make ARCH=arm64 DT_CHECKER_FLAGS=-m DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/renesas,vsp1.yaml CROSS_COMPILE=~/gcc-arm-10.3-2021.07-x86_64-aarch64-none-linux-gnu/bin/aarch64-none-linux-gnu- dt_binding_check -j8
> >   LINT    Documentation/devicetree/bindings
> >   DTEX    Documentation/devicetree/bindings/media/renesas,vsp1.example.dts
> >   CHKDT   Documentation/devicetree/bindings/processed-schema.json
> >   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
> >   DTC     Documentation/devicetree/bindings/media/renesas,vsp1.example.dtb
> >   CHECK   Documentation/devicetree/bindings/media/renesas,vsp1.example.dtb
> > /home/biju/rzg2l-linux/Documentation/devicetree/bindings/media/renesas,vsp1.example.dtb: vsp@fe928000: 'clocks' does not match any of the regexes: 'pinctrl-[0-9]+'
> > 	From schema: /home/biju/rzg2l-linux/Documentation/devicetree/bindings/media/renesas,vsp1.yaml
> > /home/biju/rzg2l-linux/Documentation/devicetree/bindings/media/renesas,vsp1.example.dtb: vsp@fe920000: 'clocks' does not match any of the regexes: 'pinctrl-[0-9]+'
> > 	From schema: /home/biju/rzg2l-linux/Documentation/devicetree/bindings/media/renesas,vsp1.yaml
> > 
> > If I drop clock-names, I get dtbs-check error for RZ/G2{L,LC},
> > 
> > make ARCH=arm64 DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/renesas,vsp1.yaml CROSS_COMPILE=~/gcc-arm-10.3-2021.07-x86_64-aarch64-none-linux-gnu/bin/aarch64-none-linux-gnu- dtbs_check -j8
> > 
> > /home/biju/rzg2l-linux/arch/arm64/boot/dts/renesas/r9a07g044c2-smarc.dtb: vsp@10870000: 'clock-names' does not match any of the regexes: 'pinctrl-[0-9]+'
> > 	From schema: /home/biju/rzg2l-linux/Documentation/devicetree/bindings/media/renesas,vsp1.yaml
> > /home/biju/rzg2l-linux/arch/arm64/boot/dts/renesas/r9a07g044l2-smarc.dtb: vsp@10870000: 'clock-names' does not match any of the regexes: 'pinctrl-[0-9]+'
> > 	From schema: /home/biju/rzg2l-linux/Documentation/devicetree/bindings/media/renesas,vsp1.yaml
> > 
> > So looks like both are required.
> 
> Indeed, we would need to switch from additionalProperties to
> unevaluatedProperties then, and that's not allowed for schemas without a
> $ref.

The issue is here if we allow properties to be defined in if/then 
schemas, we can't have some meta-schema checks on them as we don't know 
if the property is defined elsewhere or not. That's primarily a problem 
for vendor specific properties where we need to ensure a type and 
description.

Rob