Re: [PATCH] dt-bindings: adv748x: Convert bindings to json-schema

["Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>]

Hi Geert,

Thanks for your review.

On 2021-10-13 17:08:44 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> Thanks for your patch!
> 
> On Tue, Oct 12, 2021 at 8:48 PM Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > Convert ADV748X analog video decoder documentation to json-schema.
> 
> You may want to list what changes you made:
>   - Add missing ports subnode
>   - Anything else I didn't notice? ;-)

Wops, I intended to mention the addition of ports node. Thanks for 
noticing, will send a v2.

> 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/adv748x.yaml
> 
> > +  reg-names:
> > +    minItems: 1
> > +    items:
> > +      - const: main
> > +      - enum: [ dpll, cp, hdmi, edid, repeater, infoframe, cbus, cec, sdp, txa, txb ]
> > +      - enum: [ dpll, cp, hdmi, edid, repeater, infoframe, cbus, cec, sdp, txa, txb ]
> > +      - enum: [ dpll, cp, hdmi, edid, repeater, infoframe, cbus, cec, sdp, txa, txb ]
> > +      - enum: [ dpll, cp, hdmi, edid, repeater, infoframe, cbus, cec, sdp, txa, txb ]
> > +      - enum: [ dpll, cp, hdmi, edid, repeater, infoframe, cbus, cec, sdp, txa, txb ]
> > +      - enum: [ dpll, cp, hdmi, edid, repeater, infoframe, cbus, cec, sdp, txa, txb ]
> > +      - enum: [ dpll, cp, hdmi, edid, repeater, infoframe, cbus, cec, sdp, txa, txb ]
> > +      - enum: [ dpll, cp, hdmi, edid, repeater, infoframe, cbus, cec, sdp, txa, txb ]
> > +      - enum: [ dpll, cp, hdmi, edid, repeater, infoframe, cbus, cec, sdp, txa, txb ]
> > +      - enum: [ dpll, cp, hdmi, edid, repeater, infoframe, cbus, cec, sdp, txa, txb ]
> > +      - enum: [ dpll, cp, hdmi, edid, repeater, infoframe, cbus, cec, sdp, txa, txb ]
> 
> They can really appear in any order?

Yes they are refereed by i2c_new_ancillary_device() so any order is 
fine. They can only appear once tho, but I'm not sure how to enforce 
that.

> 
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: adi,adv7481
> > +    then:
> > +      properties:
> > +        interrupts:
> > +          minItems: 1
> > +          maxItems: 3
> > +
> > +        interrupt-names:
> > +          minItems: 1
> > +          items:
> > +            - enum: [ intrq1, intrq2, intrq3 ]
> > +            - enum: [ intrq1, intrq2, intrq3 ]
> > +            - enum: [ intrq1, intrq2, intrq3 ]
> 
> They can really appear in any order?

Yes, as not all interrupt lines may be wired up. It would be nice to 
enforce that intrq2 must come before intrq3 if both are wired but I'm 
not sure how to express that.

> 
> > +    else:
> > +      properties:
> > +        interrupts:
> > +          minItems: 1
> > +          maxItems: 2
> > +
> > +        interrupt-names:
> > +          minItems: 1
> > +          items:
> > +            - enum: [ intrq1, intrq2 ]
> > +            - enum: [ intrq1, intrq2 ]
> 
> They can really appear in any order?
> 
> If not, you can merge the entries, and just override maxItems.
> 
> The rest looks good to me, but I'm no graph expert.
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks!

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

-- 
Regards,
Niklas Söderlund