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

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL_JsqJY6YMC2ba4DNFnsq=XESOoUTDA6Q+Y7gutMT0VN--DNw@mail.gmail.com>
Date:   Wed, 13 Nov 2019 11:06:45 -0600
From:   Rob Herring <robh@...nel.org>
To:     Marcelo Schmitt <marcelo.schmitt1@...il.com>
Cc:     Jonathan Cameron <jic23@...nel.org>,
        Dragos Bogdan <dragos.bogdan@...log.com>,
        Alexandru Ardelean <alexandru.ardelean@...log.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "open list:IIO SUBSYSTEM AND DRIVERS" <linux-iio@...r.kernel.org>,
        devicetree@...r.kernel.org, kernel-usp@...glegroups.com
Subject: Re: [PATCH v4 1/2] dt-bindings: iio: adc: Add dt-schema for AD7292

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