iio: adc: Add support for AD4170 series of ADCs

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 <[email protected]>

---
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

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml new file +29 -0

new file mode 100644
index 000000000000..e3249ec56a14
@@ -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 <[email protected]>
+
+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

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml new file +525 -0

+
+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;
+            };
+        };
+    };
+...
+

MAINTAINERS +7 -0

index abfd5ded8735..44735314a43e 100644
@@ -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 <[email protected]>
+L:	[email protected]
+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 <[email protected]>
 M:	Nuno Sá <[email protected]>

Signature · 1 line

2.47.2

Quoted 5 lines · Marcelo Schmitt

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 <[email protected]>

---

Quoted 10 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml new file +10 -0

+
+$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

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml line 39

Quoted 10 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml new file +10 -0

+      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?

Quoted 72 lines · Marcelo Schmitt

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 <[email protected]>

---
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

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml new file +29 -0

new file mode 100644
index 000000000000..e3249ec56a14
@@ -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 <[email protected]>
+
+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

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml line 39

Quoted 10 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml new file +10 -0

+      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.

Quoted 2 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml new file +2 -0

+
+properties:

Quoted 31 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml new file +31 -0

+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:

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml line 406

Quoted 10 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml new file +10 -0

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

Quoted 29 lines

Quoted 11 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml new file +10 -0

+
+$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

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml line 39

Quoted 10 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml new file +10 -0

+      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?

Quoted 3 lines

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.

Quoted 29 lines

Quoted 29 lines

Quoted 11 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml new file +10 -0

+
+$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

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml line 39

Quoted 10 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml new file +10 -0

+      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?

Quoted 9 lines

Quoted 3 lines

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 :(

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.

From: Ana-Maria Cusco <[email protected]>

The AD4170 is a multichannel, low noise, 24-bit precision sigma-delta analog to digital converter. The AD4170 design offers a flexible data acquisition solution with crosspoint multiplexed analog inputs, configurable ADC voltage reference inputs, ultra-low noise integrated PGA, digital filtering, wide range of configurable output data rates, internal oscillator and temperature sensor, four GPIOs, and integrated features for interfacing with load cell weigh scales, RTD, and thermocouple sensors.

Add basic support for the AD4170 ADC with the following features:
- Single-shot read.
- Analog front end PGA configuration.
- Differential and pseudo-differential input configuration.

+};

drivers/iio/adc/ad4170.c new file +20 -0

+	static_assert(sizeof(*a) ==
+		      sizeof(struct {
+				     u16 misc;
+				     u16 afe;
+				     u16 filter;
+				     u16 filter_fs;
+				     u32 offset;
+				     u32 gain;
+			     }));
+
+	if (a->misc != b->misc ||
+	    a->afe != b->afe ||
+	    a->filter != b->filter ||
+	    a->filter_fs != b->filter_fs ||
+	    a->offset != b->offset ||
+	    a->gain != b->gain)
+		return false;
+
+	return true;
+}

drivers/iio/adc/ad4170.c new file +4 -0

+	if (refp <= 0)
+		return dev_err_probe(dev, -EINVAL,
+				     "REF+ <= GND for pseudo-diff chan %u\n",
+				     ch_reg);

drivers/iio/adc/ad4170.c new file +25 -0

+
+	setup->afe |= FIELD_PREP(AD4170_AFE_REF_BUF_P_MSK, aux);
+
+	/* Optional negative reference buffering */
+	aux = AD4170_REF_BUF_FULL; /* Default to full precharge buffer enabled. */
+	fwnode_property_read_u32(child, "adi,negative-reference-buffer", &aux);
+	if (aux < AD4170_REF_BUF_PRE || aux > AD4170_REF_BUF_BYPASS)
+		return dev_err_probe(dev, -EINVAL,
+				     "Invalid adi,negative-reference-buffer: %u\n",
+				     aux);
+
+	setup->afe |= FIELD_PREP(AD4170_AFE_REF_BUF_M_MSK, aux);
+
+	/* Optional voltage reference selection */
+	aux = AD4170_REF_REFOUT; /* Default reference selection. */
+	fwnode_property_read_u32(child, "adi,reference-select", &aux);
+	if (aux > AD4170_REF_AVDD)
+		return dev_err_probe(dev, -EINVAL,
+				     "Invalid reference selected %u\n",
+				     aux);
+
+	setup->afe |= FIELD_PREP(AD4170_AFE_REF_SELECT_MSK, aux);
+
+	return 0;
+}

