[v2,1/2] dt-bindings: iio: addac: add AD74115

  From: Cosmin Tanislav <cosmin.tanislav@analog.com>

The AD74115H is a single-channel, software-configurable, input and
output device for industrial control applications. The AD74115H
provides a wide range of use cases, integrated on a single chip.

These use cases include analog output, analog input, digital output,
digital input, resistance temperature detector (RTD), and thermocouple
measurement capability. The AD74115H also has an integrated HART modem.

A serial peripheral interface (SPI) is used to handle all communications
to the device, including communications with the HART modem. The digital
input and digital outputs can be accessed via the SPI or the
general-purpose input and output (GPIO) pins to support higher
speed data rates.

The device features a 16-bit, sigma-delta analog-to-digital converter
(ADC) and a 14-bit digital-to-analog converter (DAC).
The AD74115H contains a high accuracy 2.5 V on-chip reference that can
be used as the DAC and ADC reference.

Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
---
 .../bindings/iio/addac/adi,ad74115.yaml       | 370 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 377 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml
  

On Thu,  3 Nov 2022 11:44:35 +0200
Cosmin Tanislav <demonsingur@gmail.com> wrote:

> From: Cosmin Tanislav <cosmin.tanislav@analog.com>
> 
> The AD74115H is a single-channel, software-configurable, input and
> output device for industrial control applications. The AD74115H
> provides a wide range of use cases, integrated on a single chip.
> 
> These use cases include analog output, analog input, digital output,
> digital input, resistance temperature detector (RTD), and thermocouple
> measurement capability. The AD74115H also has an integrated HART modem.
> 
> A serial peripheral interface (SPI) is used to handle all communications
> to the device, including communications with the HART modem. The digital
> input and digital outputs can be accessed via the SPI or the
> general-purpose input and output (GPIO) pins to support higher
> speed data rates.
> 
> The device features a 16-bit, sigma-delta analog-to-digital converter
> (ADC) and a 14-bit digital-to-analog converter (DAC).
> The AD74115H contains a high accuracy 2.5 V on-chip reference that can
> be used as the DAC and ADC reference.
> 
> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>

Hi Cosmin,

A few questions inline.  Complex device so I'll doubt we'll ever get this
binding to be as tidy as for simpler devices.  Hence most of the below are
suggestions rather than requirements from me.

Jonathan

> ---
>  .../bindings/iio/addac/adi,ad74115.yaml       | 370 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 377 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml b/Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml
> new file mode 100644
> index 000000000000..621f11d5c1f3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml
> @@ -0,0 +1,370 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/addac/adi,ad74115.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD74115H device
> +
> +maintainers:
> +  - Cosmin Tanislav <cosmin.tanislav@analog.com>
> +
> +description: |
> +  The AD74115H is a single-channel software configurable input/output
> +  device for industrial control applications. It contains functionality for
> +  analog output, analog input, digital output, digital input, resistance
> +  temperature detector, and thermocouple measurements integrated into a single
> +  chip solution with an SPI interface. The device features a 16-bit ADC and a
> +  14-bit DAC.
> +
> +    https://www.analog.com/en/products/ad74115h.html
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad74115h
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0

I'm not seeing any child nodes, so why do we need these two?

> +
> +  avdd-supply: true
> +  avcc-supply: true
> +  dvcc-supply: true
> +  aldo1v8-supply: true

aldo1v8 is an output pin. "1.8 V Analog LDO Output. Do not use ALDO1V8 externally."
The associated input is avcc.  Given we shouldn't connect anything to the pin,
we don't want it in the binding docs

> +  dovdd-supply: true
> +  refin-supply: true
> +

...

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Conversion range for ADC conversion 2.
> +      0 - 0V to 12V
> +      1 - -12V to +12V
> +      2 - -2.5V to +2.5V
> +      3 - -2.5V to 0V
> +      4 - 0V to 2.5V
> +      5 - 0V to 0.625V
> +      6 - -104mV to +104mV
> +      7 - 0V to 12V

For a lot of similar cases we handle these numerically to give
a human readable dts.  Is there a strong reason not to do so here (in mv)


