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

["Sa, Nuno" <Nuno.Sa@xxxxxxxxxx>]

> From: Jonathan Cameron <jic23@xxxxxxxxxxxxxxxxxxxxx>
> Sent: Thursday, December 16, 2021 2:33 PM
> To: Rob Herring <robh@xxxxxxxxxx>
> Cc: Sa, Nuno <Nuno.Sa@xxxxxxxxxx>; linux-iio@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; Lars-Peter Clausen <lars@xxxxxxxxxx>;
> Hennerich, Michael <Michael.Hennerich@xxxxxxxxxx>
> Subject: Re: [PATCH 3/3] dt-bindings: iio: Add ltc2688 documentation
> 
> 
> On Wed, 15 Dec 2021 15:30:37 -0600
> Rob Herring <robh@xxxxxxxxxx> 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@xxxxxxxxxx>
> > > ---
> > >  .../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@xxxxxxxxxx>
> > > +
> > > +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á