drivers/iio/adc/ad4170.c new file +7 -0

+static int ad4170_parse_channel_node(struct iio_dev *indio_dev,
+				     struct fwnode_handle *child,
+				     unsigned int chan_num)
+{
+	struct ad4170_state *st = iio_priv(indio_dev);
+	struct device *dev = &st->spi->dev;
+	struct ad4170_chan_info *chan_info;

drivers/iio/adc/ad4170.c new file +13 -0

+				     "Failed to read channel reg\n");
+
+	if (ch_reg >= AD4170_MAX_CHANNELS)
+		return dev_err_probe(dev, -EINVAL,
+				     "Channel idx greater than no of channels\n");
+
+	chan = &st->chans[chan_num];
+	*chan = ad4170_channel_template;
+
+	chan->address = ch_reg;
+	chan->scan_index = ch_reg;
+	chan_info = &st->chan_infos[chan->address];
+

drivers/iio/adc/ad4170.c new file +10 -0

+		return ret;
+
+	ref_select = FIELD_GET(AD4170_AFE_REF_SELECT_MSK, setup->afe);
+	ret = ad4170_get_input_range(st, chan, ch_reg, ref_select);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Invalid input config\n");
+
+	chan_info->input_range_uv = ret;
+	return 0;
+}

drivers/iio/adc/ad4170.c new file +14 -0

+	if (num_channels > AD4170_MAX_CHANNELS)
+		return dev_err_probe(dev, -EINVAL, "Too many channels\n");
+
+	device_for_each_child_node_scoped(dev, child) {
+		ret = ad4170_parse_channel_node(indio_dev, child, chan_num++);
+		if (ret)
+			return ret;
+	}
+
+	indio_dev->num_channels = num_channels;
+	indio_dev->channels = st->chans;
+
+	return 0;
+}

Quoted 10 lines · Marcelo Schmitt

From: Ana-Maria Cusco <[email protected]>

The AD4170 is a multichannel, low noise, 24-bit precision sigma-delta analog to digital converter. The AD4170 design offers a flexible data acquisition solution with crosspoint multiplexed analog inputs, configurable ADC voltage reference inputs, ultra-low noise integrated PGA, digital filtering, wide range of configurable output data rates, internal oscillator and temperature sensor, four GPIOs, and integrated features for interfacing with load cell weigh scales, RTD, and thermocouple sensors.

Quoted 4 lines

Add basic support for the AD4170 ADC with the following features:
- Single-shot read.
- Analog front end PGA configuration.
- Differential and pseudo-differential input configuration.

...

drivers/iio/adc/ad4170.c line 331

Quoted 1 line

drivers/iio/adc/ad4170.c new file +1 -0

+	return spi_write(st->spi, st->tx_buf, size + 2);

... + sizeof(reg) ?

...

drivers/iio/adc/ad4170.c line 387

Quoted 8 lines

drivers/iio/adc/ad4170.c new file +8 -0

