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

[Jonathan Cameron <jic23@jic23.retrosnub.co.uk>]

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?

Thanks,

Jonathan
 
> 
> - Nuno Sá
>