[v3,01/10] dt-bindings: iio: adc: Add AD4170

Add device tree documentation for AD4170 and similar sigma-delta ADCs.
The AD4170 is a 24-bit, multichannel, sigma-delta ADC.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
Change log v2 -> v3

[device tree changes]
- Removed unneeded allOf.
- Removed occurences of adi,sensor-type type re-declaration.
- Created type for the AD4170 channels, allowing to avoid dt doc repetition.

 .../bindings/iio/adc/adi,ad4170.yaml          | 544 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 551 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml

On 5/13/25 7:33 AM, Marcelo Schmitt wrote:
> Add device tree documentation for AD4170 and similar sigma-delta ADCs.
> The AD4170 is a 24-bit, multichannel, sigma-delta ADC.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
> Change log v2 -> v3
> 
> [device tree changes]
> - Removed unneeded allOf.
> - Removed occurences of adi,sensor-type type re-declaration.
> - Created type for the AD4170 channels, allowing to avoid dt doc repetition.
> 
>  .../bindings/iio/adc/adi,ad4170.yaml          | 544 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 551 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
> new file mode 100644
> index 000000000000..0a06258b6631
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
> @@ -0,0 +1,544 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad4170.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD4170 and similar Analog to Digital Converters
> +
> +maintainers:
> +  - Marcelo Schmitt <marcelo.schmitt@analog.com>
> +
> +description: |
> +  Analog Devices AD4170 series of Sigma-delta Analog to Digital Converters.
> +  Specifications can be found at:
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4170-4.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4190-4.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4195-4.pdf
> +
> +$ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +$defs:
> +  ad4170-channel:
> +    type: object
> +    $ref: /schemas/iio/adc/adc.yaml#
> +    description:
> +      Common properties for configuring AD4170 channels.
> +
> +    properties:
> +      adi,reference-select:
> +        description: |
> +          Selects the reference source to use when converting on the specific
> +          channel. Valid values are:
> +          0: Differential reference voltage REFIN+ - REFIN−.
> +          1: Differential reference voltage REFIN2+ - REFIN2−.
> +          2: Internal 2.5V referece (REFOUT) relative to AVSS.
> +          3: Analog supply voltage (AVDD) relative AVSS.
> +        $ref: /schemas/types.yaml#/definitions/uint8
> +        enum: [0, 1, 2, 3]
Using strings instead of int for this and most of the other custom enums here
would make them self-documenting and easier to use.

> +
> +
> +  sensor-node:
> +    type: object
> +    $ref: '#/$defs/ad4170-channel'
> +    description:
> +      The AD4170 and similar designs have features to aid interfacing with weigh
> +      scale, RTD, and thermocouple sensors. Each of those sensor types requires
> +      either distinct wiring configuration or external circuitry for proper
> +      sensor operation and can use different AD4170 functionality on their
> +      setups. A key characteristic of those external sensors is that they must
> +      be excited either by voltage supply or by AD4170 excitation signals. The
> +      sensor can then be read through a pair of analog inputs. These properties
> +      describe external sensor circuitry connected to the ADC.
> +
> +    properties:
> +      reg:
> +        description:
> +          Channel number. Connects the sensor to the channel with this number
> +          of the device.
> +        minimum: 1
> +        maximum: 16
> +
> +      diff-channels:
> +        description:
> +          Defines the ADC input pins used to read sensor data. Only regular
> +          analog input pins can be used.
> +        items:
> +          enum: [0, 1, 2, 3, 4, 5, 6, 7, 8]
> +
> +      bipolar: true
> +
> +      adi,sensor-type:
> +        description: |
> +          Type of sensor connected to the device. Depending on the sensor type
> +          (weigh scale, RTD, or thermocouple) the values of sensor-node
> +          properties have slightly different constraints. This property
> +          specifies which particular external sensor is connected to the ADC so
> +          the sensor-node properties can be properly parsed and verified. The
> +          possible sensor types are:
> +          0: weigh scale;
> +          1: RTD;
> +          2: thermocouple.
> +        $ref: /schemas/types.yaml#/definitions/uint8
This property seems reduandant since it has to match the node name.