+static bool ad4170_setup_eq(struct ad4170_setup *a, struct ad4170_setup *b)
+{
+	/*
+	 * The use of static_assert() here is to make sure that, if
+	 * struct ad4170_setup is ever changed (e.g. a field is added to the
+	 * struct's declaration), the comparison below is adapted to keep
+	 * comparing each of struct ad4170_setup fields.
+	 */

Okay. But this also will trigger the case when the field just changes the type. So, it also brings false positives. I really think this is wrong place to put static_assert(). To me it looks like a solving rare problem, if any.

But I leave this to the IIO maintainers.

In my opinion static_assert() makes only sense when memcmp() is being used. Otherwise it has prons and cons.

Quoted 21 lines

drivers/iio/adc/ad4170.c new file +20 -0

+	static_assert(sizeof(*a) ==
+		      sizeof(struct {
+				     u16 misc;
+				     u16 afe;
+				     u16 filter;
+				     u16 filter_fs;
+				     u32 offset;
+				     u32 gain;
+			     }));
+
+	if (a->misc != b->misc ||
+	    a->afe != b->afe ||
+	    a->filter != b->filter ||
+	    a->filter_fs != b->filter_fs ||
+	    a->offset != b->offset ||
+	    a->gain != b->gain)
+		return false;
+
+	return true;
+}

...

drivers/iio/adc/ad4170.c line 825

Quoted 9 lines

drivers/iio/adc/ad4170.c new file +9 -0

+	/*
+	 * Some configurations can lead to invalid setups.
+	 * For example, if AVSS = -2.5V, REF_SELECT set to REFOUT (REFOUT/AVSS),
+	 * and pseudo-diff channel configuration set, then the input range
+	 * should go from 0V to +VREF (single-ended - datasheet pg 10), but
+	 * REFOUT/AVSS range would be -2.5V to 0V.
+	 * Check the positive reference is higher than 0V for pseudo-diff
+	 * channels.
+	 */

Right, the Q is, can refp contain an error code here, rather than negative value? The code above hints that in some case it may, but are all those cases were caught up already? (Comment can be extended to explain this)

Quoted 5 lines

drivers/iio/adc/ad4170.c new file +4 -0

+	if (refp <= 0)
+		return dev_err_probe(dev, -EINVAL,
+				     "REF+ <= GND for pseudo-diff chan %u\n",
+				     ch_reg);

drivers/iio/adc/ad4170.c line 1416

Quoted 3 lines

drivers/iio/adc/ad4170.c new file +2 -0

+	/* Assume AVSS at GND (0V) if not provided */
+	st->vrefs_uv[AD4170_AVSS_SUP] = ret == -ENODEV ? 0 : -ret;

-ret ?!?!

Even if you know that *now* it can't have any other error code, it's quite fragile.

Quoted 1 line · Andy Shevchenko

On Tue, Jun 10, 2025 at 05:31:25PM -0300, Marcelo Schmitt wrote:

Quoted 3 lines

drivers/iio/adc/ad4170.c line 331

Quoted 1 line

drivers/iio/adc/ad4170.c new file +1 -0

+	return spi_write(st->spi, st->tx_buf, size + 2);

... + sizeof(reg) ?

The size of the specific ADC register is stored in the size variable. The result of sizeof(reg) can be different on different machines and will probably not be equal to the size of the register in the ADC chip.

Quoted 4 lines

Quoted 4 lines

drivers/iio/adc/ad4170.c new file +4 -0

