Re: [PATCH v4 1/2] dt-bindings: iio: adc: Add dt-schema for AD7292

[Marcelo Schmitt <marcelo.schmitt1@gmail.com>]

Hi Rob,

Thanks for reviewing the binding doc again.
Aparently, this patch was added to Greg KH's staging tree.
What is the right procedure in this case? Should I send a v5 patchset or
just send a patch for this doc?

In any case, I still have some doubts about the maximum constraint of
the channel property. Comments inline.


Thanks

Marcelo

On 11/12, Rob Herring wrote:
> On Fri, Nov 08, 2019 at 10:56:09AM -0300, Marcelo Schmitt wrote:
> > Add a devicetree schema for AD7292 monitor and control system.
> > 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> > ---
> > Changelog V3 -> V4:
> > - updated SPDX identifier to GPL-2.0-only
> > - changed maxitems constraint on channel property
> > 
> >  .../bindings/iio/adc/adi,ad7292.yaml          | 104 ++++++++++++++++++
> >  MAINTAINERS                                   |   7 ++
> >  2 files changed, 111 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
> > new file mode 100644
> > index 000000000000..b68be3aaf587
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
> > @@ -0,0 +1,104 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> 
> Sigh, I gave you the exact line to use:
> 
> # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> 
> I've said to dual license with (GPL-2.0-only OR BSD-2-Clause) and people 
> think I mean to pick one. So now I just give the whole line. I don't 
> know how to be clearer.

I thought I could use just GPL-2.0 since the driver code is GPL-2.0.
Anyway, I'll use the above line to specify the dt-binding license.

> 
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/adc/adi,ad7292.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices AD7292 10-Bit Monitor and Control System
> > +
> > +maintainers:
> > +  - Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> > +
> > +description: |
> > +  Analog Devices AD7292 10-Bit Monitor and Control System with ADC, DACs,
> > +  Temperature Sensor, and GPIOs
> > +
> > +  Specifications about the part can be found at:
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7292.pdf
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,ad7292
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  vref-supply:
> > +    description: |
> > +      The regulator supply for ADC and DAC reference voltage.
> > +
> > +  spi-cpha: true
> > +
> > +  '#address-cells':
> > +    const: 1
> > +
> > +  '#size-cells':
> > +    const: 0
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - spi-cpha
> > +
> > +patternProperties:
> > +  "^channel@[0-7]$":
> > +    type: object
> > +    description: |
> > +      Represents the external channels which are connected to the ADC.
> > +      See Documentation/devicetree/bindings/iio/adc/adc.txt.
> > +
> > +    properties:
> > +      reg:
> > +        description: |
> > +          The channel number. It can have up to 8 channels numbered from 0 to 7.
> > +        items:
> > +          maximum: 7
> 
> Not what I said either. A slight but important difference in that you 
> are missing a '-' to make 'items' a list rather than a schema/dict.
> 
> Update dt-schema. This should give a warning now.

I'm confused, I don't know how to make this doc the way you want.
I pulled the updates from the master branch of dt-schema repo and
reinstalled it.
Then I tried
        items:
          - maximum: 7
I've tried
        - items:
            maximum: 7
I also tried
        - items:
          maximum: 7
all gave me parsing errors when processing the ad7292 schema with 
'make dt_binding_check' and also with 'make -k dt_binding_check'.
Am I using the right branch? Should I pull from a branch other than the
master?
I was first inspired by the adi,ad7124.yaml doc which has a similar
channel declaration. Curiously, processing the ad7292 schema the way it
was in v4 gave me no errors so, I must be missing something.

> 
> > +
> > +      diff-channels:
> > +        description: see Documentation/devicetree/bindings/iio/adc/adc.txt
> > +        maxItems: 1
> > +
> > +    required:
> > +      - reg
> > +
> > +examples:
> > +  - |
> > +    spi {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +
> > +      ad7292: adc@0 {
> > +        compatible = "adi,ad7292";
> > +        reg = <0>;
> > +        spi-max-frequency = <25000000>;
> > +        vref-supply = <&adc_vref>;
> > +        spi-cpha;
> > +
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        channel@0 {
> > +          reg = <0>;
> > +          diff-channels = <0 1>;
> > +        };
> > +        channel@2 {
> > +          reg = <2>;
> > +        };
> > +        channel@3 {
> > +          reg = <3>;
> > +        };
> > +        channel@4 {
> > +          reg = <4>;
> > +        };
> > +        channel@5 {
> > +          reg = <5>;
> > +        };
> > +        channel@6 {
> > +          reg = <6>;
> > +        };
> > +        channel@7 {
> > +          reg = <7>;
> > +        };
> > +      };
> > +    };
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 32bf5f8116d0..5d00e871c4c6 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -813,6 +813,13 @@ S:	Supported
> >  F:	drivers/iio/adc/ad7124.c
> >  F:	Documentation/devicetree/bindings/iio/adc/adi,ad7124.txt
> >  
> > +ANALOG DEVICES INC AD7292 DRIVER
> > +M:	Marcelo Schmitt <marcelo.schmitt1@gmail.com>
> > +L:	linux-iio@vger.kernel.org
> > +W:	http://ez.analog.com/community/linux-device-drivers
> > +S:	Supported
> > +F:	Documentation/devicetree/bindings/iio/adc/adi,ad7292.yaml
> > +
> >  ANALOG DEVICES INC AD7606 DRIVER
> >  M:	Stefan Popa <stefan.popa@analog.com>
> >  L:	linux-iio@vger.kernel.org
> > -- 
> > 2.23.0
> >