i.e. weighscale@... is is always adi,sensor-type = <0>; and so on.

> +
> +      adi,excitation-ac:
> +        type: boolean
> +        description:
> +          Whether the external sensor has to be AC or DC excited.
Description could be more clear than when omitted, it is DC excited.

> +
> +      adi,excitation-pins:
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        description:
> +          Pins used to excite the sensor or external circuit that contains the
> +          sensor. Thermocouples and RTD sensors are excited either with one
> +          current source or with a pair of current sources to minimize the
> +          excitation current mismatch and the excitation current drift matching
> +          on the ADC. E.g. <0>; <1>; <0 1>. Load cell weigh scales may be
> +          excited with one current source, a pair of excitation currents, or two
> +          pairs of excitation currents. When four pins are defined, the first
> +          two values specify the first pair and the last ones specify the second
> +          pair of excitation currents. E.g. <0>; <0 1>; <0 1 2 3>.
> +        items:
> +          minimum: 0
> +          maximum: 20
> +
> +      adi,excitation-current-microamp:
> +        description:
> +          Excitation current in microamperes to be output to each excitation pin
> +          specified by adi,excitation-pins property. If not provided and
> +          adi,excitation-ac is true, use predefined ACX1, ACX1 negated, ACX2,
> +          and ACX2 negated signals to AC excite the bridge circuit. Those
> +          singals are output on GPIO2, GPIO0, GPIO3, and GPIO1, respectively.
> +        enum: [0, 10, 50, 100, 250, 500, 1000, 1500]
> +        default: 0
> +
> +      adi,power-down-switch-pin:
> +        description:
> +          Number of the GPIO used as power-down switch for the bridge circuit.
> +        $ref: /schemas/types.yaml#/definitions/uint8
> +        enum: [0, 1]
This isn't required, so what is the default if omitted?

> +
> +      adi,vbias:
> +        type: boolean
> +        description:
> +          For unbiased thermocouple applications, the voltage generated by the
> +          thermocouple must be biased around some DC voltage. When present, this
> +          property specifies a bias voltage of (AVDD + AVSS)/2 to be applied as
> +          common-mode voltage for the sensor.
> +
> +    required:
> +      - reg
> +      - diff-channels
> +      - bipolar
> +      - adi,sensor-type
> +      - adi,reference-select
> +
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad4170
> +      - adi,ad4190
> +      - adi,ad4195
> +
> +  avss-supply:
> +    description:
> +      Referece voltage supply for AVSS. If provided, describes the magnitude
s/Referece/Reference/

> +      (absolute value) of the negative voltage supplied to the AVSS pin. Since
> +      AVSS must be −2.625V minimum and 0V maximum, the declared supply voltage
> +      must be between 0 and 2.65V. If not provided, AVSS is assumed to be at
> +      system ground (0V).
> +
> +  avdd-supply:
> +    description:
> +      A supply of 4.75V to 5.25V relative to AVSS that powers the chip (AVDD).
> +
> +  iovdd-supply:
> +    description: 1.7V to 5.25V reference supply to the serial interface (IOVDD).
> +
> +  refin1p-supply:
> +    description: REFIN+ supply that can be used as reference for conversion.
> +
> +  refin1n-supply:
> +    description: REFIN- supply that can be used as reference for conversion. If
> +      provided, describes the magnitude (absolute value) of the negative voltage
> +      supplied to the REFIN- pin.
> +
> +  refin2p-supply:
> +    description: REFIN2+ supply that can be used as reference for conversion.
> +
> +  refin2n-supply:
> +    description: REFIN2- supply that can be used as reference for conversion. If
> +      provided, describes the magnitude (absolute value) of the negative voltage
> +      supplied to the REFIN2- pin.
> +
> +  spi-cpol: true
> +
> +  spi-cpha: true
> +
> +  interrupts:
> +    maxItems: 1> +
> +  interrupt-names:
> +    description:
> +      Specify which pin should be configured as Data Ready interrupt.
> +    enum:
> +      - sdo
> +      - dig_aux1
> +    default: sdo> +
> +  clocks:
> +    maxItems: 1
> +    description:
> +      Optional external clock source. Can specify either an external clock or
> +      external crystal.
> +
> +  clock-names:
> +    enum:
> +      - ext-clk
> +      - xtal
> +    default: ext-clk
Shouldn't there be a depedency that if clocks is given, then clock-names is requried.