+static bool ad4170_setup_eq(struct ad4170_setup *a, struct ad4170_setup *b)
+{
+	/*
+	 * The use of static_assert() here is to make sure that, if

Quoted 8 lines

drivers/iio/adc/ad4170.c line 387

Quoted 4 lines

drivers/iio/adc/ad4170.c new file +4 -0

+	 * struct ad4170_setup is ever changed (e.g. a field is added to the
+	 * struct's declaration), the comparison below is adapted to keep
+	 * comparing each of struct ad4170_setup fields.
+	 */

Okay. But this also will trigger the case when the field just changes the type. So, it also brings false positives. I really think this is wrong place to put static_assert(). To me it looks like a solving rare problem, if any.

I think it is unlikely that struct ad4170_setup declaration will ever change. The fields match the registers that are associated with a channel setup and the their types match the size of the respective registers. So, I do agree that triggering this assert would be something rare.

Quoted 4 lines

But I leave this to the IIO maintainers.

In my opinion static_assert() makes only sense when memcmp() is being used. Otherwise it has prons and cons.

I think the most relevant reason to have this static_assert would be to keep some consistency with ad4130, ad7124, and ad7173, but no strong opinion about it.

Actually, I don't get why static_assert() would only matter if memcmp() was being used. Would it be better to not bother if the fields change type?

Anyway, I'll go with whatever be IIO maintainer's preference.

Quoted 20 lines

Quoted 20 lines

drivers/iio/adc/ad4170.c new file +20 -0

+	static_assert(sizeof(*a) ==
+		      sizeof(struct {
+				     u16 misc;
+				     u16 afe;
+				     u16 filter;
+				     u16 filter_fs;
+				     u32 offset;
+				     u32 gain;
+			     }));
+
+	if (a->misc != b->misc ||
+	    a->afe != b->afe ||
+	    a->filter != b->filter ||
+	    a->filter_fs != b->filter_fs ||
+	    a->offset != b->offset ||
+	    a->gain != b->gain)
+		return false;
+
+	return true;
+}

Quoted 4 lines

drivers/iio/adc/ad4170.c line 1416

Quoted 2 lines

drivers/iio/adc/ad4170.c new file +2 -0

+	/* Assume AVSS at GND (0V) if not provided */
+	st->vrefs_uv[AD4170_AVSS_SUP] = ret == -ENODEV ? 0 : -ret;

-ret ?!?!

That's because AVSS is never above system ground level (i.e. AVSS is either GND or a negative voltage). But we currently don't have support for reading negative voltages with the regulator framework. So, the current AD4170 support reads a positive value from the regulator, then inverts signal to make it negative :)

Quoted 2 lines

Even if you know that *now* it can't have any other error code, it's quite fragile.

Yeah, I guess ADCs that can take bipolar power supplies are not that common. I couldn't think of any better way to have that, though.

Quoted 2 lines · Marcelo Schmitt

Quoted 1 line · Andy Shevchenko

On Tue, Jun 10, 2025 at 05:31:25PM -0300, Marcelo Schmitt wrote:

...

Quoted 4 lines

Quoted 3 lines

drivers/iio/adc/ad4170.c line 331

Quoted 1 line

drivers/iio/adc/ad4170.c new file +1 -0

+	return spi_write(st->spi, st->tx_buf, size + 2);

... + sizeof(reg) ?

Quoted 3 lines

The size of the specific ADC register is stored in the size variable. The result of sizeof(reg) can be different on different machines and will probably not be equal to the size of the register in the ADC chip.

Hmm... But shouldn't we have a variable type that respects the sizeof() of the register in HW to keep it there? 2 is magic.

Quoted 6 lines

Quoted 5 lines

drivers/iio/adc/ad4170.c line 1416

Quoted 3 lines

drivers/iio/adc/ad4170.c new file +2 -0

+	/* Assume AVSS at GND (0V) if not provided */
+	st->vrefs_uv[AD4170_AVSS_SUP] = ret == -ENODEV ? 0 : -ret;

-ret ?!?!

Quoted 4 lines

That's because AVSS is never above system ground level (i.e. AVSS is either GND or a negative voltage). But we currently don't have support for reading negative voltages with the regulator framework. So, the current AD4170 support reads a positive value from the regulator, then inverts signal to make it negative :)