> +    minimum: 0
> +    maximum: 7
> +    default: 0
> +
> +  adi,sense-agnd-buffer-lp:
lp is a little ambiguous, given we have a habit of using it for low pass
in filters etc. Perhaps worth spelling these out?
     adi,sens-agnd-buffer-low-power etc?

> +    type: boolean
> +    description: |
> +      Whether to enable low-power buffered mode for the AGND sense pin.
> +
> +  adi,lf-buffer-lp:
> +    type: boolean
> +    description: |
> +      Whether to enable low-power buffered mode for the low-side filtered
> +      sense pin.
> +
> +  adi,hf-buffer-lp:
> +    type: boolean
> +    description: |
> +      Whether to enable low-power buffered mode for the high-side filtered
> +      sense pin.
> +
> +  adi,ext2-buffer-lp:
> +    type: boolean
> +    description: Whether to enable low-power buffered mode for the EXT2 pin.
> +
> +  adi,ext1-buffer-lp:
> +    type: boolean
> +    description: Whether to enable low-power buffered mode for the EXT1 pin.

> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-cpol
> +  - avdd-supply
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - if:
> +      properties:
> +        adi,digital-input-sink-range-high: true
> +    then:
> +      properties:
> +        adi,digital-input-sink-microamp:
> +          maximum: 7400
> +
> +additionalProperties: false

Does this need to be unevalutatedProperties to allow
for the extra ones in spi-periphera-props.yaml?

> +
  

On Thu, Nov 03, 2022 at 11:44:35AM +0200, Cosmin Tanislav wrote:
> From: Cosmin Tanislav <cosmin.tanislav@analog.com>
> 
> The AD74115H is a single-channel, software-configurable, input and
> output device for industrial control applications. The AD74115H
> provides a wide range of use cases, integrated on a single chip.
> 
> These use cases include analog output, analog input, digital output,
> digital input, resistance temperature detector (RTD), and thermocouple
> measurement capability. The AD74115H also has an integrated HART modem.
> 
> A serial peripheral interface (SPI) is used to handle all communications
> to the device, including communications with the HART modem. The digital
> input and digital outputs can be accessed via the SPI or the
> general-purpose input and output (GPIO) pins to support higher
> speed data rates.
> 
> The device features a 16-bit, sigma-delta analog-to-digital converter
> (ADC) and a 14-bit digital-to-analog converter (DAC).
> The AD74115H contains a high accuracy 2.5 V on-chip reference that can
> be used as the DAC and ADC reference.
> 
> Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
> ---
>  .../bindings/iio/addac/adi,ad74115.yaml       | 370 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 377 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml b/Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml
> new file mode 100644
> index 000000000000..621f11d5c1f3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml
> @@ -0,0 +1,370 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/addac/adi,ad74115.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD74115H device
> +
> +maintainers:
> +  - Cosmin Tanislav <cosmin.tanislav@analog.com>
> +
> +description: |
> +  The AD74115H is a single-channel software configurable input/output
> +  device for industrial control applications. It contains functionality for
> +  analog output, analog input, digital output, digital input, resistance
> +  temperature detector, and thermocouple measurements integrated into a single
> +  chip solution with an SPI interface. The device features a 16-bit ADC and a
> +  14-bit DAC.
> +
> +    https://www.analog.com/en/products/ad74115h.html
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad74115h
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  spi-max-frequency:
> +    maximum: 24000000
> +
> +  spi-cpol: true
> +
> +  reset-gpios: true
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  avdd-supply: true
> +  avcc-supply: true
> +  dvcc-supply: true
> +  aldo1v8-supply: true
> +  dovdd-supply: true
> +  refin-supply: true
> +
> +  adi,ch-func:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Channel function.
> +      0 - High impedance
> +      1 - Voltage output
> +      2 - Current output
> +      3 - Voltage input
> +      4 - Current input, externally-powered
> +      5 - Current input, loop-powered
> +      6 - Resistance input
> +      7 - RTD measure
> +      8 - Digital input logic
> +      9 - Digital input, loop-powered
> +      10 - Current output with HART
> +      11 - Current input, externally-powered, with HART
> +      12 - Current input, loop-powered, with HART
> +    minimum: 0
> +    maximum: 12
> +    default: 0
> +
> +  adi,conv2-mux:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Input node for ADC conversion 2.
> +      0 - SENSE_EXT1 to AGND_SENSE
> +      1 - SENSE_EXT2 to AGND_SENSE
> +      2 - SENSE_EXT2 to SENSE_EXT1
> +      3 - AGND to AGND
> +    minimum: 0
> +    maximum: 3
> +    default: 0
> +
> +  adi,conv2-range:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Conversion range for ADC conversion 2.
> +      0 - 0V to 12V
> +      1 - -12V to +12V
> +      2 - -2.5V to +2.5V
> +      3 - -2.5V to 0V
> +      4 - 0V to 2.5V
> +      5 - 0V to 0.625V
> +      6 - -104mV to +104mV
> +      7 - 0V to 12V
> +    minimum: 0
> +    maximum: 7
> +    default: 0
> +
> +  adi,sense-agnd-buffer-lp:
> +    type: boolean
> +    description: |