> +
> +  '#clock-cells':
> +    const: 0
> +
> +  clock-output-names:
> +    maxItems: 1
And if #clock-cells is given then clocks is forbidden.

> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +    description: |
> +      The first cell is for the GPIO number: 0 to 3.
> +      The second cell takes standard GPIO flags.
> +
> +  ldac-gpios:
> +    description:
> +      GPIO connected to DIG_AUX2 pin to be used as LDAC toggle to control the
> +      transfer of data from the DAC_INPUT_A register to the DAC.
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +patternProperties:
> +  "^channel@[0-9a-f]$":
> +    $ref: '#/$defs/ad4170-channel'
> +    unevaluatedProperties: false
> +    description:
> +      Represents the external channels which are connected to the ADC.
> +
> +    properties:
> +      reg:
> +        description:
> +          The channel number.
> +        minimum: 0
> +        maximum: 15
> +
> +      diff-channels:
> +        description: |
> +          This property is used for defining the inputs of a differential
> +          voltage channel. The first value is the positive input and the second
> +          value is the negative input of the channel.
> +
> +          Besides the analog input pins AIN0 to AIN8, there are special inputs
> +          that can be selected with the following values:
> +          17: Internal temperature sensor
> +          18: (AVDD-AVSS)/5
> +          19: (IOVDD-DGND)/5
> +          20: DAC output
> +          21: ALDO
> +          22: DLDO
> +          23: AVSS
> +          24: DGND
> +          25: REFIN+
> +          26: REFIN-
> +          27: REFIN2+
> +          28: REFIN2-
> +          29: REFOUT
> +          For the internal temperature sensor, use the input number for both
> +          inputs (i.e. diff-channels = <17 17>).
> +        items:
> +          enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 17, 18, 19, 20, 21, 22, 23, 24, 25,
> +                 26, 27, 28, 29]

A Header file with macros for these would be nice since it seems like we
have to use the higher-numbered ones a lot with the common-mode-channel
properties in the examples.

> +
> +      single-channel: true
> +
> +      common-mode-channel: true
> +
> +      bipolar: true
> +
> +      adi,buffered-positive:
> +        description: |
> +          Enable precharge buffer, full buffer, or skip reference buffering of
> +          the positive voltage reference. Because the output impedance of the
> +          source driving the voltage reference inputs may be dynamic, RC
> +          combinations of those inputs can cause DC gain errors if the reference
> +          inputs go unbuffered into the ADC. Enable reference buffering if the
> +          provided reference source has dynamic high impedance output. Note the
> +          absolute voltage allowed on positive reference inputs (REFIN+,
> +          REFIN2+) is from AVSS − 50 mV to AVDD + 50 mV when the reference
> +          buffers are disabled but narrows to AVSS to AVDD when reference
> +          buffering is enabled or in precharge mode.
> +          0: Reference precharge buffer.
> +          1: Full Buffer.
> +          2: Bypass reference buffers (buffering disabled).
> +        $ref: /schemas/types.yaml#/definitions/uint8
> +        enum: [0, 1, 2]
> +        default: 1
> +
> +      adi,buffered-negative:
> +        description: |
> +          Enable precharge buffer, full buffer, or skip reference buffering of
> +          the negative voltage reference. Because the output impedance of the
> +          source driving the voltage reference inputs may be dynamic, RC
> +          combinations of those inputs can cause DC gain errors if the reference
> +          inputs go unbuffered into the ADC. Enable reference buffering if the
> +          provided reference source has dynamic high impedance output. Note the
> +          absolute voltage allowed on negative reference inputs (REFIN-,
> +          REFIN2-) is from AVSS − 50 mV to AVDD + 50 mV when the reference
> +          buffers are disabled but narrows to AVSS to AVDD when reference
> +          buffering is enabled or in precharge mode.
> +          0: Reference precharge buffer.
> +          1: Full Buffer.
> +          2: Bypass reference buffers (buffering disabled).
> +        $ref: /schemas/types.yaml#/definitions/uint8
> +        enum: [0, 1, 2]
> +        default: 1
Could make a $def for these too to reduce duplication.