This needs a good comment and ideally a TODO item in the regulator framework. (It might be easy to implement by adding a flag without changing the type of
the field, if it's unsigned.)

Quoted 3 lines · Andy Shevchenko

Quoted 2 lines · Marcelo Schmitt

Quoted 1 line · Andy Shevchenko

On Tue, Jun 10, 2025 at 05:31:25PM -0300, Marcelo Schmitt wrote:

Not sure about sending a patch only adding a TODO to the regulator framework. Aren't developers expected to propose things?
I'm anticipating 'talk is cheap, show me the code' coming.

Quoted 4 lines · Marcelo Schmitt

Quoted 3 lines · Andy Shevchenko

Quoted 2 lines · Marcelo Schmitt

Quoted 1 line · Andy Shevchenko

On Tue, Jun 10, 2025 at 05:31:25PM -0300, Marcelo Schmitt wrote:

...

Quoted 3 lines

Not sure about sending a patch only adding a TODO to the regulator framework. Aren't developers expected to propose things?
I'm anticipating 'talk is cheap, show me the code' coming.

This maybe done in a form of the discussion started with the maintainers and stakeholders of regulator framework. It doesn't mean we must have something in the form of the patch right now.

Quoted 2 lines · Marcelo Schmitt

Quoted 1 line · Andy Shevchenko

On Tue, Jun 10, 2025 at 05:31:25PM -0300, Marcelo Schmitt wrote:

...

Quoted 7 lines

Quoted 3 lines

drivers/iio/adc/ad4170.c line 331

Quoted 1 line

drivers/iio/adc/ad4170.c new file +1 -0

+	return spi_write(st->spi, st->tx_buf, size + 2);  

... + sizeof(reg) ?

The size of the specific ADC register is stored in the size variable. The result of sizeof(reg) can be different on different machines and will probably not be equal to the size of the register in the ADC chip.

Hmm... But shouldn't we have a variable type that respects the sizeof() of the register in HW to keep it there? 2 is magic.

...

Quoted 13 lines

Quoted 12 lines

drivers/iio/adc/ad4170.c line 387

Quoted 8 lines

drivers/iio/adc/ad4170.c new file +8 -0

+static bool ad4170_setup_eq(struct ad4170_setup *a, struct ad4170_setup *b)
+{
+	/*
+	 * The use of static_assert() here is to make sure that, if
+	 * struct ad4170_setup is ever changed (e.g. a field is added to the
+	 * struct's declaration), the comparison below is adapted to keep
+	 * comparing each of struct ad4170_setup fields.
+	 */  

Okay. But this also will trigger the case when the field just changes the type. So, it also brings false positives. I really think this is wrong place to put static_assert(). To me it looks like a solving rare problem, if any.

Quoted 4 lines

I think it is unlikely that struct ad4170_setup declaration will ever change. The fields match the registers that are associated with a channel setup and the their types match the size of the respective registers. So, I do agree that triggering this assert would be something rare.

Yep, which thinks to me as an unneeded noise in the code, making it harder to read and maintain (in _this_ case).

From: Ana-Maria Cusco <[email protected]>

The AD4170 is a multichannel, low noise, 24-bit precision sigma-delta analog to digital converter. The AD4170 design offers a flexible data acquisition solution with crosspoint multiplexed analog inputs, configurable ADC voltage reference inputs, ultra-low noise integrated PGA, digital filtering, wide range of configurable output data rates, internal oscillator and temperature sensor, four GPIOs, and integrated features for interfacing with load cell weigh scales, RTD, and thermocouple sensors.

Add basic support for the AD4170 ADC with the following features:
- Single-shot read.
- Analog front end PGA configuration.
- Differential and pseudo-differential input configuration.

drivers/iio/adc/ad4170.c new file +7 -0

+static int ad4170_fill_scale_tbl(struct iio_dev *indio_dev,
+				 struct iio_chan_spec const *chan)
+{
+	struct ad4170_state *st = iio_priv(indio_dev);
+	struct ad4170_chan_info *chan_info = &st->chan_infos[chan->address];
+	struct device *dev = &st->spi->dev;
+	int bipolar = chan->scan_type.sign == 's' ? 1 : 0;

drivers/iio/adc/ad4170.c new file +65 -0

+		u64 nv;
+		unsigned int lshift, rshift;
+
+		/*
+		 * The PGA options are numbered from 0 to 9, with option 0 being
+		 * a gain of 2^0 (no actual gain), and 7 meaning a gain of 2^7.
+		 * Option 8, though, sets a gain of 0.5, so the input signal can
+		 * be attenuated by 2 rather than amplified. Option 9, allows
+		 * the signal to bypass the PGA circuitry (no gain).
+		 *
+		 * The scale factor to get ADC output codes to values in mV
+		 * units is given by:
+		 * _scale = (input_range / gain) / 2^precision
+		 * AD4170 gain is a power of 2 so the above can be written as
+		 * _scale = input_range / 2^(precision + gain)
+		 * Keep the input range in µV to avoid truncating the less
+		 * significan bits when right shifting it so to preserve scale
+		 * precision.
+		 */
+		nv = (u64)chan_info->input_range_uv * NANO;
+		lshift = !!(pga & BIT(3)); /* handle options 8 and 9 */
+		rshift = precision_bits - bipolar + (pga & GENMASK(2, 0)) - lshift;
+		chan_info->scale_tbl[pga][0] = 0;
+		chan_info->scale_tbl[pga][1] = div_u64(nv >> rshift, MILLI);
+
+		/*
+		 * If the negative input is not at GND, the conversion result
+		 * (which is relative to IN-) will be offset by the level at IN-.
+		 * Use the scale factor the other way around to go from a known
+		 * voltage to the corresponding ADC output code.
+		 * With that, we are able to get to what would be the output
+		 * code for the voltage at the negative input.
+		 * If the negative input is not fixed, there is no offset.
+		 */
+		offset = ((unsigned long long)abs(ainm_voltage)) * MICRO;
+		offset = DIV_ROUND_CLOSEST_ULL(offset, chan_info->scale_tbl[pga][1]);
+
+		/*
+		 * After divided by the scale, offset will always fit into 31
+		 * bits. For _raw + _offset to be relative to GND, the value
+		 * provided as _offset is of opposite sign than the real offset.
+		 */
+		if (ainm_voltage > 0)
+			chan_info->offset_tbl[pga] = -(int)(offset);
+		else
+			chan_info->offset_tbl[pga] = (int)(offset);
+	}
+	return 0;
+}
+
+static int ad4170_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long info)
+{
+	struct ad4170_state *st = iio_priv(indio_dev);
+	struct ad4170_chan_info *chan_info = &st->chan_infos[chan->address];
+
+	switch (info) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (int *)chan_info->scale_tbl;
+		*length = ARRAY_SIZE(chan_info->scale_tbl) * 2;
+		*type = IIO_VAL_INT_PLUS_NANO;
+		return IIO_AVAIL_LIST;
+	default:

drivers/iio/adc/ad4170.c new file +17 -0

+
+	for (pga = 0; pga < AD4170_NUM_PGA_OPTIONS; pga++) {
+		if (val == chan_info->scale_tbl[pga][0] &&
+		    val2 == chan_info->scale_tbl[pga][1])
+			break;
+	}
+
+	if (pga == AD4170_NUM_PGA_OPTIONS)
+		return -EINVAL;
+
+	guard(mutex)(&st->lock);
+	setup->afe &= ~AD4170_AFE_PGA_GAIN_MSK;
+	setup->afe |= FIELD_PREP(AD4170_AFE_PGA_GAIN_MSK, pga);
+
+	return ad4170_write_channel_setup(st, chan->address, false);
+}
+