Don't need '|' if no formatting to preserve.

> +      Whether to enable low-power buffered mode for the AGND sense pin.
> +
> +  adi,lf-buffer-lp:
> +    type: boolean
> +    description: |
> +      Whether to enable low-power buffered mode for the low-side filtered
> +      sense pin.
> +
> +  adi,hf-buffer-lp:
> +    type: boolean
> +    description: |
> +      Whether to enable low-power buffered mode for the high-side filtered
> +      sense pin.
> +
> +  adi,ext2-buffer-lp:
> +    type: boolean
> +    description: Whether to enable low-power buffered mode for the EXT2 pin.
> +
> +  adi,ext1-buffer-lp:
> +    type: boolean
> +    description: Whether to enable low-power buffered mode for the EXT1 pin.
> +
> +  adi,comparator-invert:
> +    type: boolean
> +    description: Whether to invert the comparator output.
> +
> +  adi,digital-input-sink-range-high:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      When not present, the digital input range is from 0 to 3700uA in steps
> +      of 120uA, with a ~2k series resistance.
> +      When present, the digital input range is from 0 to 7400uA in steps
> +      of 240uA, with a ~1k series resistance.
> +
> +  adi,digital-input-sink-microamp:
> +    description: Sink current in digital input mode.
> +    minimum: 0
> +    maximum: 3700
> +    default: 0
> +
> +  adi,digital-input-debounce-mode-counter-reset:
> +    type: boolean
> +    description: |
> +      When not present, a counter increments when the signal is asserted
> +      and decrements when the signal is de-asserted.
> +      When present, a counter increments while the signal is asserted and
> +      resets when the signal de-asserts
> +
> +  adi,digital-input-unbuffered:
> +    type: boolean
> +    description: Whether to buffer digital input signals.
> +
> +  adi,digital-input-short-circuit-detection:
> +    type: boolean
> +    description: Whether to detect digital input short circuits.
> +
> +  adi,digital-input-open-circuit-detection:
> +    type: boolean
> +    description: Whether to detect digital input open circuits.
> +
> +  adi,digital-input-threshold-mode-fixed:
> +    type: boolean
> +    description: |
> +      When not present, the digital input threshold range is -0.96 * AVDD
> +      to AVDD.
> +      When present, the threshold range is fixed from -19V to 30V.
> +
> +  adi,dac-bipolar:
> +    type: boolean
> +    description: |
> +      When not present, the DAC operates in the 0V to 12V range.
> +      When present, the DAC operates in the -12V to 12V range.
> +
> +  adi,charge-pump:
> +    type: boolean
> +    description: Whether to enable the internal charge pump.
> +
> +  adi,dac-hart-slew:
> +    type: boolean
> +    description: Whether to use a HART-compatible slew rate.
> +
> +  adi,dac-current-limit-low:
> +    type: boolean
> +    description: |
> +      When not present, the DAC short-circuit current limit is 32mA in
> +      either source or sink for VOUT and 4mA sink for IOUT.
> +      When present, the limit is 16mA in either source or sink for VOUT,
> +      1mA sink for IOUT.
> +
> +  adi,4-wire-rtd:
> +    type: boolean
> +    description: |
> +      When not present, the ADC should be used for measuring 3-wire RTDs.
> +      When present, the ADC should be used for measuring 4-wire RTDs.
> +
> +  adi,3-wire-rtd-excitation-swap:
> +    type: boolean
> +    description: Whether to swap the excitation for 3-wire RTD.
> +
> +  adi,rtd-excitation-current-microamp:
> +    description: Excitation current to apply to RTD.
> +    enum: [250, 500, 750, 1000]
> +    default: 250
> +
> +  adi,ext1-burnout:
> +    type: boolean
> +    description: Whether to enable burnout current for EXT1.
> +
> +  adi,ext1-burnout-current-nanoamp:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Burnout current in nanoamps to be applied to EXT1.
> +    enum: [0, 50, 500, 1000, 10000]
> +    default: 0
> +
> +  adi,ext1-burnout-current-polarity-sourcing:
> +    type: boolean
> +    description: |
> +      When not present, the burnout current polarity for EXT1 is sinking.
> +      When present, the burnout current polarity for EXT1 is sourcing.
> +
> +  adi,ext2-burnout:
> +    type: boolean
> +    description: Whether to enable burnout current for EXT2.
> +
> +  adi,ext2-burnout-current-nanoamp:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Burnout current in nanoamps to be applied to EXT2.
> +    enum: [0, 50, 500, 1000, 10000]
> +    default: 0
> +
> +  adi,ext2-burnout-current-polarity-sourcing:
> +    type: boolean
> +    description: |
> +      When not present, the burnout current polarity for EXT2 is sinking.
> +      When present, the burnout current polarity for EXT2 is sourcing.
> +
> +  adi,viout-burnout:
> +    type: boolean
> +    description: Whether to enable burnout current for VIOUT.
> +
> +  adi,viout-burnout-current-nanoamp:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Burnout current in nanoamps to be applied to VIOUT.
> +    enum: [0, 1000, 10000]
> +    default: 0
> +
> +  adi,viout-burnout-current-polarity-sourcing:
> +    type: boolean
> +    description: |
> +      When not present, the burnout current polarity for VIOUT is sinking.
> +      When present, the burnout current polarity for VIOUT is sourcing.
> +
> +  adi,gpio0-mode:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      GPIO functions.
> +      0 - Disabled
> +      1 - Logic I/O
> +      2 - Comparator output
> +      3 - Control HART CD
> +      4 - Monitor HART CD
> +      5 - Monitor HART EOM status
> +    minimum: 0
> +    maximum: 5
> +    default: 0
> +
> +  adi,gpio1-mode:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      GPIO functions.
> +      0 - Disabled
> +      1 - Logic I/O
> +      2 - Drive external digital output FET
> +      3 - Control HART RXD
> +      4 - Monitor HART RXD
> +      5 - Monitor HART SOM status
> +    minimum: 0
> +    maximum: 5
> +    default: 0
> +
> +  adi,gpio2-mode:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      GPIO functions.
> +      0 - Disabled
> +      1 - Logic I/O
> +      2 - Drive internal digital output FET
> +      3 - Control HART TXD
> +      4 - Monitor HART TXD
> +      5 - Monitor HART TX complete status
> +    minimum: 0
> +    maximum: 5
> +    default: 0
> +
> +  adi,gpio3-mode:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      GPIO functions.
> +      0 - Disabled
> +      1 - Logic I/O
> +      2 - High impedance
> +      3 - Control HART RTS
> +      4 - Monitor HART RTS
> +      5 - Monitor HART CD complete status
> +    minimum: 0
> +    maximum: 5
> +    default: 0
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-cpol
> +  - avdd-supply
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - if:
> +      properties:
> +        adi,digital-input-sink-range-high: true