Also another case where string type would make more sense.

> +
> +    required:
> +      - reg

reg is already required by adc.yaml

> +
> +    oneOf:
> +      - required: [single-channel]

Is there a default for common-mode-channel if it isn't required in this case?

> +        properties:
> +          diff-channels: false
> +      - required: [diff-channels]
> +        properties:
> +          single-channel: false
> +          common-mode-channel: false
> +
> +  "^weighscale@":
> +    $ref: '#/$defs/sensor-node'
> +    unevaluatedProperties: false
> +
> +    properties:
> +      diff-channels: true
> +      bipolar: true
> +
> +      adi,sensor-type:
> +        description: Weigh scale sensor.
> +        const: 0
> +
> +      adi,excitation-pins: true
> +
> +  "^rtd@":
> +    $ref: '#/$defs/sensor-node'
> +    unevaluatedProperties: false
> +
> +    properties:
> +      diff-channels: true
> +      bipolar: true
> +
> +      adi,sensor-type:
> +        description: RTD sensor.
> +        const: 1
> +
> +      adi,excitation-pins: true
> +
> +      adi,excitation-current-microamp: true
> +
> +    required:
> +      - adi,excitation-pins
> +      - adi,excitation-current-microamp
> +
> +  "^thermocouple@":
> +    $ref: '#/$defs/sensor-node'
> +    unevaluatedProperties: false
> +
> +    properties:
> +      diff-channels: true
> +      bipolar: true
> +
> +      adi,sensor-type:
> +        description: Thermocouple sensor.
> +        const: 2
> +
> +    required:
> +      - adi,excitation-pins
> +      - adi,excitation-current-microamp
> +
> +required:
> +  - compatible
> +  - reg
> +  - avdd-supply
> +  - iovdd-supply
> +  - spi-cpol
> +  - spi-cpha
> +
> +allOf:
> +  # Some devices don't have integrated DAC
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,ad4190
> +              - adi,ad4195
> +    then:
> +      properties:
> +        ldac-gpios: false
> +
> +unevaluatedProperties: false
> +

The patternProperties: section is so big, it would be nice to move these
before it so the are closer to the properties they actually affect.

On 5/16/25 10:45 AM, Marcelo Schmitt wrote:
> ...
>>> +
>>> +    properties:
>>> +      adi,reference-select:
>>> +        description: |
>>> +          Selects the reference source to use when converting on the specific
>>> +          channel. Valid values are:
>>> +          0: Differential reference voltage REFIN+ - REFIN−.
>>> +          1: Differential reference voltage REFIN2+ - REFIN2−.
>>> +          2: Internal 2.5V referece (REFOUT) relative to AVSS.
>>> +          3: Analog supply voltage (AVDD) relative AVSS.
>>> +        $ref: /schemas/types.yaml#/definitions/uint8
>>> +        enum: [0, 1, 2, 3]
>> Using strings instead of int for this and most of the other custom enums here
>> would make them self-documenting and easier to use.
> 
> The numbers match the values that are documented in the datasheet for each
> option of voltage reference available to use with a channel. So we would be
> using numbers mostly to define values of some unit and pin numbers (e.g. 100 for
> the microamp property)? Not really excited about doing this change because I
> think it will make the dtb a bit larger and the driver code a bit more lengthy,
> but can do that for v4.

I don't think it is too bad since we have match_string() to convert the strings
to an enum value. So it would just be a matter of adding the string tables.

But I don't feel terribly strongly about it anyway.

> ...
>>> +
>>> +      adi,power-down-switch-pin:
>>> +        description:
>>> +          Number of the GPIO used as power-down switch for the bridge circuit.
>>> +        $ref: /schemas/types.yaml#/definitions/uint8
>>> +        enum: [0, 1]
>> This isn't required, so what is the default if omitted?
> 
> We don't care about it when the property is omitted.
> Do we need a default even when the property is not required and we don't care
> when it's not set?

Ah, in that case, maybe add a bit to the description to say that this
is omitted when there isn't a bridge circuit wired up.

> 
> ...
>>> +      diff-channels:
>>> +        description: |
>>> +          This property is used for defining the inputs of a differential
>>> +          voltage channel. The first value is the positive input and the second
>>> +          value is the negative input of the channel.
>>> +
>>> +          Besides the analog input pins AIN0 to AIN8, there are special inputs
>>> +          that can be selected with the following values:
>>> +          17: Internal temperature sensor
>>> +          18: (AVDD-AVSS)/5
>>> +          19: (IOVDD-DGND)/5
>>> +          20: DAC output
>>> +          21: ALDO
>>> +          22: DLDO
>>> +          23: AVSS
>>> +          24: DGND
>>> +          25: REFIN+
>>> +          26: REFIN-
>>> +          27: REFIN2+
>>> +          28: REFIN2-
>>> +          29: REFOUT
>>> +          For the internal temperature sensor, use the input number for both
>>> +          inputs (i.e. diff-channels = <17 17>).
>>> +        items:
>>> +          enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 17, 18, 19, 20, 21, 22, 23, 24, 25,
>>> +                 26, 27, 28, 29]
>>
>> A Header file with macros for these would be nice since it seems like we
>> have to use the higher-numbered ones a lot with the common-mode-channel
>> properties in the examples.
> 
> The RFC set had a header with macros for those numbers, but making dt properties
> "look nice" was said to no be a reason to have binding headers. 
> 
> https://lore.kernel.org/linux-iio/ikq55kcfu2lmxzeeobu4zwf67xypyikadnpycw2m4d7o6gvmi2@tkepvcvzqzoh/
> 

Hmm, OK I never got that complaint before. Although the headers I have made before
were defining arbitrary numbers for phandle cells, and not something from the
datasheet like this.

> Also, no other binding would use those values. So, we would have a header
> specific for adi,ad4170?

Yes.

> 
>>
>>> +
>>> +      single-channel: true
>>> +
>>> +      common-mode-channel: true
>>> +
>>> +      bipolar: true
>>> +
>>> +      adi,buffered-positive:
>>> +        description: |
>>> +          Enable precharge buffer, full buffer, or skip reference buffering of
>>> +          the positive voltage reference. Because the output impedance of the
>>> +          source driving the voltage reference inputs may be dynamic, RC
>>> +          combinations of those inputs can cause DC gain errors if the reference
>>> +          inputs go unbuffered into the ADC. Enable reference buffering if the
>>> +          provided reference source has dynamic high impedance output. Note the
>>> +          absolute voltage allowed on positive reference inputs (REFIN+,
>>> +          REFIN2+) is from AVSS − 50 mV to AVDD + 50 mV when the reference
>>> +          buffers are disabled but narrows to AVSS to AVDD when reference
>>> +          buffering is enabled or in precharge mode.
>>> +          0: Reference precharge buffer.
>>> +          1: Full Buffer.
>>> +          2: Bypass reference buffers (buffering disabled).
>>> +        $ref: /schemas/types.yaml#/definitions/uint8
>>> +        enum: [0, 1, 2]
>>> +        default: 1
>>> +
>>> +      adi,buffered-negative:
>>> +        description: |
>>> +          Enable precharge buffer, full buffer, or skip reference buffering of
>>> +          the negative voltage reference. Because the output impedance of the
>>> +          source driving the voltage reference inputs may be dynamic, RC
>>> +          combinations of those inputs can cause DC gain errors if the reference
>>> +          inputs go unbuffered into the ADC. Enable reference buffering if the
>>> +          provided reference source has dynamic high impedance output. Note the
>>> +          absolute voltage allowed on negative reference inputs (REFIN-,
>>> +          REFIN2-) is from AVSS − 50 mV to AVDD + 50 mV when the reference
>>> +          buffers are disabled but narrows to AVSS to AVDD when reference
>>> +          buffering is enabled or in precharge mode.
>>> +          0: Reference precharge buffer.
>>> +          1: Full Buffer.
>>> +          2: Bypass reference buffers (buffering disabled).
>>> +        $ref: /schemas/types.yaml#/definitions/uint8
>>> +        enum: [0, 1, 2]
>>> +        default: 1
>> Could make a $def for these too to reduce duplication.
> 
> I think so, but how? They are only documented here. I can merge them into a
> single adi,buffered property. That will also reduce duplication.

