RE: [PATCH 3/3] dt-bindings: iio: Add ltc2688 documentation

Sa, Nuno
2022-01-07T15:49:38Z

> From: Jonathan Cameron <jic23@jic23.retrosnub.co.uk>
> Sent: Thursday, December 30, 2021 4:44 PM
> To: Sa, Nuno <Nuno.Sa@analog.com>; devicetree@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>; linux-iio@vger.kernel.org; Lars-
> Peter Clausen <lars@metafoo.de>; Hennerich, Michael
> <Michael.Hennerich@analog.com>
> Subject: Re: [PATCH 3/3] dt-bindings: iio: Add ltc2688 documentation
>
> [External]
>
> On Fri, 17 Dec 2021 09:09:30 +0000
> "Sa, Nuno" <Nuno.Sa@analog.com> wrote:
>
> > > From: Jonathan Cameron <jic23@jic23.retrosnub.co.uk>
> > > Sent: Thursday, December 16, 2021 2:33 PM
> > > To: Rob Herring <robh@kernel.org>
> > > Cc: Sa, Nuno <Nuno.Sa@analog.com>; linux-iio@vger.kernel.org;
> > > devicetree@vger.kernel.org; Lars-Peter Clausen
> <lars@metafoo.de>;
> > > Hennerich, Michael <Michael.Hennerich@analog.com>
> > > Subject: Re: [PATCH 3/3] dt-bindings: iio: Add ltc2688
> documentation
> > >
> > >
> > > On Wed, 15 Dec 2021 15:30:37 -0600
> > > Rob Herring <robh@kernel.org> wrote:
> > >
> > > > On Tue, Dec 14, 2021 at 05:56:08PM +0100, Nuno Sá wrote:
> > > > > Document the LTC2688 devicetree properties.
> > > > >
> > > > > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > > > > ---
> > > > > .../bindings/iio/dac/adi,ltc2688.yaml | 146
> > > ++++++++++++++++++
> > > > > MAINTAINERS | 1 +
> > > > > 2 files changed, 147 insertions(+)
> > > > > create mode 100644
> > > Documentation/devicetree/bindings/iio/dac/adi,ltc2688.yaml
> > > > >
> > > > > diff --git
> > > a/Documentation/devicetree/bindings/iio/dac/adi,ltc2688.yaml
> > > b/Documentation/devicetree/bindings/iio/dac/adi,ltc2688.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..7919cd8ec7c9
> > > > > --- /dev/null
> > > > > +++
> > > b/Documentation/devicetree/bindings/iio/dac/adi,ltc2688.yaml
> > > > > @@ -0,0 +1,146 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id:
> > >
> https://urldefense.com/v3/__http://devicetree.org/schemas/iio/dac/
> > >
> adi,ltc2688.yaml*__;Iw!!A3Ni8CS0y2Y!rHThZYvGYZfm2zOTRFsr1xH61Bf
> > > mq371ojtDKEdpTSeC7lCU_dS7CnRBJvPcEQ$
> > > > > +$schema:
> > > https://urldefense.com/v3/__http://devicetree.org/meta-
> > >
> schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!rHThZYvGYZfm2zOTRFsr1xH
> > > 61Bfmq371ojtDKEdpTSeC7lCU_dS7CnSFhKxW0w$
> > > > > +
> > > > > +title: Analog Devices LTC2688 DAC
> > > > > +
> > > > > +maintainers:
> > > > > + - Nuno Sá <nuno.sa@analog.com>
> > > > > +
> > > > > +description: |
> > > > > + Analog Devices LTC2688 16 channel, 16 bit, +-15V DAC
> > > > > + https://www.analog.com/media/en/technical-
> > > documentation/data-sheets/ltc2688.pdf
> > > > > +
> > > > > +properties:
> > > > > + compatible:
> > > > > + enum:
> > > > > + - adi,ltc2688
> > > > > +
> > > > > + reg:
> > > > > + maxItems: 1
> > > > > +
> > > > > + vcc-supply:
> > > > > + description: Analog Supply Voltage Input.
> > > > > +
> > > > > + iovcc-supply:
> > > > > + description: Digital Input/Output Supply Voltage.
> > > > > +
> > > > > + vref-supply:
> > > > > + description:
> > > > > + Reference Input/Output. The voltage at the REF pin sets
> the
> > > full-scale
> > > > > + range of all channels. By default, the internal reference is
> > > routed to
> > > > > + this pin.
> > > > > +
> > > > > + reset-gpios:
> > > > > + description:
> > > > > + If specified, it will be asserted during driver probe. As the
> line is
> > > > > + active low, it should be marked GPIO_ACTIVE_LOW.
> > > > > + maxItems: 1
> > > > > +
> > > > > + clocks:
> > > > > + minItems: 1
> > > > > + maxItems: 3
> > > > > +
> > > > > + clock-names:
> > > > > + minItems: 1
> > > > > + maxItems: 3
> > > > > + items:
> > > > > + enum: [TGP1, TGP2, TGP3]
> > > >
> > > > pattern: '^TGP[1-3]$'
> > > >
> > > > > +
> > > > > + '#address-cells':
> > > > > + const: 1
> > > > > +
> > > > > + '#size-cells':
> > > > > + const: 0
> > > > > +
> > > > > +patternProperties:
> > > > > + "^channel@([0-9]|1[0-5])$":
> > > > > + type: object
> > > > > +
> > > > > + properties:
> > > > > + reg:
> > > > > + description: The channel number representing the DAC
> > > output channel.
> > > > > + maximum: 15
> > > > > +
> > > > > + adi,toggle-mode:
> > > > > + description:
> > > > > + Set the channel as a toggle enabled channel. Toggle
> > > operation enables
> > > > > + fast switching of a DAC output between two different
> DAC
> > > codes without
> > > > > + any SPI transaction. It will result in a different ABI for the
> > > > > + channel.
> > > > > + type: boolean
> > > > > +
> > > > > + adi,output-range-millivolt:
> > > >
> > > > Not one of the defined units. Use '-microvolt'
> > >
> > > > > + description:
> > > > > + Specify the channel output full scale range. Allowed
> values
> > > are
> > > > > + <0 5000>
> > > > > + <0 10000>
> > > > > + <-5000 5000>
> > > > > + <-10000 10000>
> > > > > + <-15000 15000>
> > > >
> > > > Looks like constraints.
> > > >
> > > > items:
> > > > - enum: [ -15000, -10000, -5000, 0 ]
> > > > - enum: [ 5000, 10000, 15000 ]
> > > >
> > > > though that will need to all be x1000.
> > >
> > > also should be constrained to allowed combinations which probably
> > > means a oneOf construct.
> > >
> >
> > Exactly. AFICT, with Rob's suggestion things like <-15000 5000> would
> > be validated but not really possible and the driver does not allow it. I
> did
> > tried some stuff before sending this simplified form (constrains in
> description):
> >
> > ...
> > oneOf:
> > - items:
> > - const: 0
> > - enum: [5000, 10000]
> > - items:
> > - const: -5000
> > - const: 5000
> > ...
> >
> > Whiles things worked for <0 5000> and <0 10000>, they failed for <(-
> 5000) 5000>:
> >
> > "
> >
> next/Documentation/devicetree/bindings/iio/dac/adi,ltc2688.example
> .dt.y
> > aml: ltc2688@0: channel@1:adi,output-range-millivolt: 'oneOf'
> > conditional failed, one must be fixed:
> > 0 was expected
> > -5000 was expected
> > From schema: /home/nsa/work/linux-adi-
> > next/Documentation/devicetree/bindings/iio/dac/adi,ltc2688.yam"
> >
> > Trying this combination <0 (-5000)> led to:
> >
> > "
> >
> next/Documentation/devicetree/bindings/iio/dac/adi,ltc2688.example
> .dt.y
> > aml: ltc2688@0: channel@1:adi,output-range-microvolt: 'oneOf'
> > conditional failed, one must be fixed:
> > -5000 was expected
> > 4294962296 is not one of [5000, 10000]
> > From schema: /home/nsa/work/linux-adi-
> > next/Documentation/devicetree/bindings/iio/dac/adi,ltc2688.yaml
> > "
> >
> > it makes me feel that something is going on with signed/unsigned
> > comparisons. But I might be completely off with this approach :)
>
> I'll go with huh...
>
> Documentation/devicetree/bindings/iio/dac/adi,ad3552r.yaml
>
> and other places have pretty much the same construct though
> examples
> don't actually use it.
>
> I have some vague recollection of a previous discussion about
> negatives
> and that Rob had some plan to make them work long term. In the
> meantime
> we just avoid them in the examples.
>
> Rob, is my memory deceiving me?
>

Rob, any input on this? Should I just not add the property to the examples
for now?

- Nuno Sá