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

Marcelo Schmitt
2019-11-13T14:52:08Z

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
> >