You already have $defs:, so just add precharge-buffer: there with
the description:, etc., then here, just:

	adi,buffered-positive:
	  $ref: '#/$defs/precharge-buffer'
	adi,buffered-negative:
	  $ref: '#/$defs/precharge-buffer'

On Thu, 22 May 2025 12:07:41 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> ...
> >   
> > > +  sensor-node:
> > > +    type: object
> > > +    $ref: '#/$defs/ad4170-channel'  
> > 
> > I do not understand this binding. channel@ node is a channel and sensors
> > like rtd@ is also channel but also sensor. What is the point of channel@
> > which is not a sensor?
> >   
> The sensor node is meant to describe a channel that has extra setup
> configuration. For example, a common ADC channel could look like this
> 
>   -------- +VREF ------         +-------------------+
>     ´ `       ´ `              /                    |
>   /     \   /     \   /   --- <  IN+                |
>          `-´       `-´         |                    |
>   -------- -VREF ------        |                    |
>                                |            ADC     |
>   -------- +VREF ------        |                    |
>         ´ `       ´ `          |                    |
>   \   /     \   /     \   --- <  IN-                |
>    `-´       `-´               \       +VREF  -VREF |
>   -------- -VREF ------         +-------------------+
>                                          ^       ^
>                                          |       +---- External -VREF
>                                   External +VREF
> 
> The the channel@ node for that would look like
>     adc@0 {
>         ...
>         channel@0 {
>             reg = <0>;
>             bipolar;
>             diff-channels = <0 1>;
>         };
>     };
> 
> Though, some sigma-delta ADCs (including AD4170) are fancy and have features
> that relate to what is connected to the ADC inputs. For example, an RTD would
> be connected like
> 
>                                      External +VREF
>                                           |
>        +----------------------------------|-------+---- External -VREF
>        |           +----------------------+       |
>        +---Rref----+                +-----v-------v----+
>        |           |               /    REFIN+  REFIN- |
>    +---+-----------|------------> <  IN+               |
>    |               |              |                    |
> 3-wire RTD         |              |            ADC     |
>    |               |              |                    |
>    +--------------------+-------> <  IN-               |
>    |               |    |          \      GPIO2 GPIO3  |
>    v               |    |           +-------v-----v----+
>   GND              |    +-------------------+     |
>                    |       <--- IOUT1             |
>                    +------------------------------+
>                            <--- IOUT0
> 
> A better drawing can be found in AD4170 datasheet Figure 115. 3-Wire RTD Application.
> https://www.analog.com/media/en/technical-documentation/data-sheets/ad4170-4.pdf#unique_151_Connect_42_ID10430
> 
> Since the RTD sensor requires additional hardware connections, the proposed
> dt-binding describes those differently, e.g. 
> 
>     adc@0 {
>         ...
>         rtd@0 {
>             reg = <0>;
>             bipolar;
>             diff-channels = <0 1>;
>             adi,sensor-type = /bits/ 8 <1>;
>             adi,reference-select = <0>;
>             adi,excitation-pins = <19 20>;
>             adi,excitation-current-microamp = <500>;
>         };
> 
> That allows the ADC chip to be configured to provide the excitation signals to
> properly handle the RTD sensor. Because the hardware connections and
> operation requirements vary among those external sensor types (RTDs,
> thermocouples, load cell weigh scales), some properties are only applicable to
> specific sensor types. Also, because those are extra connections, the related
> properties are not meaningful to typical ADC channels.
> 
> Though, the supported features are not different from those described in
> Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml.
> In that binding, the properties related to external sensor support are set
> on the device node (not related to any channel). The binding proposed on AD4170
> RFC patch was more similar to adi,ad4130.yaml and somehow avoiding to introduce
> custom/new properties, but that lead to a lot of duplication.
> 
> The currently proposed dt-binding for ad4170 avoids repetition and, to some
> degree, tries to provide better description of connected bridge sensors and
> their related properties as that seemed to be one of Jonathan's suggestions
> to the RFC version.
> https://lore.kernel.org/linux-iio/20241219140353.787ffccc@jic23-huawei/

I don't always make good suggestions :(  DT maintainers have more experience
in making coherent and predictable bindings than I do.

One of the big problems is that we are fishing in the dark when trying to
make up 'generic' bindings.  Sometimes they just end up not extending well
at all.

> 
> Even though I'm naturally biased in favor of my own code, I see this is fairly
> different from bindings describing the exact same features and that can lead
> to confusion. I'll use the same types for properties that describe the same
> things/features. Would it also be preferred to drop the defs and just have 
> channel nodes?

If we did do channel nodes only. We have various options.
1) constraints against adi,sensor-types (which I think should probably be an enum)
   That should look similar to the node name constraints.
