Re: dt_binding_check report false alarm?

[William Zhang <william.zhang@xxxxxxxxxxxx>]

On 05/25/2023 08:33 AM, Conor Dooley wrote:
> On Thu, May 25, 2023 at 08:23:30AM -0700, William Zhang wrote:
> > Sorry for the multiple emails. Our mail relay server was not working
> > properly.
>
> I only got one /shrug
That's good.  Maybe it only flushed queued email to internal email accounts.

> > On 05/25/2023 06:23 AM, Conor Dooley wrote:
> > > Hey William,
> > >
> > > On Wed, May 24, 2023 at 10:02:41PM -0700, William Zhang wrote:
> > > > Hi,
> > > >
> > > > It seems dt_binding_check reports a false error when run on this
> > > > modified yaml. I picked this simple file just to demostrate this issue.
> > > > Basically I made the interrupts and interrupt-names as optional
> > > > properties. But when there are two interrupts present, then
> > > > interrupt-names are required.  However in the example, I don't define
> > > > interrupts and interrupt-name at all, the dt binding check reports error
> > > > that interrupt-names are required:
> > >
> > > Rob and Krzysztof would know more than me, but since they're not
> > > around...
> > >
> > > > diff --git a/Documentation/devicetree/bindings/crypto/fsl-imx-scc.yaml b/Documentation/devicetree/bindings/crypto/fsl-imx-scc.yaml
> > > > index 563a31605d2b..c37a3a64a78c 100644
> > > > --- a/Documentation/devicetree/bindings/crypto/fsl-imx-scc.yaml
> > > > +++ b/Documentation/devicetree/bindings/crypto/fsl-imx-scc.yaml
> > > > @@ -32,11 +32,18 @@ properties:
> > > >      clock-names:
> > > >        const: ipg
> > > > +allOf:
> > > > +  - if:
> > > > +      properties:
> > > > +        interrupts:
> > > > +          minItems: 2
> > >
> > > ...I don't think you can actually do this and "minItems: 2" will always
> > > evaluate to true because it is an assignment. Don't hold me to that
> > > though! The standard pattern here is to do:
> > > allOf:
> > >     - if:
> > >         properties:
> > >           compatible:
> > >             contains:
> > >               const: foo
> > >       then:
> > >         required:
> > >           - interrupt-names
> > >
> > > Cheers,
> > > Conor.
> > Our device can use one or two interrupt, or choose to not use interrupt at
> > all(polling mode). Interrupt names is only required when there are two
> > interrupts(so the driver code can tell which is which).  So I will need to
> > check if it contains two interrupts. My check does work if I have two
> > interrupt but don't have interrupt name, the check catches the error.  If I
> > have one interrupt without interrupt name, the check pass. Only when I does
> > not have interrupt and interrupt name,  it falsely report error.  Looks to
> > me that it does not treat minItem = 0 case properly.
>
> Right. I would not bother with the "only interrupt-names when 2
> interrupts" stuff & do the simple thing of always making it required
> when you have interrupts.
> Then you can use allOf and oneOf to allow for both schemes for the new
> device and keep enforcement of 2 items for the existing one.
>
> Cheers,
> Conor.
I agree it is better to keep it simple. Sorry I am still new to yaml but 
what is the best way to check if interrupt property exist?  What I can 
think of is similar

 - if:
      properties:
        interrupts:
          minItems: 1
    then:
      required:
        - interrupt-names

But this still reports the same error when there is no interrupt.

> > > > +    then:
> > > > +      required:
> > > > +        - interrupt-names
> > > > +
> > > >    required:
> > > >      - compatible
> > > >      - reg
> > > > -  - interrupts
> > > > -  - interrupt-names
> > > >      - clocks
> > > >      - clock-names
> > > > @@ -49,6 +56,4 @@ examples:
> > > >            reg = <0x53fac000 0x4000>;
> > > >            clocks = <&clks 111>;
> > > >            clock-names = "ipg";
> > > > -        interrupts = <49>, <50>;
> > > > -        interrupt-names = "scm", "smn";
> > > >        };
> > > > --
> > > > 2.34.1

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature