[PATCH v5 01/11] dt-bindings: iio: adc: Add AD4170

Marcelo Schmitt posted 11 patches 4 months ago
There is a newer version of this series
[PATCH v5 01/11] dt-bindings: iio: adc: Add AD4170
Posted by Marcelo Schmitt 4 months ago
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 v4 -> v5
- Dropped interrupt maxItems constraint.
- Spelled out RC acronym in reference-buffer description.
- Require to specify interrupt-names when using interrupts.
- Added interrupt-names to the examples.
- Made adi,excitation-pin properties identical to adi,ad4130.
- Removed interrupt-parent props from the examples.

Proposing new types and ways of describing hardware for weigh scale load cells
and related sensors external to ADCs can lead to potential better description of
how those components connect to the ADC. However, we must use what already
exists for properties documenting features that are the same across different
devices. 

Maybe, we could use generic defs to define adi,excitation-current-n-microamp and
adi,excitation-pin and avoid repetition with those. Though, that triggers a
dt_binding_check warning. Also, having mixed notation (some prop declarations
using defines and others not) seems to not be desirable.

It looks like the only option left is making adi,excitation-pin properties
identical to adi,ad4130.

On one hand, dropping adi,excitation-pin defs and making those properties
identical to adi,ad4130 preserves their syntax and semantics accross
dt-bindings. OTOH, we end up with more text repetition in the doc.


 .../bindings/iio/adc/adi,ad4170.yaml          | 564 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 571 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..e3249ec56a14
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
@@ -0,0 +1,564 @@
+# 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:
+  reference-buffer:
+    description: |
+      Enable precharge buffer, full buffer, or skip reference buffering of
+      the positive/negative voltage reference. Because the output impedance
+      of the source driving the voltage reference inputs may be dynamic,
+      resistive/capacitive 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 REFINn+ and REFINn-
+      inputs 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. The valid options for this property are:
+      0: Reference precharge buffer.
+      1: Full reference buffering.
+      2: Bypass reference buffers (buffering disabled).
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2]
+    default: 1
+
+properties:
+  compatible:
+    enum:
+      - adi,ad4170
+      - adi,ad4190
+      - adi,ad4195
+
+  avss-supply:
+    description:
+      Reference voltage supply for AVSS. If provided, describes the magnitude
+      (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:
+    description:
+      Interrupt for signaling the completion of conversion results. The data
+      ready signal (RDY) used as interrupt is by default provided on the SDO
+      pin. Alternatively, it can be provided on the DIG_AUX1 pin in which case
+      the chip disables the RDY function on SDO. Thus, there can be only one
+      data ready interrupt enabled at a time.
+
+  interrupt-names:
+    description:
+      Specify which pin should be configured as Data Ready interrupt.
+    enum:
+      - sdo
+      - dig_aux1
+
+  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
+
+  '#clock-cells':
+    const: 0
+
+  clock-output-names:
+    maxItems: 1
+
+  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
+
+  adi,vbias-pins:
+    description: Analog inputs to apply a voltage bias of (AVDD − AVSS) / 2 to.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 9
+    items:
+      minimum: 0
+      maximum: 8
+
+allOf:
+  # Some devices don't have integrated DAC
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,ad4190
+              - adi,ad4195
+    then:
+      properties:
+        ldac-gpios: false
+
+  # Require to specify the interrupt pin when using interrupts
+  - if:
+      required:
+        - interrupts
+    then:
+      required:
+        - interrupt-names
+
+  # If an external clock is set, the internal clock cannot go out and vice versa
+  - oneOf:
+      - required: [clocks]
+        properties:
+          '#clock-cells': false
+      - required: ['#clock-cells']
+        properties:
+          clocks: false
+
+patternProperties:
+  "^channel@[0-9a-f]$":
+    $ref: /schemas/iio/adc/adc.yaml#
+    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]
+
+      adi,reference-select:
+        description: |
+          Select the reference source to use when converting on the
+          specific channel. Valid values are:
+          0: REFIN+/REFIN-
+          1: REFIN2+/REFIN2−
+          2: REFOUT/AVSS (internal reference)
+          3: AVDD/AVSS
+          If not specified, REFOUT/AVSS is used.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        enum: [0, 1, 2, 3]
+        default: 1
+
+      adi,positive-reference-buffer:
+        $ref: '#/$defs/reference-buffer'
+
+      adi,negative-reference-buffer:
+        $ref: '#/$defs/reference-buffer'
+
+      adi,sensor-type:
+        description:
+          The AD4170 and similar designs have features to aid interfacing with
+          load cell 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
+          ADC chip functionality on their setups. A key characteristic of those
+          external sensors is that they must be excited either by voltage supply
+          or by ADC chip excitation signals. The sensor can then be read through
+          a pair of analog inputs. This property specifies which particular
+          sensor type is connected to the ADC so it can be properly setup and
+          handled. Omit this property for conventional (not weigh scale, RTD, or
+          thermocouple) ADC channel setups.
+        $ref: /schemas/types.yaml#/definitions/string
+        enum: [ weighscale, rtd, thermocouple ]
+
+      adi,excitation-pin-0:
+        description:
+          Analog input to apply excitation current to while the channel
+          is active.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 20
+        default: 0
+
+      adi,excitation-pin-1:
+        description:
+          Analog input to apply excitation current to while the channel
+          is active.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 20
+        default: 0
+
+      adi,excitation-pin-2:
+        description:
+          Analog input to apply excitation current to while the channel
+          is active.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 20
+        default: 0
+
+      adi,excitation-pin-3:
+        description:
+          Analog input to apply excitation current to while the channel
+          is active.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 20
+        default: 0
+
+      adi,excitation-current-0-microamp:
+        description:
+          Excitation current in microamperes to be applied to pin specified in
+          adi,excitation-pin-0 while this channel is active.
+        enum: [0, 10, 50, 100, 250, 500, 1000, 1500]
+        default: 0
+
+      adi,excitation-current-1-microamp:
+        description:
+          Excitation current in microamperes to be applied to pin specified in
+          adi,excitation-pin-1 while this channel is active.
+        enum: [0, 10, 50, 100, 250, 500, 1000, 1500]
+        default: 0
+
+      adi,excitation-current-2-microamp:
+        description:
+          Excitation current in microamperes to be applied to pin specified in
+          adi,excitation-pin-2 while this channel is active.
+        enum: [0, 10, 50, 100, 250, 500, 1000, 1500]
+        default: 0
+
+      adi,excitation-current-3-microamp:
+        description:
+          Excitation current in microamperes to be applied to pin specified in
+          adi,excitation-pin-3 while this channel is active.
+        enum: [0, 10, 50, 100, 250, 500, 1000, 1500]
+        default: 0
+
+      adi,excitation-ac:
+        type: boolean
+        description:
+          Whether the external sensor has to be AC or DC excited. When omitted,
+          it is DC excited.
+
+    allOf:
+      - oneOf:
+          - required: [single-channel, common-mode-channel]
+            properties:
+              diff-channels: false
+          - required: [diff-channels]
+            properties:
+              single-channel: false
+              common-mode-channel: false
+      # Usual ADC channels don't need external circuitry excitation.
+      - if:
+          not:
+            required:
+              - adi,sensor-type
+        then:
+          properties:
+            adi,excitation-pin-0: false
+            adi,excitation-pin-1: false
+            adi,excitation-pin-2: false
+            adi,excitation-pin-3: false
+            adi,excitation-current-0-microamp: false
+            adi,excitation-current-1-microamp: false
+            adi,excitation-current-2-microamp: false
+            adi,excitation-current-3-microamp: false
+            adi,excitation-ac: false
+      # Weigh scale bridge AC excited with one pair of predefined signals.
+      - if:
+          allOf:
+            - properties:
+                adi,sensor-type:
+                  contains:
+                    const: weighscale
+            - required:
+                - adi,excitation-ac
+                - adi,excitation-pin-2
+                - adi,excitation-pin-3
+            - not:
+                required:
+                  - adi,excitation-current-2-microamp
+                  - adi,excitation-current-3-microamp
+        then:
+          properties:
+            adi,excitation-pin-2:
+              const: 19
+            adi,excitation-pin-3:
+              const: 20
+      # Weigh scale bridge AC excited with two pairs of predefined signals.
+      - if:
+          allOf:
+            - properties:
+                adi,sensor-type:
+                  contains:
+                    const: weighscale
+            - required:
+                - adi,excitation-ac
+                - adi,excitation-pin-0
+                - adi,excitation-pin-1
+                - adi,excitation-pin-2
+                - adi,excitation-pin-3
+            - not:
+                required:
+                  - adi,excitation-current-0-microamp
+                  - adi,excitation-current-1-microamp
+                  - adi,excitation-current-2-microamp
+                  - adi,excitation-current-3-microamp
+        then:
+          properties:
+            adi,excitation-pin-0:
+              const: 17
+            adi,excitation-pin-1:
+              const: 18
+            adi,excitation-pin-2:
+              const: 19
+            adi,excitation-pin-3:
+              const: 20
+
+required:
+  - compatible
+  - reg
+  - avdd-supply
+  - iovdd-supply
+  - spi-cpol
+  - spi-cpha
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            compatible = "adi,ad4170";
+            reg = <0>;
+            spi-max-frequency = <20000000>;
+            spi-cpol;
+            spi-cpha;
+            avdd-supply = <&avdd>;
+            iovdd-supply = <&iovdd>;
+            clocks = <&clk>;
+            clock-names = "xtal";
+            interrupts = <0 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-names = "dig_aux1";
+            adi,vbias-pins = <7>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            // Sample AIN0 with respect to DGND throughout AVDD/DGND input range
+            // Pseudo-differential unipolar
+            channel@0 {
+                reg = <0>;
+                single-channel = <0>;
+                common-mode-channel = <24>;
+                adi,reference-select = <3>;
+            };
+            // Weigh scale sensor
+            channel@1 {
+                reg = <1>;
+                bipolar;
+                diff-channels = <1 2>;
+                adi,reference-select = <0>;
+                adi,positive-reference-buffer = <0>;
+                adi,negative-reference-buffer = <0>;
+                adi,sensor-type = "weighscale";
+                adi,excitation-pin-2 = <19>;
+                adi,excitation-pin-3 = <20>;
+                adi,excitation-ac;
+            };
+            // RTD sensor
+            channel@2 {
+                reg = <2>;
+                bipolar;
+                diff-channels = <3 4>;
+                adi,reference-select = <0>;
+                adi,sensor-type = "rtd";
+                adi,excitation-pin-0 = <5>;
+                adi,excitation-pin-1 = <6>;
+                adi,excitation-current-0-microamp = <500>;
+                adi,excitation-current-1-microamp = <500>;
+                adi,excitation-ac;
+            };
+            // Thermocouple sensor
+            channel@3 {
+                reg = <3>;
+                bipolar;
+                diff-channels = <7 8>;
+                adi,reference-select = <0>;
+                adi,sensor-type = "thermocouple";
+                adi,excitation-pin-0 = <18>;
+                adi,excitation-current-0-microamp = <500>;
+            };
+        };
+    };
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            compatible = "adi,ad4170";
+            reg = <0>;
+            spi-max-frequency = <20000000>;
+            spi-cpol;
+            spi-cpha;
+            avdd-supply = <&avdd>;
+            iovdd-supply = <&iovdd>;
+            #clock-cells = <0>;
+            clock-output-names = "ad4170-clk16mhz";
+            interrupts = <0 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-names = "dig_aux1";
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            // Sample AIN0 with respect to AIN1 throughout AVDD/AVSS input range
+            // Differential bipolar. If AVSS < 0V, differential true bipolar
+            channel@0 {
+                reg = <0>;
+                bipolar;
+                diff-channels = <0 1>;
+                adi,reference-select = <3>;
+            };
+            // Sample AIN2 with respect to DGND throughout AVDD/DGND input range
+            // Pseudo-differential unipolar
+            channel@1 {
+                reg = <1>;
+                single-channel = <2>;
+                common-mode-channel = <24>;
+                adi,reference-select = <3>;
+            };
+            // Sample AIN3 with respect to 2.5V throughout AVDD/AVSS input range
+            // Pseudo-differential bipolar
+            channel@2 {
+                reg = <2>;
+                bipolar;
+                single-channel = <3>;
+                common-mode-channel = <29>;
+                adi,reference-select = <3>;
+            };
+            // Sample AIN4 with respect to DGND throughout AVDD/AVSS input range
+            // Pseudo-differential bipolar
+            channel@3 {
+                reg = <3>;
+                bipolar;
+                single-channel = <4>;
+                common-mode-channel = <24>;
+                adi,reference-select = <3>;
+            };
+            // Sample AIN5 with respect to 2.5V throughout AVDD/AVSS input range
+            // Pseudo-differential unipolar (AD4170 datasheet page 46 example)
+            channel@4 {
+                reg = <4>;
+                single-channel = <5>;
+                common-mode-channel = <29>;
+                adi,reference-select = <3>;
+            };
+            // Sample AIN6 with respect to 2.5V throughout REFIN+/REFIN- input range
+            // Pseudo-differential bipolar
+            channel@5 {
+                reg = <5>;
+                bipolar;
+                single-channel = <6>;
+                common-mode-channel = <29>;
+                adi,reference-select = <0>;
+            };
+            // Weigh scale sensor
+            channel@6 {
+                reg = <6>;
+                bipolar;
+                diff-channels = <7 8>;
+                adi,reference-select = <0>;
+                adi,sensor-type = "weighscale";
+                adi,excitation-pin-0 = <17>;
+                adi,excitation-pin-1 = <18>;
+                adi,excitation-pin-2 = <19>;
+                adi,excitation-pin-3 = <20>;
+                adi,excitation-ac;
+            };
+        };
+    };
+...
+
diff --git a/MAINTAINERS b/MAINTAINERS
index abfd5ded8735..44735314a43e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1392,6 +1392,13 @@ F:	Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
 F:	drivers/iio/adc/ad4130.c
 