2) constrain only via documentation. (I don't like this at all)
3) have no constraints at all and rely on documenting what makes sense.
   I'm not entirely clear on whether we can ignore the sensor type etc and
   provide properties to control more directly everything to do with it?  So the
   excitation types etc

Jonathan

> 
> Thanks,
> Marcelo
>

On Tue, 13 May 2025 09:33:40 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:

> Add device tree documentation for AD4170 and similar sigma-delta ADCs.
> The AD4170 is a 24-bit, multichannel, sigma-delta ADC.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
Hi Marcelo,

A few additional comments from me.

J
> ---
> Change log v2 -> v3
> 
> [device tree changes]
> - Removed unneeded allOf.
> - Removed occurences of adi,sensor-type type re-declaration.
> - Created type for the AD4170 channels, allowing to avoid dt doc repetition.
> 
>  .../bindings/iio/adc/adi,ad4170.yaml          | 544 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 551 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
> new file mode 100644
> index 000000000000..0a06258b6631
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
> @@ -0,0 +1,544 @@

> +  sensor-node:
...
> +
> +      adi,sensor-type:
> +        description: |
> +          Type of sensor connected to the device. Depending on the sensor type
> +          (weigh scale, RTD, or thermocouple) the values of sensor-node
> +          properties have slightly different constraints. This property
> +          specifies which particular external sensor is connected to the ADC so
> +          the sensor-node properties can be properly parsed and verified. The
> +          possible sensor types are:
> +          0: weigh scale;
> +          1: RTD;
> +          2: thermocouple.
> +        $ref: /schemas/types.yaml#/definitions/uint8

I think this should be an enum of strings.  That will give us a clean
way to extend it for other sensor types in future.

> +    required:
> +      - reg
> +      - diff-channels
> +      - bipolar
> +      - adi,sensor-type
> +      - adi,reference-select
> +
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad4170
> +      - adi,ad4190
> +      - adi,ad4195
>
> +
> +  interrupts:
> +    maxItems: 1

What if they are both wired?  At that point shouldn't software
make up it's mind which to use?

> +
> +  interrupt-names:
> +    description:
> +      Specify which pin should be configured as Data Ready interrupt.
> +    enum:
> +      - sdo
> +      - dig_aux1
> +    default: sdo
> +

Hi Jonathan,

I'm still working on the changes suggested for this set.
Just to mention, I've updated the dt-binding to declare only channel nodes under
the ADC node.