The 'if' is true if adi,digital-input-sink-range-high is not present 
which is probably not what you want. Use 'required' instead.

> +    then:
> +      properties:
> +        adi,digital-input-sink-microamp:
> +          maximum: 7400
> +
> +additionalProperties: false

As pointed out, yes, this needs to be unevaluatedProperties.

> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      addac@0 {
> +        compatible = "adi,ad74115h";
> +        reg = <0>;
> +
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        spi-max-frequency = <12000000>;
> +        spi-cpol;
> +
> +        reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
> +
> +        interrupt-parent = <&gpio>;
> +        interrupts = <26 IRQ_TYPE_EDGE_FALLING>;
> +
> +        avdd-supply = <&ad74115_avdd>;
> +
> +        adi,ch-func = <1>;
> +        adi,conv2-mux = <2>;
> +        adi,conv2-range = <1>;
> +
> +        adi,gpio0-mode = <1>;
> +        adi,gpio1-mode = <1>;
> +        adi,gpio2-mode = <1>;
> +        adi,gpio3-mode = <1>;
> +
> +        adi,dac-bipolar;
> +      };
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7a56c344ca67..5b5533f54f67 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1175,6 +1175,13 @@ W:	https://ez.analog.com/linux-software-drivers
>  F:	Documentation/devicetree/bindings/iio/adc/adi,ad7780.yaml
>  F:	drivers/iio/adc/ad7780.c
>  
> +ANALOG DEVICES INC AD74115 DRIVER
> +M:	Cosmin Tanislav <cosmin.tanislav@analog.com>
> +L:	linux-iio@vger.kernel.org
> +S:	Supported
> +W:	http://ez.analog.com/community/linux-device-drivers
> +F:	Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml
> +
>  ANALOG DEVICES INC AD74413R DRIVER
>  M:	Cosmin Tanislav <cosmin.tanislav@analog.com>
>  L:	linux-iio@vger.kernel.org
> -- 
> 2.38.1
> 
>
  