Quoted 15 lines · Marcelo Schmitt

From: Ana-Maria Cusco <[email protected]>

The AD4170 is a multichannel, low noise, 24-bit precision sigma-delta analog to digital converter. The AD4170 design offers a flexible data acquisition solution with crosspoint multiplexed analog inputs, configurable ADC voltage reference inputs, ultra-low noise integrated PGA, digital filtering, wide range of configurable output data rates, internal oscillator and temperature sensor, four GPIOs, and integrated features for interfacing with load cell weigh scales, RTD, and thermocouple sensors.

Add basic support for the AD4170 ADC with the following features:
- Single-shot read.
- Analog front end PGA configuration.
- Differential and pseudo-differential input configuration.

...

Quoted 13 lines

drivers/iio/adc/ad4170.c new file +13 -0

+static int ad4170_fill_scale_tbl(struct iio_dev *indio_dev,
+				 struct iio_chan_spec const *chan)
+{
+	struct ad4170_state *st = iio_priv(indio_dev);
+	struct ad4170_chan_info *chan_info = &st->chan_infos[chan->address];
+	struct device *dev = &st->spi->dev;
+	int bipolar = chan->scan_type.sign == 's' ? 1 : 0;
+	int precision_bits = chan->scan_type.realbits;
+	int pga, ainm_voltage, ret;
+	unsigned long long offset;
+
+	ainm_voltage = 0;
+	ret = ad4170_get_ain_voltage_uv(st, chan->channel2, &ainm_voltage);

drivers/iio/adc/ad4170.c line 974

Quoted 4 lines

drivers/iio/adc/ad4170.c new file +4 -0

+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to fill scale table\n");
+
+	for (pga = 0; pga < AD4170_NUM_PGA_OPTIONS; pga++) {

From what I read in the datasheet, it sounds like if adi,reference-buffer is precharge, then the PGA is bypassed, so there would only be 1 option in that case (gain = 1).

Hi Marcelo,

kernel test robot noticed the following build warnings:

url:    https://github.com/intel-lab-lkp/linux/commits/Marcelo-Schmitt/dt-bindings-iio-adc-Add-AD4170/20250611-101842
base:   4c6073fec2fee4827fa0dd8a4ab4e6f7bbc05ee6
patch link:    https://lore.kernel.org/r/48598c0753cccf515addbe85acba3f883ff8f036.1749582679.git.marcelo.schmitt%40analog.com
patch subject: [PATCH v5 02/11] iio: adc: Add basic support for AD4170
config: powerpc-randconfig-r072-20250613 (https://download.01.org/0day-ci/archive/20250614/[email protected]/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags

| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/

smatch warnings:
drivers/iio/adc/ad4170.c:1181 ad4170_parse_adc_channel_type() warn: passing zero to 'dev_err_probe'

vim +/dev_err_probe +1181 drivers/iio/adc/ad4170.c

dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1170  static int ad4170_parse_adc_channel_type(struct device *dev,
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1171  					 struct fwnode_handle *child,
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1172  					 struct iio_chan_spec *chan)
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1173  {
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1174  	int ret, ret2;
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1175  	u32 pins[2];
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1176  
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1177  	/* Parse pseudo-differential channel configuration */
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1178  	ret = fwnode_property_read_u32(child, "single-channel", &pins[0]);
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1179  	ret2 = fwnode_property_read_u32(child, "common-mode-channel", &pins[1]);
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1180  	if (!ret && ret2)
dfefd2b2405829 Ana-Maria Cusco 2025-06-10 @1181  		return dev_err_probe(dev, ret,
                                                                                          ^^^

ret is zero, so this returns success. s/ret/ret2/.

dfefd2b2405829 Ana-Maria Cusco 2025-06-10 1182 "single-ended channels must define common-mode-channel\n");

dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1184  	if (!ret && !ret2) {
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1185  		chan->differential = false;
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1186  		chan->channel = pins[0];
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1187  		chan->channel2 = pins[1];
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1188  		return 0;
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1189  	}
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1190  	/* Failed to parse pseudo-diff chan props so try diff chan */
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1191  
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1192  	/* Parse differential channel configuration */
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1193  	ret = fwnode_property_read_u32_array(child, "diff-channels", pins,
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1194  					     ARRAY_SIZE(pins));
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1195  	if (!ret) {
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1196  		chan->differential = true;
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1197  		chan->channel = pins[0];
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1198  		chan->channel2 = pins[1];
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1199  		return 0;
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1200  	}
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1201  	return dev_err_probe(dev, ret,
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1202  		"Channel must define one of diff-channels or single-channel.\n");
dfefd2b2405829 Ana-Maria Cusco 2025-06-10  1203  }

Add support for sinc3, sinc5, and averaged sinc5 digital filters along with sample frequency configuration.

Signed-off-by Marcelo Schmitt <[email protected]>

---

drivers/iio/adc/ad4170.c +4 -0

+	[AD4170_SINC5] = "sinc5",
+	[AD4170_SINC3] = "sinc3",
+};
+

Extend the AD4170 driver to allow buffered data capture in continuous read mode. In continuous read mode, the chip skips the instruction phase and outputs just ADC sample data, enabling faster sample rates to be reached. The internal channel sequencer always starts sampling from channel 0 and channel 0 must be enabled if more than one channel is selected for data capture. The scan mask validation callback checks if the aforementioned condition is met.

drivers/iio/adc/ad4170.c +6 -0

+static bool ad4170_validate_scan_mask(struct iio_dev *indio_dev,
+				      const unsigned long *scan_mask)
+{
+	unsigned int masklength = iio_get_masklength(indio_dev);
+
+	/*

+}

The AD4170 chip can use an externally supplied clock at the XTAL2 pin, or an external crystal connected to the XTAL1 and XTAL2 pins. Alternatively, the AD4170 can provide its 16 MHz internal clock at the XTAL2 pin. In addition, the chip has a programmable clock divider that allows dividing the external or internal clock frequency, however, control for that is not provided in this patch. Extend the AD4170 driver so it effectively uses the provided external clock, if any, or supplies its own clock as a clock provider.

drivers/iio/adc/ad4170.c +9 -0

+static int ad4170_clock_select(struct iio_dev *indio_dev)
+{
+	struct ad4170_state *st = iio_priv(indio_dev);
+	struct device *dev = &st->spi->dev;
+	struct clk *ext_clk;
+	int ret;
+
+	ext_clk = devm_clk_get_optional_enabled(dev, NULL);
+	if (IS_ERR(ext_clk))

drivers/iio/adc/ad4170.c +24 -0

+			return 0;
+
+		return ad4170_register_clk_provider(indio_dev);
+	}
+
+	/* Read optional clock-names prop to specify the external clock type */
+	ret = device_property_match_property_string(dev, "clock-names",
+						    ad4170_clk_sel,
+						    ARRAY_SIZE(ad4170_clk_sel));
+
+	ret = ret < 0 ? 0 : ret; /* Default to external clock if no clock-names */
+	st->clock_ctrl |= FIELD_PREP(AD4170_CLOCK_CTRL_CLOCKSEL_MSK,
+				     AD4170_CLOCK_CTRL_CLOCKSEL_EXT + ret);
+
+	st->mclk_hz = clk_get_rate(ext_clk);
+	if (st->mclk_hz < AD4170_EXT_CLOCK_MHZ_MIN ||
+	    st->mclk_hz > AD4170_EXT_CLOCK_MHZ_MAX) {
+		return dev_err_probe(dev, -EINVAL,
+				     "Invalid external clock frequency %u\n",
+				     st->mclk_hz);
+	}
+
+	return 0;
+}

On 6/10/25 3:33 PM, Marcelo Schmitt wrote:

Quoted 1 line

+ if (!device_property_read_bool(&st->spi->dev, "#clock-cells"))

This isn't a flag, so device_property_present() is probably more correct.

Quoted 1 line

+ return 0;

The AD4170 has four multifunctional pins that can be used as GPIOs. The GPIO functionality can be accessed when the AD4170 chip is not busy performing continuous data capture or handling any other register read/write request. Also, the AD4170 does not provide any interrupt based on GPIO pin states so AD4170 GPIOs can't be used as interrupt sources.

Implement gpio_chip callbacks to make AD4170 GPIO pins controllable through the gpiochip interface.

Acked-by Bartosz Golaszewski <[email protected]>

Signed-off-by Marcelo Schmitt <[email protected]>