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

Miquel Raynal
2024-08-21T07:42:07Z

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@30f0000 {
> 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@30f0000 {
> 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