linux-kernel - Re: [PATCH v4 1/3] dt-bindings: iio: adc: sophgo,cv18xx-saradc.yaml: Add Sophgo CV18XX SARADC binding

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: <20240821094150.5787905b@xps-13>
Date: Wed, 21 Aug 2024 09:41:50 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Conor Dooley <conor@...nel.org>
Cc: Thomas Bonnefille <thomas.bonnefille@...tlin.com>, Jonathan Cameron
 <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>, Rob Herring
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Chen Wang <unicorn_wang@...look.com>, Inochi Amaoto
 <inochiama@...look.com>, Paul Walmsley <paul.walmsley@...ive.com>, Palmer
 Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>, Thomas
 Petazzoni <thomas.petazzoni@...tlin.com>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v4 1/3] dt-bindings: iio: adc:
 sophgo,cv18xx-saradc.yaml: Add Sophgo CV18XX SARADC binding

Hello,

> > > > +      Represents the channels of the ADC.
> > > > +
> > > > +    properties:
> > > > +      reg:
> > > > +        description: |
> > > > +          The channel number. It can have up to 3 channels numbered from 0 to 2.
> > > > +        items:
> > > > +          - minimum: 0
> > > > +            maximum: 2  
> > > 
> > > Is this sufficient to limit the number of channels to 3? Aren't you relying
> > > on the unique unit addresses warning in dtc to limit it, rather than
> > > actually limiting with min/maxItems?
> > >   
> > It seems like I can't use min/maxItems on this property. I think that it is
> > using size-cells + address-cells to deduce that the number of items should
> > be equal to 1.  

Looking at dt-schema, I couldn't personally understand from where did
the error messages reported by Thomas came from. There are clear
constraints over minItems/maxItems regarding the use of {#address-cells,
#sizez-cells} being {1, 1}, {2, 2} and {2, 1} (in reg.yaml), but nothing
explicit regarding the other situations, namely {1, 0} in this case
which enforces maxItems to 1 is not clearly stated in any of the core
yaml files. Any idea where to look at? Although, I'm convinced there is
something defined because renaming the property from 'reg' to 'foo'
silences these warnings.

> I think I was mistaken in talking about mix/max items here. I had the
> right idea, but mentioned an incorrect solution - sorry about that. I
> wasn't talking about the number of elements in the reg property, what I
> meant was limiting the number of channel nodes in the first place -
> something which min/maxItems cannot do. As examples of the problem I was
> thinking of, see the below two examples:
> 
>     adc@...0000 {
>         compatible = "sophgo,cv1800b-saradc";
>         reg = <0x030f0000 0x1000>;
>         clocks = <&clk CLK_SARADC>;
>         interrupts = <100 IRQ_TYPE_LEVEL_HIGH>;
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         channel@0 {
>             reg = <0>;
>         };
>         channel@2 {
>             reg = <2>;
>         };
>         channel@22 {
>             reg = <2>;
>         };
>     };
> 
>     adc@...0000 {
>         compatible = "sophgo,cv1800b-saradc";
>         reg = <0x030f0000 0x1000>;
>         clocks = <&clk CLK_SARADC>;
>         interrupts = <100 IRQ_TYPE_LEVEL_HIGH>;
>         #address-cells = <1>;
>         #size-cells = <0>;
> 
>         channel@0 {
>             reg = <0>;
>         };
>         channel@2 {
>             reg = <2>;
>         };
>         channel@22 {
>             reg = <2>;
>         };
>     };
> 
> The solution is simple, remove the + from the regex. Sorry for sending
> you on the wrong track Thomas.

Ah! Thanks Conor for the details, now it makes full sense :-) BTW Thomas
the regex is

	^channel@[0-3]+$

and I guess it should instead be

	^channel@[0-2]$
                    ^

in order to fully match the real indexing constraints you're enforcing
with minimum/maximum.

Thanks,
Miquèl