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

Sa, Nuno
2021-12-17T09:09:55Z

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

- Nuno Sá