linux-kernel - Re: [PATCH v2 1/4] dt-bindings: clock: si5351: convert to yaml

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <tw55zws77ydpjjzjg7itzfphuo3w6yqp6sgdt6ylwfsexjz2lm@qruycd72w56s>
Date:   Sat, 14 Oct 2023 14:00:24 +0000
From:   Alvin Šipraga <ALSI@...g-olufsen.dk>
To:     Rob Herring <robh@...nel.org>
CC:     Alvin Šipraga <alvin@...s.dk>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Andrew Lunn <andrew@...n.ch>,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
        Gregory Clement <gregory.clement@...tlin.com>,
        Rabeeh Khoury <rabeeh@...id-run.com>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 1/4] dt-bindings: clock: si5351: convert to yaml

On Tue, Oct 10, 2023 at 09:50:03AM -0500, Rob Herring wrote:
> On Wed, Oct 4, 2023 at 4:40 PM Alvin Šipraga <ALSI@...g-olufsen.dk> wrote:
> >
> > On Wed, Oct 04, 2023 at 09:39:37AM -0500, Rob Herring wrote:
> > > > +      silabs,multisynth-source:
> > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > +        enum: [ 0, 1 ]
> > > > +        description: |
> > >
> > > Don't need '|' if no formatting to preserve.
> >
> > I thought the line would be too long otherwise.
> > Column width is 80 in dt-schema as well, right?
> 
> Yes, and up to 100 is fine as an exception.
> 
> > >
> > > > +          Source PLL A (0) or B (1) for the corresponding multisynth divider.
> 
> But this doesn't look like it is over 80. Maybe if you put after
> 'description:' on the same line, but that's not what I said. It can
> still be on the next line. No '|' just means the line endings aren't
> fixed. Not important now, but if we were to generate pretty
> documentation from the schemas, then it would matter.

Ah I see. OK, I removed the | now and it works fine. :)

> 
> >
> > [...]
> >
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          contains:
> > > > +            enum:
> > > > +              - silabs,si5351a
> > > > +              - silabs,si5351a-msop
> > > > +              - silabs,si5351b
> > >
> > > Isn't this just the 'else' for the next one? Or more parts are coming?
> >
> > Not sure if more parts are coming - these are the only ones I am aware of. But I
> > have not checked thoroughly. I thought it better to be explicit, but I will
> > change the next one to an else: in v3 unless you change your mind.
> >
> > >
> > > > +    then:
> > > > +      properties:
> > > > +        clocks:
> > > > +          minItems: 1
> > > > +          maxItems: 1
> > > > +        clock-names:
> > > > +          items:
> > > > +            - const: xtal
> > > > +
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          contains:
> > > > +            const: silabs,si5351c
> > > > +    then:
> > > > +      properties:
> > > > +        clocks:
> > > > +          minItems: 1
> > > > +          maxItems: 2
> > > > +        clock-names:
> > > > +          minItems: 1
> > > > +          items:
> > > > +            - const: xtal
> > > > +            - const: clkin
> > >
> > > Define clocks and clock-names at the top level and just use
> > > minItems/maxItems in the if/then schemas.
> >
> > I was trying to imply here that it is invalid to specify clkin for the former
> > three part types - only for the si5351c. If I specify both in the top-level
> > clock-names:items then it would allow something like this:
> >
> >   clk {
> >     compatible = "silabs,si5351a-msop";
> >     clocks = <&ref25>;
> >     clock-names = "clkin"; /* not OK - Si5351A-MSOP only supports XTAL */
> >   };
> 
> What I'm saying will work. There are lots of examples in the tree. The
> top-level defines the full array and then the if/then schema just sets
> the max size to 1 so that the clkin entry is not allowed.
> 
> properties:
>   clock-names:
>     minItems: 1
>     items:
>       - const: xtal
>       - const: clkin
> 
> if:
>   properties:
>     compatible:
>       contains:
>         const: silabs,si5351a-msop
> then:
>   properties:
>     clock-names:
>       maxItems: 1
> 
> (and then "minItems: 2" for the cases with 2 clocks)

Thanks Rob, your suggestion worked! I will send a new version now.

Kind regards,
Alvin