On Sun, 2022-11-06 at 15:46 +0000, Jonathan Cameron wrote:
> On Thu,  3 Nov 2022 11:44:35 +0200
> Cosmin Tanislav <demonsingur@gmail.com> wrote:
> 
> > From: Cosmin Tanislav <cosmin.tanislav@analog.com>
> > 
> > The AD74115H is a single-channel, software-configurable, input and
> > output device for industrial control applications. The AD74115H
> > provides a wide range of use cases, integrated on a single chip.
> > 
> > These use cases include analog output, analog input, digital output,
> > digital input, resistance temperature detector (RTD), and thermocouple
> > measurement capability. The AD74115H also has an integrated HART modem.
> > 
> > A serial peripheral interface (SPI) is used to handle all communications
> > to the device, including communications with the HART modem. The digital
> > input and digital outputs can be accessed via the SPI or the
> > general-purpose input and output (GPIO) pins to support higher
> > speed data rates.
> > 
> > The device features a 16-bit, sigma-delta analog-to-digital converter
> > (ADC) and a 14-bit digital-to-analog converter (DAC).
> > The AD74115H contains a high accuracy 2.5 V on-chip reference that can
> > be used as the DAC and ADC reference.
> > 
> > Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
> 
> Hi Cosmin,
> 
> A few questions inline.  Complex device so I'll doubt we'll ever get this
> binding to be as tidy as for simpler devices.  Hence most of the below are
> suggestions rather than requirements from me.
> 
> Jonathan
> 
> > ---
> >  .../bindings/iio/addac/adi,ad74115.yaml       | 370 ++++++++++++++++++
> >  MAINTAINERS                                   |   7 +
> >  2 files changed, 377 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml b/Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml
> > new file mode 100644
> > index 000000000000..621f11d5c1f3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml
> > @@ -0,0 +1,370 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/addac/adi,ad74115.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices AD74115H device
> > +
> > +maintainers:
> > +  - Cosmin Tanislav <cosmin.tanislav@analog.com>
> > +
> > +description: |
> > +  The AD74115H is a single-channel software configurable input/output
> > +  device for industrial control applications. It contains functionality for
> > +  analog output, analog input, digital output, digital input, resistance
> > +  temperature detector, and thermocouple measurements integrated into a single
> > +  chip solution with an SPI interface. The device features a 16-bit ADC and a
> > +  14-bit DAC.
> > +
> > +    https://www.analog.com/en/products/ad74115h.html
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,ad74115h
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  '#address-cells':
> > +    const: 1
> > +
> > +  '#size-cells':
> > +    const: 0
> 
> I'm not seeing any child nodes, so why do we need these two?
> 

Will fix.

> > +
> > +  avdd-supply: true
> > +  avcc-supply: true
> > +  dvcc-supply: true
> > +  aldo1v8-supply: true
> 
> aldo1v8 is an output pin. "1.8 V Analog LDO Output. Do not use ALDO1V8 externally."
> The associated input is avcc.  Given we shouldn't connect anything to the pin,
> we don't want it in the binding docs
> 