+ANALOG DEVICES INC AD4170 DRIVER
+M:	Marcelo Schmitt <marcelo.schmitt@analog.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
+
 ANALOG DEVICES INC AD4695 DRIVER
 M:	Michael Hennerich <michael.hennerich@analog.com>
 M:	Nuno Sá <nuno.sa@analog.com>
-- 
2.47.2

Re: [PATCH v5 01/11] dt-bindings: iio: adc: Add AD4170
Posted by Conor Dooley 3 months, 3 weeks ago
On Tue, Jun 10, 2025 at 05:31:04PM -0300, 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 v4 -> v5
> - Dropped interrupt maxItems constraint.
> - Spelled out RC acronym in reference-buffer description.
> - Require to specify interrupt-names when using interrupts.
> - Added interrupt-names to the examples.
> - Made adi,excitation-pin properties identical to adi,ad4130.
> - Removed interrupt-parent props from the examples.
> 
> Proposing new types and ways of describing hardware for weigh scale load cells
> and related sensors external to ADCs can lead to potential better description of
> how those components connect to the ADC. However, we must use what already
> exists for properties documenting features that are the same across different
> devices. 
> 
> Maybe, we could use generic defs to define adi,excitation-current-n-microamp and
> adi,excitation-pin and avoid repetition with those. Though, that triggers a
> dt_binding_check warning. Also, having mixed notation (some prop declarations
> using defines and others not) seems to not be desirable.
> 
> It looks like the only option left is making adi,excitation-pin properties
> identical to adi,ad4130.
> 
> On one hand, dropping adi,excitation-pin defs and making those properties
> identical to adi,ad4130 preserves their syntax and semantics accross
> dt-bindings. OTOH, we end up with more text repetition in the doc.
> 
> 
>  .../bindings/iio/adc/adi,ad4170.yaml          | 564 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 571 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..e3249ec56a14
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
> @@ -0,0 +1,564 @@
> +# 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:
> +  reference-buffer:
> +    description: |
> +      Enable precharge buffer, full buffer, or skip reference buffering of
> +      the positive/negative voltage reference. Because the output impedance
> +      of the source driving the voltage reference inputs may be dynamic,
> +      resistive/capacitive 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 REFINn+ and REFINn-
> +      inputs 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. The valid options for this property are:
> +      0: Reference precharge buffer.
> +      1: Full reference buffering.
> +      2: Bypass reference buffers (buffering disabled).
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 2]
> +    default: 1

Why make this property a uint32, rather than a string where you can use
something like "precharge", "full" and "bypass" (or "disabled")? The
next similar device could use something slightly different then the
binding becomes pretty clunky.
Can you explain why this is a dt property rather than something
adjustable at runtime?

Otherwise, what you have here looks sane enough to me - but I'd like to
see some comments from Jonathan or David etc about your approach to the
excitation properties.

Cheers,
Conor.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad4170
> +      - adi,ad4190
> +      - adi,ad4195
> +
> +  avss-supply:
> +    description:
> +      Reference voltage supply for AVSS. If provided, describes the magnitude
> +      (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:
> +    description:
> +      Interrupt for signaling the completion of conversion results. The data
> +      ready signal (RDY) used as interrupt is by default provided on the SDO
> +      pin. Alternatively, it can be provided on the DIG_AUX1 pin in which case
> +      the chip disables the RDY function on SDO. Thus, there can be only one
> +      data ready interrupt enabled at a time.
> +
> +  interrupt-names:
> +    description:
> +      Specify which pin should be configured as Data Ready interrupt.
> +    enum:
> +      - sdo
> +      - dig_aux1
> +
> +  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
> +
> +  '#clock-cells':
> +    const: 0
> +
> +  clock-output-names:
> +    maxItems: 1
> +
> +  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
> +
> +  adi,vbias-pins:
> +    description: Analog inputs to apply a voltage bias of (AVDD − AVSS) / 2 to.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 9
> +    items:
> +      minimum: 0
> +      maximum: 8
> +
> +allOf:
> +  # Some devices don't have integrated DAC
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,ad4190
> +              - adi,ad4195
> +    then:
> +      properties:
> +        ldac-gpios: false
> +
> +  # Require to specify the interrupt pin when using interrupts
> +  - if:
> +      required:
> +        - interrupts
> +    then:
> +      required:
> +        - interrupt-names
> +
> +  # If an external clock is set, the internal clock cannot go out and vice versa
> +  - oneOf:
> +      - required: [clocks]
> +        properties:
> +          '#clock-cells': false
> +      - required: ['#clock-cells']
> +        properties:
> +          clocks: false
> +
> +patternProperties:
> +  "^channel@[0-9a-f]$":
> +    $ref: /schemas/iio/adc/adc.yaml#
> +    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]
> +
> +      adi,reference-select:
> +        description: |
> +          Select the reference source to use when converting on the
> +          specific channel. Valid values are:
> +          0: REFIN+/REFIN-
> +          1: REFIN2+/REFIN2−
> +          2: REFOUT/AVSS (internal reference)
> +          3: AVDD/AVSS
> +          If not specified, REFOUT/AVSS is used.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        enum: [0, 1, 2, 3]
> +        default: 1
> +
> +      adi,positive-reference-buffer:
> +        $ref: '#/$defs/reference-buffer'
> +
> +      adi,negative-reference-buffer:
> +        $ref: '#/$defs/reference-buffer'
> +
> +      adi,sensor-type:
> +        description:
> +          The AD4170 and similar designs have features to aid interfacing with
> +          load cell 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
> +          ADC chip functionality on their setups. A key characteristic of those
> +          external sensors is that they must be excited either by voltage supply
> +          or by ADC chip excitation signals. The sensor can then be read through
> +          a pair of analog inputs. This property specifies which particular
> +          sensor type is connected to the ADC so it can be properly setup and
> +          handled. Omit this property for conventional (not weigh scale, RTD, or
> +          thermocouple) ADC channel setups.
> +        $ref: /schemas/types.yaml#/definitions/string
> +        enum: [ weighscale, rtd, thermocouple ]
> +
> +      adi,excitation-pin-0:
> +        description:
> +          Analog input to apply excitation current to while the channel
> +          is active.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0
> +        maximum: 20
> +        default: 0
> +
> +      adi,excitation-pin-1:
> +        description:
> +          Analog input to apply excitation current to while the channel
> +          is active.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0
> +        maximum: 20
> +        default: 0
> +
> +      adi,excitation-pin-2:
> +        description:
> +          Analog input to apply excitation current to while the channel
> +          is active.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0
> +        maximum: 20
> +        default: 0
> +
> +      adi,excitation-pin-3:
> +        description:
> +          Analog input to apply excitation current to while the channel
> +          is active.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0
> +        maximum: 20
> +        default: 0
> +
> +      adi,excitation-current-0-microamp:
> +        description:
> +          Excitation current in microamperes to be applied to pin specified in
> +          adi,excitation-pin-0 while this channel is active.
> +        enum: [0, 10, 50, 100, 250, 500, 1000, 1500]
> +        default: 0
> +
> +      adi,excitation-current-1-microamp:
> +        description:
> +          Excitation current in microamperes to be applied to pin specified in
> +          adi,excitation-pin-1 while this channel is active.
> +        enum: [0, 10, 50, 100, 250, 500, 1000, 1500]
> +        default: 0
> +
> +      adi,excitation-current-2-microamp:
> +        description:
> +          Excitation current in microamperes to be applied to pin specified in
> +          adi,excitation-pin-2 while this channel is active.
> +        enum: [0, 10, 50, 100, 250, 500, 1000, 1500]
> +        default: 0
> +
> +      adi,excitation-current-3-microamp:
> +        description:
> +          Excitation current in microamperes to be applied to pin specified in
> +          adi,excitation-pin-3 while this channel is active.
> +        enum: [0, 10, 50, 100, 250, 500, 1000, 1500]
> +        default: 0
> +
> +      adi,excitation-ac:
> +        type: boolean
> +        description:
> +          Whether the external sensor has to be AC or DC excited. When omitted,
> +          it is DC excited.
> +
> +    allOf:
> +      - oneOf:
> +          - required: [single-channel, common-mode-channel]
> +            properties:
> +              diff-channels: false
> +          - required: [diff-channels]
> +            properties:
> +              single-channel: false
> +              common-mode-channel: false
> +      # Usual ADC channels don't need external circuitry excitation.
> +      - if:
> +          not:
> +            required:
> +              - adi,sensor-type
> +        then:
> +          properties:
> +            adi,excitation-pin-0: false
> +            adi,excitation-pin-1: false
> +            adi,excitation-pin-2: false
> +            adi,excitation-pin-3: false
> +            adi,excitation-current-0-microamp: false
> +            adi,excitation-current-1-microamp: false
> +            adi,excitation-current-2-microamp: false
> +            adi,excitation-current-3-microamp: false
> +            adi,excitation-ac: false
> +      # Weigh scale bridge AC excited with one pair of predefined signals.
> +      - if:
> +          allOf:
> +            - properties:
> +                adi,sensor-type:
> +                  contains:
> +                    const: weighscale
> +            - required:
> +                - adi,excitation-ac
> +                - adi,excitation-pin-2
> +                - adi,excitation-pin-3
> +            - not:
> +                required:
> +                  - adi,excitation-current-2-microamp
> +                  - adi,excitation-current-3-microamp
> +        then:
> +          properties:
> +            adi,excitation-pin-2:
> +              const: 19
> +            adi,excitation-pin-3:
> +              const: 20
> +      # Weigh scale bridge AC excited with two pairs of predefined signals.
> +      - if:
> +          allOf:
> +            - properties:
> +                adi,sensor-type:
> +                  contains:
> +                    const: weighscale
> +            - required:
> +                - adi,excitation-ac
> +                - adi,excitation-pin-0
> +                - adi,excitation-pin-1
> +                - adi,excitation-pin-2
> +                - adi,excitation-pin-3
> +            - not:
> +                required:
> +                  - adi,excitation-current-0-microamp
> +                  - adi,excitation-current-1-microamp
> +                  - adi,excitation-current-2-microamp
> +                  - adi,excitation-current-3-microamp
> +        then:
> +          properties:
> +            adi,excitation-pin-0:
> +              const: 17
> +            adi,excitation-pin-1:
> +              const: 18
> +            adi,excitation-pin-2:
> +              const: 19
> +            adi,excitation-pin-3:
> +              const: 20
> +
> +required:
> +  - compatible
> +  - reg
> +  - avdd-supply
> +  - iovdd-supply
> +  - spi-cpol
> +  - spi-cpha
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +            compatible = "adi,ad4170";
> +            reg = <0>;
> +            spi-max-frequency = <20000000>;
> +            spi-cpol;
> +            spi-cpha;
> +            avdd-supply = <&avdd>;
> +            iovdd-supply = <&iovdd>;
> +            clocks = <&clk>;
> +            clock-names = "xtal";
> +            interrupts = <0 IRQ_TYPE_EDGE_FALLING>;
> +            interrupt-names = "dig_aux1";
> +            adi,vbias-pins = <7>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            // Sample AIN0 with respect to DGND throughout AVDD/DGND input range
> +            // Pseudo-differential unipolar
> +            channel@0 {
> +                reg = <0>;
> +                single-channel = <0>;
> +                common-mode-channel = <24>;
> +                adi,reference-select = <3>;
> +            };
> +            // Weigh scale sensor
> +            channel@1 {
> +                reg = <1>;
> +                bipolar;
> +                diff-channels = <1 2>;
> +                adi,reference-select = <0>;
> +                adi,positive-reference-buffer = <0>;
> +                adi,negative-reference-buffer = <0>;
> +                adi,sensor-type = "weighscale";
> +                adi,excitation-pin-2 = <19>;
> +                adi,excitation-pin-3 = <20>;
> +                adi,excitation-ac;
> +            };
> +            // RTD sensor
> +            channel@2 {
> +                reg = <2>;
> +                bipolar;
> +                diff-channels = <3 4>;
> +                adi,reference-select = <0>;
> +                adi,sensor-type = "rtd";
> +                adi,excitation-pin-0 = <5>;
> +                adi,excitation-pin-1 = <6>;
> +                adi,excitation-current-0-microamp = <500>;
> +                adi,excitation-current-1-microamp = <500>;
> +                adi,excitation-ac;
> +            };
> +            // Thermocouple sensor
> +            channel@3 {
> +                reg = <3>;
> +                bipolar;
> +                diff-channels = <7 8>;
> +                adi,reference-select = <0>;
> +                adi,sensor-type = "thermocouple";
> +                adi,excitation-pin-0 = <18>;
> +                adi,excitation-current-0-microamp = <500>;
> +            };
> +        };
> +    };
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +            compatible = "adi,ad4170";
> +            reg = <0>;
> +            spi-max-frequency = <20000000>;
> +            spi-cpol;
> +            spi-cpha;
> +            avdd-supply = <&avdd>;
> +            iovdd-supply = <&iovdd>;
> +            #clock-cells = <0>;
> +            clock-output-names = "ad4170-clk16mhz";
> +            interrupts = <0 IRQ_TYPE_EDGE_FALLING>;
> +            interrupt-names = "dig_aux1";
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            // Sample AIN0 with respect to AIN1 throughout AVDD/AVSS input range
> +            // Differential bipolar. If AVSS < 0V, differential true bipolar
> +            channel@0 {
> +                reg = <0>;
> +                bipolar;
> +                diff-channels = <0 1>;
> +                adi,reference-select = <3>;
> +            };
> +            // Sample AIN2 with respect to DGND throughout AVDD/DGND input range
> +            // Pseudo-differential unipolar
> +            channel@1 {
> +                reg = <1>;
> +                single-channel = <2>;
> +                common-mode-channel = <24>;
> +                adi,reference-select = <3>;
> +            };
> +            // Sample AIN3 with respect to 2.5V throughout AVDD/AVSS input range
> +            // Pseudo-differential bipolar
> +            channel@2 {
> +                reg = <2>;
> +                bipolar;
> +                single-channel = <3>;
> +                common-mode-channel = <29>;
> +                adi,reference-select = <3>;
> +            };
> +            // Sample AIN4 with respect to DGND throughout AVDD/AVSS input range
> +            // Pseudo-differential bipolar
> +            channel@3 {
> +                reg = <3>;
> +                bipolar;
> +                single-channel = <4>;
> +                common-mode-channel = <24>;
> +                adi,reference-select = <3>;
> +            };
> +            // Sample AIN5 with respect to 2.5V throughout AVDD/AVSS input range
> +            // Pseudo-differential unipolar (AD4170 datasheet page 46 example)
> +            channel@4 {
> +                reg = <4>;
> +                single-channel = <5>;
> +                common-mode-channel = <29>;
> +                adi,reference-select = <3>;
> +            };
> +            // Sample AIN6 with respect to 2.5V throughout REFIN+/REFIN- input range
> +            // Pseudo-differential bipolar
> +            channel@5 {
> +                reg = <5>;
> +                bipolar;
> +                single-channel = <6>;
> +                common-mode-channel = <29>;
> +                adi,reference-select = <0>;
> +            };
> +            // Weigh scale sensor
> +            channel@6 {
> +                reg = <6>;
> +                bipolar;
> +                diff-channels = <7 8>;
> +                adi,reference-select = <0>;
> +                adi,sensor-type = "weighscale";
> +                adi,excitation-pin-0 = <17>;
> +                adi,excitation-pin-1 = <18>;
> +                adi,excitation-pin-2 = <19>;
> +                adi,excitation-pin-3 = <20>;
> +                adi,excitation-ac;
> +            };
> +        };
> +    };
> +...
> +
> diff --git a/MAINTAINERS b/MAINTAINERS
> index abfd5ded8735..44735314a43e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1392,6 +1392,13 @@ F:	Documentation/ABI/testing/sysfs-bus-iio-adc-ad4130
>  F:	Documentation/devicetree/bindings/iio/adc/adi,ad4130.yaml
>  F:	drivers/iio/adc/ad4130.c
>  
> +ANALOG DEVICES INC AD4170 DRIVER
> +M:	Marcelo Schmitt <marcelo.schmitt@analog.com>
> +L:	linux-iio@vger.kernel.org
> +S:	Supported
> +W:	https://ez.analog.com/linux-software-drivers
> +F:	Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
> +
>  ANALOG DEVICES INC AD4695 DRIVER
>  M:	Michael Hennerich <michael.hennerich@analog.com>
>  M:	Nuno Sá <nuno.sa@analog.com>
> -- 
> 2.47.2
> 
Re: [PATCH v5 01/11] dt-bindings: iio: adc: Add AD4170
Posted by Jonathan Cameron 3 months, 3 weeks ago
> 
> Otherwise, what you have here looks sane enough to me - but I'd like to
> see some comments from Jonathan or David etc about your approach to the
> excitation properties.

They look sane to me.  The complexity of devices that handle weigh cells and
similar are always a pain.  In theory we could have a go at describing
the weigh-cell in DT and then try to derive the 'right' settings but that
seems like a very complex thing to do.  Long time since I did anything with
weigh cells, but I think you mostly read the settings to use of a datasheet rather
than deriving them from first principles. Thermocouples are similar (I'm not
that familiar with RTDs)

As to the handling of the different sensor types - that seems like a sensible
way to constrain the binding and end up with sane readable combinations rather
than a bunch of excitation settings with no info on what is connected.

Not sure this is the perfect solution, but to me it looks good enough and
flexible / general enough to cover a wide range of other devices.

Jonathan

> 
> Cheers,
> Conor.
Re: [PATCH v5 01/11] dt-bindings: iio: adc: Add AD4170
Posted by David Lechner 3 months, 3 weeks ago
On 6/16/25 10:41 AM, Conor Dooley wrote:
> On Tue, Jun 10, 2025 at 05:31:04PM -0300, 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 v4 -> v5
>> - Dropped interrupt maxItems constraint.
>> - Spelled out RC acronym in reference-buffer description.
>> - Require to specify interrupt-names when using interrupts.
>> - Added interrupt-names to the examples.
>> - Made adi,excitation-pin properties identical to adi,ad4130.
>> - Removed interrupt-parent props from the examples.
>>
>> Proposing new types and ways of describing hardware for weigh scale load cells
>> and related sensors external to ADCs can lead to potential better description of
>> how those components connect to the ADC. However, we must use what already
>> exists for properties documenting features that are the same across different
>> devices. 
>>
>> Maybe, we could use generic defs to define adi,excitation-current-n-microamp and
>> adi,excitation-pin and avoid repetition with those. Though, that triggers a
>> dt_binding_check warning. Also, having mixed notation (some prop declarations
>> using defines and others not) seems to not be desirable.
>>
>> It looks like the only option left is making adi,excitation-pin properties
>> identical to adi,ad4130.
>>
>> On one hand, dropping adi,excitation-pin defs and making those properties
>> identical to adi,ad4130 preserves their syntax and semantics accross
>> dt-bindings. OTOH, we end up with more text repetition in the doc.
>>
>>
>>  .../bindings/iio/adc/adi,ad4170.yaml          | 564 ++++++++++++++++++
>>  MAINTAINERS                                   |   7 +
>>  2 files changed, 571 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..e3249ec56a14
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
>> @@ -0,0 +1,564 @@
>> +# 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:
>> +  reference-buffer:
>> +    description: |
>> +      Enable precharge buffer, full buffer, or skip reference buffering of
>> +      the positive/negative voltage reference. Because the output impedance
>> +      of the source driving the voltage reference inputs may be dynamic,
>> +      resistive/capacitive 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 REFINn+ and REFINn-
>> +      inputs 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. The valid options for this property are:
>> +      0: Reference precharge buffer.
>> +      1: Full reference buffering.
>> +      2: Bypass reference buffers (buffering disabled).
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [0, 1, 2]
>> +    default: 1
> 
> Why make this property a uint32, rather than a string where you can use
> something like "precharge", "full" and "bypass" (or "disabled")? The
> next similar device could use something slightly different then the
> binding becomes pretty clunky.
> Can you explain why this is a dt property rather than something
> adjustable at runtime?
> 
> Otherwise, what you have here looks sane enough to me - but I'd like to
> see some comments from Jonathan or David etc about your approach to the
> excitation properties.

This looks like something that should be in the devicetree to me. For example
if the external reference supply is high impedance, buffering is pretty
much required. And using precharge is an application design choice to
reduce THD at the expense of other limitations.


> 
> Cheers,
> Conor.
> 
>> +
>> +properties:

...

>> +allOf:
>> +  # Some devices don't have integrated DAC
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - adi,ad4190
>> +              - adi,ad4195
>> +    then:
>> +      properties:
>> +        ldac-gpios: false
>> +
>> +  # Require to specify the interrupt pin when using interrupts
>> +  - if:
>> +      required:
>> +        - interrupts
>> +    then:
>> +      required:
>> +        - interrupt-names
>> +
>> +  # If an external clock is set, the internal clock cannot go out and vice versa
>> +  - oneOf:
>> +      - required: [clocks]
>> +        properties:
>> +          '#clock-cells': false
>> +      - required: ['#clock-cells']
>> +        properties:
>> +          clocks: false
>> +
>> +patternProperties:

...

>> +required:
>> +  - compatible
>> +  - reg
>> +  - avdd-supply
>> +  - iovdd-supply
>> +  - spi-cpol
>> +  - spi-cpha
>> +
>> +unevaluatedProperties: false
>> +

It would be more logical to place these before patternProperties (actually
really before allOf) so that they are close to the properties that they are
referencing.
Re: [PATCH v5 01/11] dt-bindings: iio: adc: Add AD4170
Posted by Jonathan Cameron 3 months, 3 weeks ago
> >> +
> >> +$defs:
> >> +  reference-buffer:
> >> +    description: |
> >> +      Enable precharge buffer, full buffer, or skip reference buffering of
> >> +      the positive/negative voltage reference. Because the output impedance
> >> +      of the source driving the voltage reference inputs may be dynamic,
> >> +      resistive/capacitive 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 REFINn+ and REFINn-
> >> +      inputs 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. The valid options for this property are:
> >> +      0: Reference precharge buffer.
> >> +      1: Full reference buffering.
> >> +      2: Bypass reference buffers (buffering disabled).
> >> +    $ref: /schemas/types.yaml#/definitions/uint32
> >> +    enum: [0, 1, 2]
> >> +    default: 1  
> > 
> > Why make this property a uint32, rather than a string where you can use
> > something like "precharge", "full" and "bypass" (or "disabled")? The
> > next similar device could use something slightly different then the
> > binding becomes pretty clunky.
> > Can you explain why this is a dt property rather than something
> > adjustable at runtime?
> > 
> > Otherwise, what you have here looks sane enough to me - but I'd like to
> > see some comments from Jonathan or David etc about your approach to the
> > excitation properties.  
> 
> This looks like something that should be in the devicetree to me. For example
> if the external reference supply is high impedance, buffering is pretty
> much required. And using precharge is an application design choice to
> reduce THD at the expense of other limitations.
> 