... 
> > +      adi,sensor-type:
> > +        description: |
> > +          Type of sensor connected to the device. Depending on the sensor type
> > +          (weigh scale, RTD, or thermocouple) the values of sensor-node
> > +          properties have slightly different constraints. This property
> > +          specifies which particular external sensor is connected to the ADC so
> > +          the sensor-node properties can be properly parsed and verified. The
> > +          possible sensor types are:
> > +          0: weigh scale;
> > +          1: RTD;
> > +          2: thermocouple.
> > +        $ref: /schemas/types.yaml#/definitions/uint8
> 
> I think this should be an enum of strings.  That will give us a clean
> way to extend it for other sensor types in future.
Ack

...
> > +
> > +  interrupts:
> > +    maxItems: 1
> 
> What if they are both wired?  At that point shouldn't software
> make up it's mind which to use?

Not sure I understand the concern here. Is 'interrupt-names' property expected
to precede 'interrupts' in dt-doc? For AD4170 and similar parts, the data
ready signal (/RDY) is by default provided on the SDO line after the completion
of a conversion. Alternatively, it can be provided on the DIG_AUX1 pin in
which case the chip disables the RDY function on SDO. So, there can only be one
data ready interrupt enabled at a time. Guess I'll add some description to make
clear only one interrupt can be enabled.

> > +
> > +  interrupt-names:
> > +    description:
> > +      Specify which pin should be configured as Data Ready interrupt.
> > +    enum:
> > +      - sdo
> > +      - dig_aux1
> > +    default: sdo
> > +

Thanks,
Marcelo

On Mon, 26 May 2025 18:59:48 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> Hi Jonathan,
> 
> I'm still working on the changes suggested for this set.
> Just to mention, I've updated the dt-binding to declare only channel nodes under
> the ADC node.
> 
> ... 
> > > +      adi,sensor-type:
> > > +        description: |
> > > +          Type of sensor connected to the device. Depending on the sensor type
> > > +          (weigh scale, RTD, or thermocouple) the values of sensor-node
> > > +          properties have slightly different constraints. This property
> > > +          specifies which particular external sensor is connected to the ADC so
> > > +          the sensor-node properties can be properly parsed and verified. The
> > > +          possible sensor types are:
> > > +          0: weigh scale;
> > > +          1: RTD;
> > > +          2: thermocouple.
> > > +        $ref: /schemas/types.yaml#/definitions/uint8  
> > 
> > I think this should be an enum of strings.  That will give us a clean
> > way to extend it for other sensor types in future.  
> Ack
> 
> ...
> > > +
> > > +  interrupts:
> > > +    maxItems: 1  
> > 
> > What if they are both wired?  At that point shouldn't software
> > make up it's mind which to use?  
> 
> Not sure I understand the concern here. Is 'interrupt-names' property expected
> to precede 'interrupts' in dt-doc? For AD4170 and similar parts, the data
> ready signal (/RDY) is by default provided on the SDO line after the completion
> of a conversion. Alternatively, it can be provided on the DIG_AUX1 pin in
> which case the chip disables the RDY function on SDO. So, there can only be one
> data ready interrupt enabled at a time. Guess I'll add some description to make
> clear only one interrupt can be enabled.

DT is describing the wiring, not what should be enabled by the driver.

There is no need for it to restrict to 1 interrupt if someone has wired them
both that I can think of.  In that circumstance the driver gets to decide
between the mess of shared SDO / dataready handling vs a nice
separate signal on dig_aux1

If both are wired to the host on appropriate pins, up to the driver to
decide what it wants.  A driver may also only supports the
dig_aux1 combination or only the sdo combination and so just fail to load
if it isn't happy with the one the DT author thought mattered.
Providing both if that is what is wired, gives the flexibility to find
the right one. 

> 
> > > +
> > > +  interrupt-names:
> > > +    description:
> > > +      Specify which pin should be configured as Data Ready interrupt.
> > > +    enum:
> > > +      - sdo
> > > +      - dig_aux1
> > > +    default: sdo
> > > +  
> 
> Thanks,
> Marcelo
>