Will fix.

> > +  dovdd-supply: true
> > +  refin-supply: true
> > +
> 
> ...
> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +      Conversion range for ADC conversion 2.
> > +      0 - 0V to 12V
> > +      1 - -12V to +12V
> > +      2 - -2.5V to +2.5V
> > +      3 - -2.5V to 0V
> > +      4 - 0V to 2.5V
> > +      5 - 0V to 0.625V
> > +      6 - -104mV to +104mV
> > +      7 - 0V to 12V
> 
> For a lot of similar cases we handle these numerically to give
> a human readable dts.  Is there a strong reason not to do so here (in mv)
> 

I used this approach mostly because it maps dirrectly to register values
and because it's easier to parse. dts isn't exactly nice at handling
negative values. I can switch it to mv array if you insist.

> 
> > +    minimum: 0
> > +    maximum: 7
> > +    default: 0
> > +
> > +  adi,sense-agnd-buffer-lp:
> lp is a little ambiguous, given we have a habit of using it for low pass
> in filters etc. Perhaps worth spelling these out?
>      adi,sens-agnd-buffer-low-power etc?

Will fix.

> 
> > +    type: boolean
> > +    description: |
> > +      Whether to enable low-power buffered mode for the AGND sense pin.
> > +
> > +  adi,lf-buffer-lp:
> > +    type: boolean
> > +    description: |
> > +      Whether to enable low-power buffered mode for the low-side filtered
> > +      sense pin.
> > +
> > +  adi,hf-buffer-lp:
> > +    type: boolean
> > +    description: |
> > +      Whether to enable low-power buffered mode for the high-side filtered
> > +      sense pin.
> > +
> > +  adi,ext2-buffer-lp:
> > +    type: boolean
> > +    description: Whether to enable low-power buffered mode for the EXT2 pin.
> > +
> > +  adi,ext1-buffer-lp:
> > +    type: boolean
> > +    description: Whether to enable low-power buffered mode for the EXT1 pin.
> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - spi-cpol
> > +  - avdd-supply
> > +
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +  - if:
> > +      properties:
> > +        adi,digital-input-sink-range-high: true
> > +    then:
> > +      properties:
> > +        adi,digital-input-sink-microamp:
> > +          maximum: 7400
> > +
> > +additionalProperties: false
> 
> Does this need to be unevalutatedProperties to allow
> for the extra ones in spi-periphera-props.yaml?
> 

Will fix.

> > +
  

> >   
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description: |
> > > +      Conversion range for ADC conversion 2.
> > > +      0 - 0V to 12V
> > > +      1 - -12V to +12V
> > > +      2 - -2.5V to +2.5V
> > > +      3 - -2.5V to 0V
> > > +      4 - 0V to 2.5V
> > > +      5 - 0V to 0.625V
> > > +      6 - -104mV to +104mV
> > > +      7 - 0V to 12V  
> > 
> > For a lot of similar cases we handle these numerically to give
> > a human readable dts.  Is there a strong reason not to do so here (in mv)
> >   
> 
> I used this approach mostly because it maps dirrectly to register values
> and because it's easier to parse. dts isn't exactly nice at handling
> negative values. I can switch it to mv array if you insist.

We have quite a few existing cases of
adi,[output-]range-microvolt so it would be good to copy that style here.

> 
> >   
> > > +    minimum: 0
> > > +    maximum: 7
> > > +    default: 0
> > > +
  

On Sat, 2022-11-12 at 15:40 +0000, Jonathan Cameron wrote:
> > >   
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    description: |
> > > > +      Conversion range for ADC conversion 2.
> > > > +      0 - 0V to 12V
> > > > +      1 - -12V to +12V
> > > > +      2 - -2.5V to +2.5V
> > > > +      3 - -2.5V to 0V
> > > > +      4 - 0V to 2.5V
> > > > +      5 - 0V to 0.625V
> > > > +      6 - -104mV to +104mV
> > > > +      7 - 0V to 12V  
> > > 
> > > For a lot of similar cases we handle these numerically to give
> > > a human readable dts.  Is there a strong reason not to do so here (in mv)
> > >   
> > 
> > I used this approach mostly because it maps dirrectly to register values
> > and because it's easier to parse. dts isn't exactly nice at handling
> > negative values. I can switch it to mv array if you insist.
> 
> We have quite a few existing cases of
> adi,[output-]range-microvolt so it would be good to copy that style here.
> 

