On Wed, May 6, 2020 at 8:11 AM Ricardo Cañuelo
<ricardo.canuelo@xxxxxxxxxxxxx> wrote:
>
> Hi Rob, thanks for taking the time to review the patch. Some comments
> below:
>
> On mar 05-05-2020 13:56:07, Rob Herring wrote:
> > > + # adi,input-style and adi,input-justification are required except in
> > > + # "rgb 1x" and "yuv444 1x" modes.
> > > + - if:
> > > + not:
> > > + properties:
> > > + adi,input-colorspace:
> > > + contains:
> > > + enum: [ rgb, yuv444 ]
> > > + adi,input-clock:
> > > + contains:
> > > + const: 1x
> >
> > I believe this will be true (before the not) if the properties are not
> > present. You need 'required' if that's not what you want.
>
> I'm not sure I understand what you mean, but dt_binding_check doesn't
> say anything about adi,input-style and adi,input-justification being
> required when adi,input-colorspace and adi,input-clock are not present.
>
> I think I covered every possible case wrt those properties when running
> dt_binding_check and I got the results I was looking for:
>
> - When compatible is either "adi,adv7533" or "adi,adv7535",
> adi,input-colorspace and adi,input-clock aren't required
>
> - For any of the other compatible strings, adi,input-colorspace and
> adi,input-clock are required.
>
> - When adi,input-colorspace and adi,input-clock are defined and they are
> different than "rgb 1x" or "yuv444 1x", adi,input-style and
> adi,input-justification are required.
>
> There's an issue I can't figure out, though. adi,input-colorspace and
> adi,input-clock are defined only for devices other than "adi,adv7533"
> and "adi,adv7535", but a DT for one of these devices can use those
> properties and the binding check won't complain. Moreover, it will check
> the above condition even if it doesn't make sense for them (ie. it may
> complain that adi,input-style and adi,input-justification are required
> even if they aren't defined for "adi,adv7533" and "adi,adv7535").
>
> I think it's a minor issue, but do you know if there's a way to model
> that? Are properties always unconditionally defined?
I think you want something like this:
if:
properties:
compatible:
contains:
enum:
- adi,adv7533
- adi,adv7535
then:
not:
properties:
adi,input-style: false
adi,input-justification: false
Which means the schema fails if either property is present.
It may also just be easier to split this schema into 2. It's a
judgement call on how much is shared vs. how much if/then/else logic
there is.
Rob
