Re: [PATCH v2 1/4] media: dt-bindings: adi,adv7180: add VPP and CSI register maps

[Michael Tretter <m.tretter@pengutronix.de>]

On Thu, 20 Nov 2025 09:04:48 +0100, Krzysztof Kozlowski wrote:
> On Wed, Nov 19, 2025 at 05:25:51PM +0100, Michael Tretter wrote:
> > diff --git a/Documentation/devicetree/bindings/media/i2c/adi,adv7180.yaml b/Documentation/devicetree/bindings/media/i2c/adi,adv7180.yaml
> > index dee8ce7cb7ba..dbbbe76291bc 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/adi,adv7180.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/adi,adv7180.yaml
> > @@ -30,7 +30,27 @@ properties:
> >            - adi,adv7282-m
> >  
> >    reg:
> > -    maxItems: 1
> > +    minItems: 1
> > +    items:
> > +      - description: main register map
> > +      - description: CSI register map
> > +      - description: VPP register map
> > +    description:
> > +      The ADV7180 family may have up to three register maps. All chips have
> > +      the main register map. The availability of the CSI and VPP register maps
> > +      depends on the chip variant.
> > +
> > +      The addresses of the CSI and VPP register maps are programmable by
> > +      software. They depend on the board layout and other devices on the I2C
> > +      bus and are determined by the hardware designer to avoid address
> > +      conflicts on the I2C bus.
> > +
> > +  reg-names:
> > +    minItems: 1
> > +    items:
> > +      - const: main
> > +      - enum: [ csi, vpp ]
> > +      - enum: [ csi, vpp ]
> 
> Last entry must be:
> 
> const: vpp
> 
> We do not allow flexible order... but the problem is that your if:then:
> does not match above at all. You do not have three items anywhere.

I'm not entirely sure, if I correctly understand that comment.

The adi,adv7280-m and adi,adv7282-m have all three items and don't need
an if:then:. Do I have explicitly define the binding with three items,
too?

The chip has the following variants:

adi,adv7180:     main
adi,adv7180cp:   main
adi,adv7180st:   main
adi,adv7182:     main
adi,adv7280:     main, vpp
adi,adv7280-m:   main, csi, vpp
adi,adv7281:     main, csi
adi,adv7281-m:   main, csi
adi,adv7281-ma:  main, csi
adi,adv7282:     main, vpp
adi,adv7282-m:   main, csi, vpp

If I make the last entry (vpp) const, I allow exactly these variants.

For the adi,adv7280-m compatible, the following combinations would be
valid or invalid:

adi,adv7280-m:   main

is valid, because only main is mandatory. For csi and vpp, the default
addresses are used.

adi,adv7280-m:   main, vpp

is valid, because the second entry may be vpp. For csi, the default
address is used.

adi,adv7280-m:   main, vpp, csi

is invalid, because the entries must be in the defined order, and
flexible order is not possible.

Is this correct and matches the binding definition?

Thanks!

Michael

> 
> 
> >  
> >    powerdown-gpios:
> >      maxItems: 1
> > @@ -138,6 +158,58 @@ allOf:
> >        required:
> >          - ports
> >  
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - adi,adv7180
> > +              - adi,adv7180cp
> > +              - adi,adv7180st
> > +              - adi,adv7182
> > +    then:
> > +      properties:
> > +        reg:
> > +          maxItems: 1
> > +
> > +        reg-names:
> > +          maxItems: 1
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - adi,adv7281
> > +              - adi,adv7281-m
> > +              - adi,adv7281-ma
> > +    then:
> > +      properties:
> > +        reg:
> > +          maxItems: 2
> > +
> > +        reg-names:
> > +          items:
> > +            - const: main
> > +            - enum: [ csi ]
> 
> const
> 
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - adi,adv7280
> > +              - adi,adv7282
> > +    then:
> > +      properties:
> > +        reg:
> > +          maxItems: 2
> > +
> > +        reg-names:
> > +          items:
> > +            - const: main
> > +            - enum: [ vpp ]
> 
> const
> 
> > +
> >  examples: