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

Message ID	a8c614894252bb139a213b8c0219f3f46210b136.1573145089.git.marcelo.schmitt1@gmail.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <SRS0=9tCV=ZA=vger.kernel.org=linux-iio-owner@kernel.org> Date: Fri, 8 Nov 2019 10:56:09 -0300 From: Marcelo Schmitt <marcelo.schmitt1@gmail.com> To: jic23@kernel.org, robh@kernel.org Cc: dragos.bogdan@analog.com, alexandru.ardelean@analog.com, linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, kernel-usp@googlegroups.com Subject: [PATCH v4 1/2] dt-bindings: iio: adc: Add dt-schema for AD7292 Message-ID: <a8c614894252bb139a213b8c0219f3f46210b136.1573145089.git.marcelo.schmitt1@gmail.com> References: <cover.1573145089.git.marcelo.schmitt1@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <cover.1573145089.git.marcelo.schmitt1@gmail.com> User-Agent: NeoMutt/20180716 Sender: linux-iio-owner@vger.kernel.org Precedence: bulk
Series	iio: adc: Add support for AD7292 \| expand [v4,0/2] iio: adc: Add support for AD7292 [v4,1/2] dt-bindings: iio: adc: Add dt-schema for AD7292 [v4,2/2] iio: adc: Add driver support for AD7292

Marcelo Schmitt Nov. 8, 2019, 1:56 p.m. UTC

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

Jonathan Cameron Nov. 10, 2019, 4:07 p.m. UTC | #1

On Fri, 8 Nov 2019 10:56:09 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> Add a devicetree schema for AD7292 monitor and control system.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
Seems like you have addressed everything Rob raised, so I'll apply this
and it can go out for build testing.  Still some time for Rob to add
a tag if he wants to!

Thanks,

Jonathan

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

Rob Herring Nov. 12, 2019, 7:39 p.m. UTC | #2

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.

> +%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.

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

Marcelo Schmitt Nov. 13, 2019, 2:52 p.m. UTC | #3

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

Rob Herring Nov. 13, 2019, 5:06 p.m. UTC | #4

On Wed, Nov 13, 2019 at 8:52 AM Marcelo Schmitt
<marcelo.schmitt1@gmail.com> wrote:
>
> 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?

You need to do an incremental patch. Greg doesn't rebase.

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

Sorry, my fault there. The meta-schema requires 'minimum' if you give
'maximum'. So:

        items:
          - minimum: 0
            maximum: 7

The error message was less than useful, but I think I have a fix for that too.

Rob

Jonathan Cameron Nov. 16, 2019, 3:22 p.m. UTC | #5

On Wed, 13 Nov 2019 11:06:45 -0600
Rob Herring <robh@kernel.org> wrote:

> On Wed, Nov 13, 2019 at 8:52 AM Marcelo Schmitt
> <marcelo.schmitt1@gmail.com> wrote:
> >
> > 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?  
> 
> You need to do an incremental patch. Greg doesn't rebase.
Sorry. I jumped the gun a bit here.  Send me an incremental patch and
we'll get it queued up.  As it can be considered 'a license fix'
we can get it lined up for straight after the merge window.

Thanks,

Jonathan


> 
> > 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?  
> 
> Sorry, my fault there. The meta-schema requires 'minimum' if you give
> 'maximum'. So:
> 
>         items:
>           - minimum: 0
>             maximum: 7
> 
> The error message was less than useful, but I think I have a fix for that too.
> 
> Rob

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

Commit Message

Comments

Patch