With this:

  adi,conv2-range-microvolt:
    description: Conversion range for ADC conversion 2.
    oneOf:
      - items:
          - enum: [-2500000, 0]
          - const: 2500000
      - items:
          - enum: [-12000000, 0]
          - const: 12000000
      - items:
          - const: -2500000
          - const: 0
      - items:
          - const: -104000
          - const: 104000
      - items:
          - const: 0
          - const: 625000

And this:

adi,conv2-range-microvolt = <(-12000000) 12000000>;

I get this:

Documentation/devicetree/bindings/iio/addac/adi,ad74115.example.dtb:
addac@0: adi,conv2-range-microvolt: 'oneOf' conditional failed,
one must be fixed:
        4282967296 is not one of [-2500000, 0]
        4282967296 is not one of [-12000000, 0]
        -2500000 was expected
        -104000 was expected
        625000 was expected
        From schema: Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml

As I said, negative numbers don't play too nice...

> > 
> > >   
> > > > +    minimum: 0
> > > > +    maximum: 7
> > > > +    default: 0
> > > > +
  

On Tue, Nov 15, 2022 at 10:07 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Tue, 15 Nov 2022 14:43:53 +0200
> Cosmin Tanislav <demonsingur@gmail.com> wrote:
>
> > On Sat, 2022-11-12 at 15:40 +0000, Jonathan Cameron wrote:
> > > > >
> > > > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > +    description: |
> > > > > > +      Conversion range for ADC conversion 2.
> > > > > > +      0 - 0V to 12V
> > > > > > +      1 - -12V to +12V
> > > > > > +      2 - -2.5V to +2.5V
> > > > > > +      3 - -2.5V to 0V
> > > > > > +      4 - 0V to 2.5V
> > > > > > +      5 - 0V to 0.625V
> > > > > > +      6 - -104mV to +104mV
> > > > > > +      7 - 0V to 12V
> > > > >
> > > > > For a lot of similar cases we handle these numerically to give
> > > > > a human readable dts.  Is there a strong reason not to do so here (in mv)
> > > > >
> > > >
> > > > I used this approach mostly because it maps dirrectly to register values
> > > > and because it's easier to parse. dts isn't exactly nice at handling
> > > > negative values. I can switch it to mv array if you insist.
> > >
> > > We have quite a few existing cases of
> > > adi,[output-]range-microvolt so it would be good to copy that style here.
> > >
> >
> > With this:
> >
> >   adi,conv2-range-microvolt:
> >     description: Conversion range for ADC conversion 2.
> >     oneOf:
> >       - items:
> >           - enum: [-2500000, 0]
> >           - const: 2500000
> >       - items:
> >           - enum: [-12000000, 0]
> >           - const: 12000000
> >       - items:
> >           - const: -2500000
> >           - const: 0
> >       - items:
> >           - const: -104000
> >           - const: 104000
> >       - items:
> >           - const: 0
> >           - const: 625000
> >
> > And this:
> >
> > adi,conv2-range-microvolt = <(-12000000) 12000000>;
> >
> > I get this:
> >
> > Documentation/devicetree/bindings/iio/addac/adi,ad74115.example.dtb:
> > addac@0: adi,conv2-range-microvolt: 'oneOf' conditional failed,
> > one must be fixed:
> >         4282967296 is not one of [-2500000, 0]
> >         4282967296 is not one of [-12000000, 0]
> >         -2500000 was expected
> >         -104000 was expected
> >         625000 was expected
> >         From schema: Documentation/devicetree/bindings/iio/addac/adi,ad74115.yaml
> >
> > As I said, negative numbers don't play too nice...
>
> From what I recall we just ignore those warnings :)
>
> Rob, do I remember correctly that there was a plan to make this work longer term?

Yes, but handling signed types is working now (since the move to
validating dtbs directly).

The issue here is -microvolt is defined as unsigned. IIRC, I had some
issue changing it, but I think that was just with the YAML encoding
which I intend to remove. I'll give it another look and update the
type if there's no issues.

Rob