Agreed that this pretty much only makes sense in DT.

In the ideal world we would have firm rules on when to enable buffering
etc and then the DT would describe the impedance of the circuit connected
and any other relevant properties and then we'd have the driver enable it
only when those rigid rules dictated that we should.

Sadly no such simple rules exist (as far as I know) so we just expose the thing
that gets set dependent on someone's judgement of the suitability of
the buffering choice given the circuit being connected to the input.

If we pushed it to userspace we'd just end up with a per device blob
that dictated the mode to pick on boot and left it like that.  So effectively
another bit of firmware :(


J

<cropping other comments>
Re: [PATCH v5 01/11] dt-bindings: iio: adc: Add AD4170
Posted by Conor Dooley 3 months, 2 weeks ago
On Sat, Jun 21, 2025 at 05:28:08PM +0100, Jonathan Cameron wrote:
> 
> > >> +
> > >> +$defs:
> > >> +  reference-buffer:
> > >> +    description: |
> > >> +      Enable precharge buffer, full buffer, or skip reference buffering of
> > >> +      the positive/negative voltage reference. Because the output impedance
> > >> +      of the source driving the voltage reference inputs may be dynamic,
> > >> +      resistive/capacitive 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 REFINn+ and REFINn-
> > >> +      inputs 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. The valid options for this property are:
> > >> +      0: Reference precharge buffer.
> > >> +      1: Full reference buffering.
> > >> +      2: Bypass reference buffers (buffering disabled).
> > >> +    $ref: /schemas/types.yaml#/definitions/uint32
> > >> +    enum: [0, 1, 2]
> > >> +    default: 1  
> > > 
> > > Why make this property a uint32, rather than a string where you can use
> > > something like "precharge", "full" and "bypass" (or "disabled")? The
> > > next similar device could use something slightly different then the
> > > binding becomes pretty clunky.
> > > Can you explain why this is a dt property rather than something
> > > adjustable at runtime?
> > > 
> > > Otherwise, what you have here looks sane enough to me - but I'd like to
> > > see some comments from Jonathan or David etc about your approach to the
> > > excitation properties.  
> > 
> > This looks like something that should be in the devicetree to me. For example
> > if the external reference supply is high impedance, buffering is pretty
> > much required. And using precharge is an application design choice to
> > reduce THD at the expense of other limitations.
> > 
> 
> Agreed that this pretty much only makes sense in DT.
> 
> In the ideal world we would have firm rules on when to enable buffering
> etc and then the DT would describe the impedance of the circuit connected
> and any other relevant properties and then we'd have the driver enable it
> only when those rigid rules dictated that we should.
> 
> Sadly no such simple rules exist (as far as I know) so we just expose the thing
> that gets set dependent on someone's judgement of the suitability of
> the buffering choice given the circuit being connected to the input.
> 
> If we pushed it to userspace we'd just end up with a per device blob
> that dictated the mode to pick on boot and left it like that.  So effectively
> another bit of firmware :(

Can't remember if I replied to David's mail here or not, I pretty much
just asked the question because I didn't understand why the usecase and
wanted an explanation for my benefit. Probably makes no sense to explain
why a knob exists in the binding when anyone using the device will not
need one, but that does mean that at times with these devices that are
unfamiliar to me I have to have it explained to me why something is set
in stone.
Re: [PATCH v5 01/11] dt-bindings: iio: adc: Add AD4170
Posted by Marcelo Schmitt 3 months, 3 weeks ago
On 06/16, Conor Dooley wrote:
> On Tue, Jun 10, 2025 at 05:31:04PM -0300, 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>
> > ---
...
> > +
> > +$defs:
> > +  reference-buffer:
> > +    description: |
> > +      Enable precharge buffer, full buffer, or skip reference buffering of
> > +      the positive/negative voltage reference. Because the output impedance
> > +      of the source driving the voltage reference inputs may be dynamic,
> > +      resistive/capacitive 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 REFINn+ and REFINn-
> > +      inputs 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. The valid options for this property are:
> > +      0: Reference precharge buffer.
> > +      1: Full reference buffering.
> > +      2: Bypass reference buffers (buffering disabled).
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [0, 1, 2]
> > +    default: 1
> 
> Why make this property a uint32, rather than a string where you can use
> something like "precharge", "full" and "bypass" (or "disabled")? The
> next similar device could use something slightly different then the
> binding becomes pretty clunky.

Oh, good point. Will make it string type (if going to keep the property).

> Can you explain why this is a dt property rather than something
> adjustable at runtime?

The reference buffer configuration affects the allowed absolute maximum input
ratings of voltage reference supplies. Some bindings (adi,ad7192, adi,ad4130,
adi,ad7124) have dt properties for buffering of analog inputs and adi,max11410
has a prop for reference buffering. It looked like adi,ad4170 having a dt prop
for reference buf would make it more consistent with other bindings. Though, I'm
fine with dropping ad4170 reference buffer props if that would be better.

Thanks,
Marcelo