[PATCH v3 0/3] iio: adc: Add AD4134 minimum I/O support

[PATCH v3 0/3] iio: adc: Add AD4134 minimum I/O support

[Marcelo Schmitt]

This patch series adds basic support for ad4134. AD4134 is a very flexible
device that can be configured in many different ways. This series aims to
support the simplest way of interfacing with AD4134 which is called minimum I/O
mode in data sheet. This is essentially usual SPI with the addition of an ODR
(Output Data Rate) GPIO which functions as conversion start signal in minimum
I/O mode.

This set provides just one feature:
- Single-shot ADC sample read.

[PATCH 1] Device tree documentation for AD4134.
[PATCH 2] IIO Linux driver for AD4134.
[PATCH 3] Initial IIO documentation.

Change log v2 -> v3:
[device tree]
- fixed typo in powerdown-gpios description.
- picked up Conor's review tag. 
[IIO]
- Dropped leftover dev field from struct ad4134_state.
- Inverted if condition and reduced indentation in ad4134_regulator_setup().
- Use (MICRO / MILLI) for microvolt/milivolt conversion.
- Handled clock deferred probe cases.

Link to v2: https://lore.kernel.org/linux-iio/cover.1763478299.git.marcelo.schmitt@analog.com/
Link to v1: https://lore.kernel.org/linux-iio/cover.1762777931.git.marcelo.schmitt@analog.com/

With best regards,
Marcelo

Marcelo Schmitt (3):
  dt-bindings: iio: adc: Add AD4134
  iio: adc: Initial support for AD4134
  Docs: iio: Add AD4134

 .../bindings/iio/adc/adi,ad4134.yaml          | 198 +++++++
 Documentation/iio/ad4134.rst                  |  58 ++
 Documentation/iio/index.rst                   |   1 +
 MAINTAINERS                                   |   9 +
 drivers/iio/adc/Kconfig                       |  11 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ad4134.c                      | 503 ++++++++++++++++++
 7 files changed, 781 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml
 create mode 100644 Documentation/iio/ad4134.rst
 create mode 100644 drivers/iio/adc/ad4134.c


base-commit: f9e05791642810a0cf6237d39fafd6fec5e0b4bb
-- 
2.51.0

[PATCH v3 1/3] dt-bindings: iio: adc: Add AD4134

[Marcelo Schmitt]

Add device tree documentation for AD4134 24-Bit, 4-channel simultaneous
sampling, precision ADC.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
Change log v2 -> v3:
- fixed typo in powerdown-gpios description.
- picked up Conor's review tag. 

 .../bindings/iio/adc/adi,ad4134.yaml          | 198 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 205 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml
new file mode 100644
index 000000000000..69a6ddf6ca92
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml
@@ -0,0 +1,198 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad4134.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD4134 ADC
+
+maintainers:
+  - Marcelo Schmitt <marcelo.schmitt@analog.com>
+
+description: |
+  The AD4134 is a quad channel, low noise, simultaneous sampling, precision
+  analog-to-digital converter (ADC).
+  Specifications can be found at:
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4134.pdf
+
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - adi,ad4134
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 50000000
+
+  avdd5-supply:
+    description: A 5V supply that powers the chip's analog circuitry.
+
+  dvdd5-supply:
+    description: A 5V supply that powers the chip's digital circuitry.
+
+  iovdd-supply:
+    description:
+      A 1.8V supply that sets the logic levels for the digital interface pins.
+
+  refin-supply:
+    description:
+      A 4.096V or 5V supply that serves as reference for ADC conversions.
+
+  avdd1v8-supply:
+    description: A 1.8V supply used by the analog circuitry.
+
+  dvdd1v8-supply:
+    description: A 1.8V supply used by the digital circuitry.
+
+  clkvdd-supply:
+    description: A 1.8V supply for the chip's clock management circuit.
+
+  ldoin-supply:
+    description:
+      A 2.6V to 5.5V supply that generates 1.8V for AVDD1V8, DVDD1V8, and CLKVDD
+      pins.
+
+  clocks:
+    maxItems: 1
+    description:
+      Required external clock source. Can specify either a crystal or CMOS clock
+      source. If an external crystal is set, connect the CLKSEL pin to IOVDD.
+      Otherwise, connect the CLKSEL pin to IOGND and the external CMOS clock
+      signal to the XTAL2/CLKIN pin.
+
+  clock-names:
+    enum:
+      - xtal1-xtal2
+      - clkin
+    default: clkin
+
+  '#clock-cells':
+    const: 0
+
+  clock-output-names:
+    maxItems: 1
+
+  regulators:
+    type: object
+    description:
+      list of regulators provided by this controller.
+
+    properties:
+      vcm-output:
+        $ref: /schemas/regulator/regulator.yaml#
+        type: object
+        unevaluatedProperties: false
+
+    additionalProperties: false
+
+  reset-gpios:
+    maxItems: 1
+
+  powerdown-gpios:
+    description:
+      Active low GPIO connected to the /PDN pin. Forces the device into full
+      power-down mode when brought low. Pull this input to IOVDD for normal
+      operation.
+    maxItems: 1
+
+  odr-gpios:
+    description:
+      GPIO connected to ODR pin. Used to sample ADC data in minimum I/O mode.
+    maxItems: 1
+
+  adi,asrc-mode:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Asynchronous Sample Rate Converter (ASRC) operation mode control input.
+      Describes whether the MODE pin is set to a high level (for master mode
+      operation) or to a low level (for slave mode operation).
+    enum: [ high, low ]
+    default: low
+
+  adi,dclkio:
+    description:
+      DCLK pin I/O direction control for when the device operates in Pin Control
+      Slave Mode or in SPI Control Mode. Describes if DEC0/DCLKIO pin is at a
+      high level (which configures DCLK as an output) or to set to a low level
+      (configuring DCLK for input).
+    enum: [ out, in ]
+    default: in
+
+  adi,dclkmode:
+    description:
+      DCLK mode control for when the device operates in Pin Control Slave Mode
+      or in SPI Control Mode. Describes whether the DEC1/DCLKMODE pin is set to
+      a high level (configuring the DCLK to operate in free running mode) or
+      to a low level (to configure DCLK to operate in gated mode).
+    enum: [ free-running, gated ]
+    default: gated
+
+required:
+  - compatible
+  - reg
+  - avdd5-supply
+  - dvdd5-supply
+  - iovdd-supply
+  - refin-supply
+  - clocks
+  - clock-names
+
+allOf:
+  - if:
+      not:
+        required:
+          - ldoin-supply
+    then:
+      required:
+        - avdd1v8-supply
+        - dvdd1v8-supply
+        - clkvdd-supply
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            compatible = "adi,ad4134";
+            reg = <0>;
+
+            spi-max-frequency = <1000000>;
+
+            reset-gpios = <&gpio0 86 GPIO_ACTIVE_LOW>;
+            odr-gpios = <&gpio0 87 GPIO_ACTIVE_HIGH>;
+            powerdown-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
+
+            clocks = <&sys_clk>;
+            clock-names = "clkin";
+
+            avdd5-supply = <&avdd5>;
+            dvdd5-supply = <&dvdd5>;
+            iovdd-supply = <&iovdd>;
+            refin-supply = <&refin>;
+            avdd1v8-supply = <&avdd1v8>;
+            dvdd1v8-supply = <&dvdd1v8>;
+            clkvdd-supply = <&clkvdd>;
+
+            adi,asrc-mode = "low";
+            adi,dclkio = "in";
+            adi,dclkmode = "gated";
+
+            regulators {
+                vcm_reg: vcm-output {
+                    regulator-name = "ad4134-vcm";
+                };
+            };
+
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 31d98efb1ad1..b9029c4055e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1449,6 +1449,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 AD4134 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,ad4134.yaml
+
 ANALOG DEVICES INC AD4170-4 DRIVER
 M:	Marcelo Schmitt <marcelo.schmitt@analog.com>
 L:	linux-iio@vger.kernel.org
-- 
2.51.0

Re: [PATCH v3 1/3] dt-bindings: iio: adc: Add AD4134

[Tomas Melin]

Hi,

On 02/12/2025 22:55, Marcelo Schmitt wrote:
> Add device tree documentation for AD4134 24-Bit, 4-channel simultaneous
> sampling, precision ADC.
> 
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
> Change log v2 -> v3:
> - fixed typo in powerdown-gpios description.
> - picked up Conor's review tag. 
> 
>  .../bindings/iio/adc/adi,ad4134.yaml          | 198 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 205 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml
> new file mode 100644
> index 000000000000..69a6ddf6ca92
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml
> @@ -0,0 +1,198 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad4134.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD4134 ADC
> +
> +maintainers:
> +  - Marcelo Schmitt <marcelo.schmitt@analog.com>
> +
> +description: |
> +  The AD4134 is a quad channel, low noise, simultaneous sampling, precision
> +  analog-to-digital converter (ADC).
> +  Specifications can be found at:
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4134.pdf
> +
> +$ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +    maxItems: 1
> +
> +  adi,asrc-mode:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description:
> +      Asynchronous Sample Rate Converter (ASRC) operation mode control input.
> +      Describes whether the MODE pin is set to a high level (for master mode
> +      operation) or to a low level (for slave mode operation).
> +    enum: [ high, low ]
> +    default: low
Since minimim I/O mode is only one currently supported, this should
always be low, right? Is the property needed at this point?

> +
> +  adi,dclkio:
> +    description:
> +      DCLK pin I/O direction control for when the device operates in Pin Control
> +      Slave Mode or in SPI Control Mode. Describes if DEC0/DCLKIO pin is at a
> +      high level (which configures DCLK as an output) or to set to a low level
> +      (configuring DCLK for input).
> +    enum: [ out, in ]
> +    default: in
> +
> +  adi,dclkmode:
> +    description:
> +      DCLK mode control for when the device operates in Pin Control Slave Mode
> +      or in SPI Control Mode. Describes whether the DEC1/DCLKMODE pin is set to
> +      a high level (configuring the DCLK to operate in free running mode) or
> +      to a low level (to configure DCLK to operate in gated mode).
> +    enum: [ free-running, gated ]
> +    default: gated
In minimum I/O mode datasheet mentions this should always be gated.
Perhaps this and adi,dclkio can be left out and added when driver gains
other support than I/O mode?

Thanks,
Tomas


> +
> +required:
> +  - compatible
> +  - reg
> +  - avdd5-supply
> +  - dvdd5-supply
> +  - iovdd-supply
> +  - refin-supply
> +  - clocks
> +  - clock-names
> +
> +allOf:
> +  - if:
> +      not:
> +        required:
> +          - ldoin-supply
> +    then:
> +      required:
> +        - avdd1v8-supply
> +        - dvdd1v8-supply
> +        - clkvdd-supply
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +            compatible = "adi,ad4134";
> +            reg = <0>;
> +
> +            spi-max-frequency = <1000000>;
> +
> +            reset-gpios = <&gpio0 86 GPIO_ACTIVE_LOW>;
> +            odr-gpios = <&gpio0 87 GPIO_ACTIVE_HIGH>;
> +            powerdown-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> +
> +            clocks = <&sys_clk>;
> +            clock-names = "clkin";
> +
> +            avdd5-supply = <&avdd5>;
> +            dvdd5-supply = <&dvdd5>;
> +            iovdd-supply = <&iovdd>;
> +            refin-supply = <&refin>;
> +            avdd1v8-supply = <&avdd1v8>;
> +            dvdd1v8-supply = <&dvdd1v8>;
> +            clkvdd-supply = <&clkvdd>;
> +
> +            adi,asrc-mode = "low";
> +            adi,dclkio = "in";
> +            adi,dclkmode = "gated";
> +
> +            regulators {
> +                vcm_reg: vcm-output {
> +                    regulator-name = "ad4134-vcm";
> +                };
> +            };
> +
> +        };
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 31d98efb1ad1..b9029c4055e3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1449,6 +1449,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 AD4134 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,ad4134.yaml
> +
>  ANALOG DEVICES INC AD4170-4 DRIVER
>  M:	Marcelo Schmitt <marcelo.schmitt@analog.com>
>  L:	linux-iio@vger.kernel.org

Re: [PATCH v3 1/3] dt-bindings: iio: adc: Add AD4134

[Marcelo Schmitt]

On 12/05, Tomas Melin wrote:
> Hi,
> 
> On 02/12/2025 22:55, Marcelo Schmitt wrote:
> > Add device tree documentation for AD4134 24-Bit, 4-channel simultaneous
> > sampling, precision ADC.
> > 
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> > Change log v2 -> v3:
> > - fixed typo in powerdown-gpios description.
> > - picked up Conor's review tag. 
> > 
> >  .../bindings/iio/adc/adi,ad4134.yaml          | 198 ++++++++++++++++++
...
> > +description: |
> > +  The AD4134 is a quad channel, low noise, simultaneous sampling, precision
> > +  analog-to-digital converter (ADC).
> > +  Specifications can be found at:
> > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4134.pdf
> > +
> > +$ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +properties:
> > +    maxItems: 1
> > +
> > +  adi,asrc-mode:
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    description:
> > +      Asynchronous Sample Rate Converter (ASRC) operation mode control input.
> > +      Describes whether the MODE pin is set to a high level (for master mode
> > +      operation) or to a low level (for slave mode operation).
> > +    enum: [ high, low ]
> > +    default: low
> Since minimim I/O mode is only one currently supported, this should
> always be low, right? Is the property needed at this point?

Correct, it is expected that adi,asrc-mode will always be low for minimum I/O mode.
The property is not _needed_ but, according to dt-binding guidelines [1], it is
desired.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/writing-bindings.rst?h=v6.18#n17

> 
> > +
> > +  adi,dclkio:
> > +    description:
> > +      DCLK pin I/O direction control for when the device operates in Pin Control
> > +      Slave Mode or in SPI Control Mode. Describes if DEC0/DCLKIO pin is at a
> > +      high level (which configures DCLK as an output) or to set to a low level
> > +      (configuring DCLK for input).
> > +    enum: [ out, in ]
> > +    default: in
> > +
> > +  adi,dclkmode:
> > +    description:
> > +      DCLK mode control for when the device operates in Pin Control Slave Mode
> > +      or in SPI Control Mode. Describes whether the DEC1/DCLKMODE pin is set to
> > +      a high level (configuring the DCLK to operate in free running mode) or
> > +      to a low level (to configure DCLK to operate in gated mode).
> > +    enum: [ free-running, gated ]
> > +    default: gated
> In minimum I/O mode datasheet mentions this should always be gated.
> Perhaps this and adi,dclkio can be left out and added when driver gains
> other support than I/O mode?

Yes, that's also correct. A few properties are actually not needed for minimum
I/O mode (i.e. can have their values inferred from the minimum I/O mode requirements).
Sure, from developer's perspective it's easier to document only what the driver
uses. adi,dclkio, adi,dclkmode, adi,asrc-mode, powerdown-gpios, regulators, could
all be left to a latter time. Fine by me if dt maintainers agree with that.

With best regards,
Marcelo

Re: [PATCH v3 1/3] dt-bindings: iio: adc: Add AD4134

[Jonathan Cameron]

On Fri, 5 Dec 2025 09:50:08 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 12/05, Tomas Melin wrote:
> > Hi,
> > 
> > On 02/12/2025 22:55, Marcelo Schmitt wrote:  
> > > Add device tree documentation for AD4134 24-Bit, 4-channel simultaneous
> > > sampling, precision ADC.
> > > 
> > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > > ---
> > > Change log v2 -> v3:
> > > - fixed typo in powerdown-gpios description.
> > > - picked up Conor's review tag. 
> > > 
> > >  .../bindings/iio/adc/adi,ad4134.yaml          | 198 ++++++++++++++++++  
> ...
> > > +description: |
> > > +  The AD4134 is a quad channel, low noise, simultaneous sampling, precision
> > > +  analog-to-digital converter (ADC).
> > > +  Specifications can be found at:
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4134.pdf
> > > +
> > > +$ref: /schemas/spi/spi-peripheral-props.yaml#
> > > +
> > > +properties:
> > > +    maxItems: 1
> > > +
> > > +  adi,asrc-mode:
> > > +    $ref: /schemas/types.yaml#/definitions/string
> > > +    description:
> > > +      Asynchronous Sample Rate Converter (ASRC) operation mode control input.
> > > +      Describes whether the MODE pin is set to a high level (for master mode
> > > +      operation) or to a low level (for slave mode operation).
> > > +    enum: [ high, low ]
> > > +    default: low  
> > Since minimim I/O mode is only one currently supported, this should
> > always be low, right? Is the property needed at this point?  
> 
> Correct, it is expected that adi,asrc-mode will always be low for minimum I/O mode.
> The property is not _needed_ but, according to dt-binding guidelines [1], it is
> desired.
> 
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/writing-bindings.rst?h=v6.18#n17
> 
> >   
> > > +
> > > +  adi,dclkio:
> > > +    description:
> > > +      DCLK pin I/O direction control for when the device operates in Pin Control
> > > +      Slave Mode or in SPI Control Mode. Describes if DEC0/DCLKIO pin is at a
> > > +      high level (which configures DCLK as an output) or to set to a low level
> > > +      (configuring DCLK for input).
> > > +    enum: [ out, in ]
> > > +    default: in
> > > +
> > > +  adi,dclkmode:
> > > +    description:
> > > +      DCLK mode control for when the device operates in Pin Control Slave Mode
> > > +      or in SPI Control Mode. Describes whether the DEC1/DCLKMODE pin is set to
> > > +      a high level (configuring the DCLK to operate in free running mode) or
> > > +      to a low level (to configure DCLK to operate in gated mode).
> > > +    enum: [ free-running, gated ]
> > > +    default: gated  
> > In minimum I/O mode datasheet mentions this should always be gated.
> > Perhaps this and adi,dclkio can be left out and added when driver gains
> > other support than I/O mode?  
> 
> Yes, that's also correct. A few properties are actually not needed for minimum
> I/O mode (i.e. can have their values inferred from the minimum I/O mode requirements).
> Sure, from developer's perspective it's easier to document only what the driver
> uses. adi,dclkio, adi,dclkmode, adi,asrc-mode, powerdown-gpios, regulators, could
> all be left to a latter time. Fine by me if dt maintainers agree with that.

Add as much as you are sure about to the binding now.  Sometimes
we will add properties later (with defaults) to cover things that are
complex enough that they will delay the initial binding merge.

Jonathan

> 
> With best regards,
> Marcelo

Re: [PATCH v3 1/3] dt-bindings: iio: adc: Add AD4134

[Jonathan Cameron]

On Tue, 2 Dec 2025 17:55:03 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:

> Add device tree documentation for AD4134 24-Bit, 4-channel simultaneous
> sampling, precision ADC.
> 
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
A few comments inline. Not necessarily anything to change.

Thanks

Jonathan

> ---
> Change log v2 -> v3:
> - fixed typo in powerdown-gpios description.
> - picked up Conor's review tag. 
> 
>  .../bindings/iio/adc/adi,ad4134.yaml          | 198 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 205 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml
> new file mode 100644
> index 000000000000..69a6ddf6ca92
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml

>
> +allOf:
> +  - if:
> +      not:
> +        required:
> +          - ldoin-supply
> +    then:
> +      required:
> +        - avdd1v8-supply
> +        - dvdd1v8-supply
> +        - clkvdd-supply

Conor gave a tag, so I'm sure this is fine, but could we do this as
a oneOf given it's a pick between two options?

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +            compatible = "adi,ad4134";
> +            reg = <0>;
> +
> +            spi-max-frequency = <1000000>;
> +
> +            reset-gpios = <&gpio0 86 GPIO_ACTIVE_LOW>;
> +            odr-gpios = <&gpio0 87 GPIO_ACTIVE_HIGH>;
> +            powerdown-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> +
> +            clocks = <&sys_clk>;
> +            clock-names = "clkin";
> +
> +            avdd5-supply = <&avdd5>;
> +            dvdd5-supply = <&dvdd5>;
> +            iovdd-supply = <&iovdd>;
> +            refin-supply = <&refin>;
> +            avdd1v8-supply = <&avdd1v8>;
> +            dvdd1v8-supply = <&dvdd1v8>;
> +            clkvdd-supply = <&clkvdd>;
> +
> +            adi,asrc-mode = "low";
> +            adi,dclkio = "in";
> +            adi,dclkmode = "gated";

Interesting question on whether it is worth adding properties
to examples if they are set to the defaults. Generally we wouldn't
expect actual DT files to set them.  Meh, doesn't matter much to me
either way!

> +
> +            regulators {
> +                vcm_reg: vcm-output {
> +                    regulator-name = "ad4134-vcm";
> +                };
> +            };
> +
> +        };
> +    };
> +...

[PATCH v3 2/3] iio: adc: Initial support for AD4134

[Marcelo Schmitt]

AD4134 is a 24-bit, 4-channel, simultaneous sampling, precision
analog-to-digital converter (ADC). The device can be managed through SPI or
direct control of pin logical levels (pin control mode). The AD4134 design
also features a dedicated bus for ADC sample data output. Though, this
initial driver for AD4134 only supports usual SPI connections.

Add basic support for AD4134 that enables single-shot ADC sample read.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
Change log v2 -> v3:
- Dropped leftover dev field from struct ad4134_state.
- Inverted if condition and reduced indentation in ad4134_regulator_setup().
- Use (MICRO / MILLI) for microvolt/milivolt conversion.
- Handled clock deferred probe cases.

I tried using the reset-gpio driver to handle AD4134 reset GPIO. I had changed
the device tree to set a reset-controller node and had referenced that from the
ADC node. I also updated the ad4134 driver to use a reset controller to handle
its reset GPIO. Though, after those changes, the AD4134 driver would defer
device initialization forever because it missed a reset-controller. To make the
reset-gpio driver probe and instantiate a reset controller, it would take a
platform device to be set within a machine-specific, hardcoded platform data.
AD4134 is not bound to any specific platform, so it doesn't make much sense to
have a reset-gpio platform device for that. Thanks for mentioning reset-gpio. It
was interesting looking into the reset-gpio driver and the reset framework. It
looks cool. But I don't think the reset-gpio driver suits the AD4134 reset use
case.

The updated routine to get the external clock source now copes with deferred
device probing. I still think the v1 way of getting the clock looked better, but
if the way it is done in v3 looks better for maintainers and reviewers, I'll
acknowledge and keep that.

For reference, v1 clock initialization was
	ret = device_property_match_property_string(dev, "clock-names",
						    ad4134_clk_sel,
						    ARRAY_SIZE(ad4134_clk_sel));
	if (ret < 0)
		return dev_err_probe(dev, ret, "failed to find external clock\n");

	sys_clk = devm_clk_get_enabled(dev, ad4134_clk_sel[ret]);
	if (IS_ERR(sys_clk))
		return dev_err_probe(dev, PTR_ERR(sys_clk),
				     "failed to get %s external clock\n",
				     ad4134_clk_sel[ret]);

 MAINTAINERS              |   1 +
 drivers/iio/adc/Kconfig  |  11 +
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad4134.c | 503 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 516 insertions(+)
 create mode 100644 drivers/iio/adc/ad4134.c

diff --git a/MAINTAINERS b/MAINTAINERS
index b9029c4055e3..a1541cf3967b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1455,6 +1455,7 @@ L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml
+F:	drivers/iio/adc/ad4134.c
 
 ANALOG DEVICES INC AD4170-4 DRIVER
 M:	Marcelo Schmitt <marcelo.schmitt@analog.com>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 58da8255525e..e711a146bd23 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -99,6 +99,17 @@ config AD4130
 	  To compile this driver as a module, choose M here: the module will be
 	  called ad4130.
 
+config AD4134
+	tristate "Analog Device AD4134 ADC Driver"
+	depends on SPI
+	select REGMAP_SPI
+	select CRC8
+	help
+	  Say yes here to build support for Analog Devices AD4134 SPI analog to
+	  digital converters (ADC).
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called ad4134_spi.
 
 config AD4170_4
 	tristate "Analog Device AD4170-4 ADC Driver"
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 7cc8f9a12f76..4f834b56e0f0 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_AD4000) += ad4000.o
 obj-$(CONFIG_AD4030) += ad4030.o
 obj-$(CONFIG_AD4080) += ad4080.o
 obj-$(CONFIG_AD4130) += ad4130.o
+obj-$(CONFIG_AD4134) += ad4134.o
 obj-$(CONFIG_AD4170_4) += ad4170-4.o
 obj-$(CONFIG_AD4695) += ad4695.o
 obj-$(CONFIG_AD4851) += ad4851.o
diff --git a/drivers/iio/adc/ad4134.c b/drivers/iio/adc/ad4134.c
new file mode 100644
index 000000000000..7158eefcd877
--- /dev/null
+++ b/drivers/iio/adc/ad4134.c
@@ -0,0 +1,503 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2025 Analog Devices, Inc.
+ * Author: Marcelo Schmitt <marcelo.schmitt@analog.com>
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/crc8.h>
+#include <linux/delay.h>
+#include <linux/dev_printk.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/types.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/time.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+#include <linux/units.h>
+
+#define AD4134_RESET_TIME_US			(10 * USEC_PER_SEC)
+
+#define AD4134_REG_READ_MASK			BIT(7)
+#define AD4134_SPI_MAX_XFER_LEN			3
+
+#define AD4134_EXT_CLOCK_MHZ			(48 * HZ_PER_MHZ)
+
+#define AD4134_NUM_CHANNELS			4
+#define AD4134_CHAN_PRECISION_BITS		24
+
+#define AD4134_IFACE_CONFIG_A_REG		0x00
+#define AD4134_IFACE_CONFIG_B_REG		0x01
+#define AD4134_IFACE_CONFIG_B_SINGLE_INSTR	BIT(7)
+
+#define AD4134_DEVICE_CONFIG_REG		0x02
+#define AD4134_DEVICE_CONFIG_POWER_MODE_MASK	BIT(0)
+#define AD4134_POWER_MODE_HIGH_PERF		0x1
+
+#define AD4134_SILICON_REV_REG			0x07
+#define AD4134_SCRATCH_PAD_REG			0x0A
+#define AD4134_STREAM_MODE_REG			0x0E
+#define AD4134_SDO_PIN_SRC_SEL_REG		0x10
+#define AD4134_SDO_PIN_SRC_SEL_SDO_SEL_MASK	BIT(2)
+
+#define AD4134_DATA_PACKET_CONFIG_REG		0x11
+#define AD4134_DATA_PACKET_CONFIG_FRAME_MASK	GENMASK(5, 4)
+#define AD4134_DATA_PACKET_24BIT_FRAME		0x2
+
+#define AD4134_DIG_IF_CFG_REG			0x12
+#define AD4134_DIF_IF_CFG_FORMAT_MASK		GENMASK(1, 0)
+#define AD4134_DATA_FORMAT_SINGLE_CH_MODE	0x0
+
+#define AD4134_PW_DOWN_CTRL_REG			0x13
+#define AD4134_DEVICE_STATUS_REG		0x15
+#define AD4134_ODR_VAL_INT_LSB_REG		0x16
+#define AD4134_CH3_OFFSET_MSB_REG		0x3E
+#define AD4134_AIN_OR_ERROR_REG			0x48
+
+/*
+ * AD4134 register map ends at address 0x48 and there is no register for
+ * retrieving ADC sample data. Though, to make use of Linux regmap API both
+ * for register access and sample read, we define one virtual register for each
+ * ADC channel. AD4134_CH_VREG(x) maps a channel number to it's virtual register
+ * address while AD4134_VREG_CH(x) tells which channel given the address.
+ */
+#define AD4134_CH_VREG(x)			((x) + 0x50)
+#define AD4134_VREG_CH(x)			((x) - 0x50)
+
+#define AD4134_SPI_CRC_POLYNOM			0x07
+#define AD4134_SPI_CRC_INIT_VALUE		0xA5
+static unsigned char ad4134_spi_crc_table[CRC8_TABLE_SIZE];
+
+#define AD4134_CHANNEL(_index) {						\
+	.type = IIO_VOLTAGE,							\
+	.indexed = 1,								\
+	.channel = (_index),							\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),				\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),			\
+}
+
+static const struct iio_chan_spec ad4134_chan_set[] = {
+	AD4134_CHANNEL(0),
+	AD4134_CHANNEL(1),
+	AD4134_CHANNEL(2),
+	AD4134_CHANNEL(3),
+};
+
+struct ad4134_state {
+	struct spi_device *spi;
+	struct regmap *regmap;
+	unsigned long sys_clk_hz;
+	struct gpio_desc *odr_gpio;
+	int refin_mv;
+	/*
+	 * DMA (thus cache coherency maintenance) requires the transfer buffers
+	 * to live in their own cache lines.
+	 */
+	u8 rx_buf[AD4134_SPI_MAX_XFER_LEN] __aligned(IIO_DMA_MINALIGN);
+	u8 tx_buf[AD4134_SPI_MAX_XFER_LEN];
+};
+
+static const struct regmap_range ad4134_regmap_rd_range[] = {
+	regmap_reg_range(AD4134_IFACE_CONFIG_A_REG, AD4134_SILICON_REV_REG),
+	regmap_reg_range(AD4134_SCRATCH_PAD_REG, AD4134_PW_DOWN_CTRL_REG),
+	regmap_reg_range(AD4134_DEVICE_STATUS_REG, AD4134_AIN_OR_ERROR_REG),
+	regmap_reg_range(AD4134_CH_VREG(0), AD4134_CH_VREG(AD4134_NUM_CHANNELS)),
+};
+
+static const struct regmap_range ad4134_regmap_wr_range[] = {
+	regmap_reg_range(AD4134_IFACE_CONFIG_A_REG, AD4134_DEVICE_CONFIG_REG),
+	regmap_reg_range(AD4134_SCRATCH_PAD_REG, AD4134_SCRATCH_PAD_REG),
+	regmap_reg_range(AD4134_STREAM_MODE_REG, AD4134_PW_DOWN_CTRL_REG),
+	regmap_reg_range(AD4134_ODR_VAL_INT_LSB_REG, AD4134_CH3_OFFSET_MSB_REG),
+};
+
+static const struct regmap_access_table ad4134_regmap_rd_table = {
+	.yes_ranges = ad4134_regmap_rd_range,
+	.n_yes_ranges = ARRAY_SIZE(ad4134_regmap_rd_range),
+};
+
+static const struct regmap_access_table ad4134_regmap_wr_table = {
+	.yes_ranges = ad4134_regmap_wr_range,
+	.n_yes_ranges = ARRAY_SIZE(ad4134_regmap_wr_range),
+};
+
+static int ad4134_calc_spi_crc(u8 inst, u8 data)
+{
+	u8 buf[] = { inst, data };
+
+	return crc8(ad4134_spi_crc_table, buf, ARRAY_SIZE(buf),
+		    AD4134_SPI_CRC_INIT_VALUE);
+}
+
+static void ad4134_prepare_spi_tx_buf(u8 inst, u8 data, u8 *buf)
+{
+	buf[0] = inst;
+	buf[1] = data;
+	buf[2] = ad4134_calc_spi_crc(inst, data);
+}
+
+static int ad4134_reg_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct ad4134_state *st = context;
+	struct spi_transfer xfer = {
+		.tx_buf = st->tx_buf,
+		.rx_buf = st->rx_buf,
+		.len = AD4134_SPI_MAX_XFER_LEN,
+	};
+	int ret;
+
+	ad4134_prepare_spi_tx_buf(reg, val, st->tx_buf);
+
+	ret = spi_sync_transfer(st->spi, &xfer, 1);
+	if (ret)
+		return ret;
+
+	if (st->rx_buf[2] != st->tx_buf[2])
+		dev_dbg(&st->spi->dev, "reg write CRC check failed\n");
+
+	return 0;
+}
+
+static int ad4134_data_read(struct ad4134_state *st, unsigned int reg,
+			    unsigned int *val)
+{
+	unsigned int i;
+	int ret;
+
+	/*
+	 * To be able to read data from all 4 channels through a single line, we
+	 * set DOUTx output format to 0 in the digital interface config register
+	 * (0x12). With that, data from all four channels is serialized and
+	 * output on DOUT0. During probe, we also set SDO_PIN_SRC_SEL in
+	 * DEVICE_CONFIG_1 register to duplicate DOUT0 on the SDO pin. Combined,
+	 * those configurations enable ADC data read through a conventional SPI
+	 * interface. Now we read data from all channels but keep only the bits
+	 * from the requested one.
+	 */
+	for (i = 0; i < ARRAY_SIZE(ad4134_chan_set); i++) {
+		ret = spi_write_then_read(st->spi, NULL, 0, st->rx_buf,
+					  BITS_TO_BYTES(AD4134_CHAN_PRECISION_BITS));
+		if (ret)
+			return ret;
+
+		if (i != AD4134_VREG_CH(reg))
+			continue;
+
+		*val = get_unaligned_be24(st->rx_buf);
+	}
+
+	return 0;
+}
+
+static int ad4134_register_read(struct ad4134_state *st, unsigned int reg,
+				unsigned int *val)
+{
+	struct spi_transfer xfer = {
+		.tx_buf = st->tx_buf,
+		.rx_buf = st->rx_buf,
+		.len = AD4134_SPI_MAX_XFER_LEN,
+	};
+	unsigned int inst;
+	int ret;
+
+	inst = AD4134_REG_READ_MASK | reg;
+	ad4134_prepare_spi_tx_buf(inst, 0, st->tx_buf);
+
+	ret = spi_sync_transfer(st->spi, &xfer, 1);
+	if (ret)
+		return ret;
+
+	*val = st->rx_buf[1];
+
+	/* Check CRC */
+	if (st->rx_buf[2] != st->tx_buf[2])
+		dev_dbg(&st->spi->dev, "reg read CRC check failed\n");
+
+	return 0;
+}
+
+static int ad4134_reg_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct ad4134_state *st = context;
+
+	if (reg >= AD4134_CH_VREG(0))
+		return ad4134_data_read(st, reg, val);
+
+	return ad4134_register_read(st, reg, val);
+}
+
+static const struct regmap_config ad4134_regmap_config = {
+	.reg_read = ad4134_reg_read,
+	.reg_write = ad4134_reg_write,
+	.rd_table = &ad4134_regmap_rd_table,
+	.wr_table = &ad4134_regmap_wr_table,
+	.max_register = AD4134_CH_VREG(ARRAY_SIZE(ad4134_chan_set)),
+};
+
+static int ad4134_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long info)
+{
+	struct ad4134_state *st = iio_priv(indio_dev);
+	int ret;
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		gpiod_set_value_cansleep(st->odr_gpio, 1);
+		fsleep(1);
+		gpiod_set_value_cansleep(st->odr_gpio, 0);
+		ret = regmap_read(st->regmap, AD4134_CH_VREG(chan->channel), val);
+		if (ret)
+			return ret;
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = st->refin_mv;
+		*val2 = AD4134_CHAN_PRECISION_BITS - 1;
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ad4134_debugfs_reg_access(struct iio_dev *indio_dev,
+				     unsigned int reg, unsigned int writeval,
+				     unsigned int *readval)
+{
+	struct ad4134_state *st = iio_priv(indio_dev);
+
+	if (readval)
+		return regmap_read(st->regmap, reg, readval);
+
+	return regmap_write(st->regmap, reg, writeval);
+}
+
+static int ad4134_min_io_mode_setup(struct ad4134_state *st)
+{
+	struct device *dev = &st->spi->dev;
+	int ret;
+
+	st->odr_gpio = devm_gpiod_get(dev, "odr", GPIOD_OUT_LOW);
+	if (IS_ERR(st->odr_gpio))
+		return dev_err_probe(dev, PTR_ERR(st->odr_gpio),
+				     "failed to get ODR GPIO\n");
+
+	ret = regmap_update_bits(st->regmap, AD4134_DIG_IF_CFG_REG,
+				 AD4134_DIF_IF_CFG_FORMAT_MASK,
+				 FIELD_PREP(AD4134_DIF_IF_CFG_FORMAT_MASK,
+					    AD4134_DATA_FORMAT_SINGLE_CH_MODE));
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to set single channel mode\n");
+
+	ret = regmap_set_bits(st->regmap, AD4134_SDO_PIN_SRC_SEL_REG,
+			      AD4134_SDO_PIN_SRC_SEL_SDO_SEL_MASK);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to set SDO source selection\n");
+
+	return regmap_set_bits(st->regmap, AD4134_IFACE_CONFIG_B_REG,
+			       AD4134_IFACE_CONFIG_B_SINGLE_INSTR);
+}
+
+static const struct iio_info ad4134_info = {
+	.read_raw = ad4134_read_raw,
+	.debugfs_reg_access = ad4134_debugfs_reg_access,
+};
+
+static const char * const ad4143_regulator_names[] = {
+	"avdd5", "dvdd5", "iovdd", "refin",
+	"avdd1v8", "dvdd1v8", "clkvdd",	"ldoin",
+};
+
+static int ad4134_regulator_setup(struct ad4134_state *st)
+{
+	struct device *dev = &st->spi->dev;
+	bool use_internal_ldo_regulator;
+	int ret;
+
+	/* Required regulators */
+	ret = devm_regulator_bulk_get_enable(dev, 3, ad4143_regulator_names);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to enable power supplies\n");
+
+	/* Required regulator that we need to read the voltage */
+	ret = devm_regulator_get_enable_read_voltage(dev, "refin");
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "failed to get REFIN voltage.\n");
+
+	st->refin_mv = ret / (MICRO / MILLI);
+
+	/*
+	 * If ldoin is not provided, then avdd1v8, dvdd1v8, and clkvdd are
+	 * required.
+	 */
+	ret = devm_regulator_get_enable_optional(dev, "ldoin");
+	if (ret < 0 && ret != -ENODEV)
+		return dev_err_probe(dev, ret, "failed to enable ldoin supply\n");
+
+	use_internal_ldo_regulator = ret == 0;
+
+	if (use_internal_ldo_regulator)
+		return 0;
+
+	ret = devm_regulator_get_enable(dev, "avdd1v8");
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to enable avdd1v8 supply\n");
+
+	ret = devm_regulator_get_enable(dev, "dvdd1v8");
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to enable dvdd1v8 supply\n");
+
+	ret = devm_regulator_get_enable(dev, "clkvdd");
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to enable clkvdd supply\n");
+
+	return 0;
+}
+
+static int ad4134_clock_select(struct ad4134_state *st)
+{
+	struct device *dev = &st->spi->dev;
+	struct clk *sys_clk;
+
+	/*
+	 * AD4134 requires one external clock source and only one external clock
+	 * source can be provided at a time. Try get a crystal provided clock.
+	 * If that fails, try to get a CMOS clock.
+	 */
+	sys_clk = devm_clk_get_optional_enabled(dev, "xtal1-xtal2");
+	if (IS_ERR_OR_NULL(sys_clk)) {
+		if (PTR_ERR(sys_clk) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+
+		/* Try the CMOS clock */
+		sys_clk = devm_clk_get_enabled(dev, "clkin");
+		if (IS_ERR(sys_clk)) {
+			if (PTR_ERR(sys_clk) == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
+
+			return dev_err_probe(dev, PTR_ERR(sys_clk),
+					     "failed to get external clock\n");
+		}
+	}
+
+	st->sys_clk_hz = clk_get_rate(sys_clk);
+	if (st->sys_clk_hz != AD4134_EXT_CLOCK_MHZ)
+		dev_warn(dev, "invalid external clock frequency %lu\n",
+			 st->sys_clk_hz);
+
+	return 0;
+}
+
+static int ad4134_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct gpio_desc *reset_gpio;
+	struct iio_dev *indio_dev;
+	struct ad4134_state *st;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+
+	indio_dev->name = "ad4134";
+	indio_dev->channels = ad4134_chan_set;
+	indio_dev->num_channels = ARRAY_SIZE(ad4134_chan_set);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &ad4134_info;
+
+	ret = ad4134_regulator_setup(st);
+	if (ret)
+		return ret;
+
+	ret = ad4134_clock_select(st);
+	if (ret)
+		return ret;
+
+	reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(reset_gpio),
+				     "failed to find reset GPIO\n");
+
+	if (reset_gpio) {
+		fsleep(AD4134_RESET_TIME_US);
+		gpiod_set_value_cansleep(reset_gpio, 0);
+	}
+
+	crc8_populate_msb(ad4134_spi_crc_table, AD4134_SPI_CRC_POLYNOM);
+
+	st->regmap = devm_regmap_init(dev, NULL, st, &ad4134_regmap_config);
+	if (IS_ERR(st->regmap))
+		return dev_err_probe(dev, PTR_ERR(st->regmap),
+				     "failed to initialize regmap");
+
+	ret = ad4134_min_io_mode_setup(st);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to setup minimum I/O mode\n");
+
+	/* Bump precision to 24-bit */
+	ret = regmap_update_bits(st->regmap, AD4134_DATA_PACKET_CONFIG_REG,
+				 AD4134_DATA_PACKET_CONFIG_FRAME_MASK,
+				 FIELD_PREP(AD4134_DATA_PACKET_CONFIG_FRAME_MASK,
+					    AD4134_DATA_PACKET_24BIT_FRAME));
+	if (ret)
+		return ret;
+
+	/* Set high performance power mode */
+	ret = regmap_update_bits(st->regmap, AD4134_DEVICE_CONFIG_REG,
+				 AD4134_DEVICE_CONFIG_POWER_MODE_MASK,
+				 FIELD_PREP(AD4134_DEVICE_CONFIG_POWER_MODE_MASK,
+					    AD4134_POWER_MODE_HIGH_PERF));
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct spi_device_id ad4134_id[] = {
+	{ "ad4134" },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ad4134_id);
+
+static const struct of_device_id ad4134_of_match[] = {
+	{ .compatible = "adi,ad4134" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ad4134_of_match);
+
+static struct spi_driver ad4134_driver = {
+	.driver = {
+		.name = "ad4134",
+		.of_match_table = ad4134_of_match,
+	},
+	.probe = ad4134_probe,
+	.id_table = ad4134_id,
+};
+module_spi_driver(ad4134_driver);
+
+MODULE_AUTHOR("Marcelo Schmitt <marcelo.schmitt@analog.com>");
+MODULE_DESCRIPTION("Analog Devices AD4134 SPI driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("IIO_AD4134");
-- 
2.51.0

Re: [PATCH v3 2/3] iio: adc: Initial support for AD4134

[Andy Shevchenko]

On Tue, Dec 2, 2025 at 10:55 PM Marcelo Schmitt
<marcelo.schmitt@analog.com> wrote:
>
> AD4134 is a 24-bit, 4-channel, simultaneous sampling, precision
> analog-to-digital converter (ADC). The device can be managed through SPI or
> direct control of pin logical levels (pin control mode). The AD4134 design
> also features a dedicated bus for ADC sample data output. Though, this
> initial driver for AD4134 only supports usual SPI connections.
>
> Add basic support for AD4134 that enables single-shot ADC sample read.

...

> I tried using the reset-gpio driver to handle AD4134 reset GPIO. I had changed
> the device tree to set a reset-controller node and had referenced that from the
> ADC node. I also updated the ad4134 driver to use a reset controller to handle
> its reset GPIO. Though, after those changes, the AD4134 driver would defer
> device initialization forever because it missed a reset-controller. To make the
> reset-gpio driver probe and instantiate a reset controller, it would take a
> platform device to be set within a machine-specific, hardcoded platform data.
> AD4134 is not bound to any specific platform, so it doesn't make much sense to
> have a reset-gpio platform device for that. Thanks for mentioning reset-gpio. It
> was interesting looking into the reset-gpio driver and the reset framework. It
> looks cool. But I don't think the reset-gpio driver suits the AD4134 reset use
> case.

Bart converted it to be an aux driver and it should work. Please, give
a try after v6.19-rc1 is out.

...

> +       /*
> +        * To be able to read data from all 4 channels through a single line, we
> +        * set DOUTx output format to 0 in the digital interface config register
> +        * (0x12). With that, data from all four channels is serialized and
> +        * output on DOUT0. During probe, we also set SDO_PIN_SRC_SEL in

During the probe

> +        * DEVICE_CONFIG_1 register to duplicate DOUT0 on the SDO pin. Combined,
> +        * those configurations enable ADC data read through a conventional SPI
> +        * interface. Now we read data from all channels but keep only the bits
> +        * from the requested one.
> +        */
> +       for (i = 0; i < ARRAY_SIZE(ad4134_chan_set); i++) {
> +               ret = spi_write_then_read(st->spi, NULL, 0, st->rx_buf,
> +                                         BITS_TO_BYTES(AD4134_CHAN_PRECISION_BITS));
> +               if (ret)
> +                       return ret;
> +
> +               if (i != AD4134_VREG_CH(reg))
> +                       continue;
> +               *val = get_unaligned_be24(st->rx_buf);

Hmm...

In this case it might be better to  use

  if (i == ...)
    *val = ...

but it's still unclear on how many times the conditional can be true
in the loop.

> +       }

...

> +static int ad4134_clock_select(struct ad4134_state *st)
> +{
> +       struct device *dev = &st->spi->dev;
> +       struct clk *sys_clk;
> +
> +       /*
> +        * AD4134 requires one external clock source and only one external clock
> +        * source can be provided at a time. Try get a crystal provided clock.
> +        * If that fails, try to get a CMOS clock.
> +        */
> +       sys_clk = devm_clk_get_optional_enabled(dev, "xtal1-xtal2");
> +       if (IS_ERR_OR_NULL(sys_clk)) {
> +               if (PTR_ERR(sys_clk) == -EPROBE_DEFER)
> +                       return -EPROBE_DEFER;

But this will ignore other errors when clock _is_ available.

This should be as simple as

sys_clk = devm_clk_get_...(...);
if (!sys_clk)
  sys_clk = devm_clk_get_enabled(...);
if (IS_ERR(...))
  return dev_err_probe(..., PTR_ERR(...), ...);

See how other drivers do in the similar situation (IIRC 8250_dw does this).

> +               /* Try the CMOS clock */
> +               sys_clk = devm_clk_get_enabled(dev, "clkin");
> +               if (IS_ERR(sys_clk)) {
> +                       if (PTR_ERR(sys_clk) == -EPROBE_DEFER)
> +                               return -EPROBE_DEFER;
> +
> +                       return dev_err_probe(dev, PTR_ERR(sys_clk),
> +                                            "failed to get external clock\n");
> +               }
> +       }
> +
> +       st->sys_clk_hz = clk_get_rate(sys_clk);
> +       if (st->sys_clk_hz != AD4134_EXT_CLOCK_MHZ)
> +               dev_warn(dev, "invalid external clock frequency %lu\n",
> +                        st->sys_clk_hz);
> +
> +       return 0;
> +}

...

> +       reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +       if (IS_ERR(reset_gpio))
> +               return dev_err_probe(dev, PTR_ERR(reset_gpio),
> +                                    "failed to find reset GPIO\n");
> +
> +       if (reset_gpio) {
> +               fsleep(AD4134_RESET_TIME_US);
> +               gpiod_set_value_cansleep(reset_gpio, 0);
> +       }

I still think that reset-gpio driver is the right way to go (after
Bart's changes, which should be part of v6.19-rc1).

...

The rest LGTM.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/3] iio: adc: Initial support for AD4134

[Nuno Sá]

On Tue, 2025-12-02 at 23:26 +0200, Andy Shevchenko wrote:
> On Tue, Dec 2, 2025 at 10:55 PM Marcelo Schmitt
> <marcelo.schmitt@analog.com> wrote:
> > 
> > AD4134 is a 24-bit, 4-channel, simultaneous sampling, precision
> > analog-to-digital converter (ADC). The device can be managed through SPI or
> > direct control of pin logical levels (pin control mode). The AD4134 design
> > also features a dedicated bus for ADC sample data output. Though, this
> > initial driver for AD4134 only supports usual SPI connections.
> > 
> > Add basic support for AD4134 that enables single-shot ADC sample read.
> 
> ...
> 
> > I tried using the reset-gpio driver to handle AD4134 reset GPIO. I had changed
> > the device tree to set a reset-controller node and had referenced that from the
> > ADC node. I also updated the ad4134 driver to use a reset controller to handle
> > its reset GPIO. Though, after those changes, the AD4134 driver would defer
> > device initialization forever because it missed a reset-controller. To make the
> > reset-gpio driver probe and instantiate a reset controller, it would take a
> > platform device to be set within a machine-specific, hardcoded platform data.
> > AD4134 is not bound to any specific platform, so it doesn't make much sense to
> > have a reset-gpio platform device for that. Thanks for mentioning reset-gpio. It
> > was interesting looking into the reset-gpio driver and the reset framework. It
> > looks cool. But I don't think the reset-gpio driver suits the AD4134 reset use
> > case.
> 
> Bart converted it to be an aux driver and it should work. Please, give
> a try after v6.19-rc1 is out.
> 
> ...
> 
> > +       /*
> > +        * To be able to read data from all 4 channels through a single line, we
> > +        * set DOUTx output format to 0 in the digital interface config register
> > +        * (0x12). With that, data from all four channels is serialized and
> > +        * output on DOUT0. During probe, we also set SDO_PIN_SRC_SEL in
> 
> During the probe
> 
> > +        * DEVICE_CONFIG_1 register to duplicate DOUT0 on the SDO pin. Combined,
> > +        * those configurations enable ADC data read through a conventional SPI
> > +        * interface. Now we read data from all channels but keep only the bits
> > +        * from the requested one.
> > +        */
> > +       for (i = 0; i < ARRAY_SIZE(ad4134_chan_set); i++) {
> > +               ret = spi_write_then_read(st->spi, NULL, 0, st->rx_buf,
> > +                                         BITS_TO_BYTES(AD4134_CHAN_PRECISION_BITS));
> > +               if (ret)
> > +                       return ret;
> > +
> > +               if (i != AD4134_VREG_CH(reg))
> > +                       continue;
> > +               *val = get_unaligned_be24(st->rx_buf);
> 
> Hmm...
> 
> In this case it might be better to  use
> 
>   if (i == ...)
>     *val = ...
> 
> but it's still unclear on how many times the conditional can be true
> in the loop.
> 
> > +       }
> 
> ...
> 
> > +static int ad4134_clock_select(struct ad4134_state *st)
> > +{
> > +       struct device *dev = &st->spi->dev;
> > +       struct clk *sys_clk;
> > +
> > +       /*
> > +        * AD4134 requires one external clock source and only one external clock
> > +        * source can be provided at a time. Try get a crystal provided clock.
> > +        * If that fails, try to get a CMOS clock.
> > +        */
> > +       sys_clk = devm_clk_get_optional_enabled(dev, "xtal1-xtal2");
> > +       if (IS_ERR_OR_NULL(sys_clk)) {
> > +               if (PTR_ERR(sys_clk) == -EPROBE_DEFER)
> > +                       return -EPROBE_DEFER;
> 
> But this will ignore other errors when clock _is_ available.
> 
> This should be as simple as
> 
> sys_clk = devm_clk_get_...(...);
> if (!sys_clk)
>   sys_clk = devm_clk_get_enabled(...);
> if (IS_ERR(...))
>   return dev_err_probe(..., PTR_ERR(...), ...);
> 
> See how other drivers do in the similar situation (IIRC 8250_dw does this).
> 
> > +               /* Try the CMOS clock */
> > +               sys_clk = devm_clk_get_enabled(dev, "clkin");
> > +               if (IS_ERR(sys_clk)) {
> > +                       if (PTR_ERR(sys_clk) == -EPROBE_DEFER)
> > +                               return -EPROBE_DEFER;
> > +
> > +                       return dev_err_probe(dev, PTR_ERR(sys_clk),
> > +                                            "failed to get external clock\n");
> > +               }
> > +       }
> > +
> > +       st->sys_clk_hz = clk_get_rate(sys_clk);
> > +       if (st->sys_clk_hz != AD4134_EXT_CLOCK_MHZ)
> > +               dev_warn(dev, "invalid external clock frequency %lu\n",
> > +                        st->sys_clk_hz);
> > +
> > +       return 0;
> > +}
> 
> ...
> 
> > +       reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> > +       if (IS_ERR(reset_gpio))
> > +               return dev_err_probe(dev, PTR_ERR(reset_gpio),
> > +                                    "failed to find reset GPIO\n");
> > +
> > +       if (reset_gpio) {
> > +               fsleep(AD4134_RESET_TIME_US);
> > +               gpiod_set_value_cansleep(reset_gpio, 0);
> > +       }
> 
> I still think that reset-gpio driver is the right way to go (after
> Bart's changes, which should be part of v6.19-rc1).

Hmm, can you share why we should have a reset controller for the above? 

Unless I'm missing something, even with the aux device, you'll need the code to
optionally add it which (I think) will already force you to check the existence for
the pin (which would be a bit odd IMO).

But more importantly, for things like the above I'm failing to see the benefit in
registering a reset controller. In fact, I think it would be dangerous to "allow" other
potential consumers to randomly reset the device. 

If you look at Krzysztof's log when adding the driver, you see:

"Add a simple driver to control GPIO-based resets using the reset
controller API for the cases when the GPIOs are shared and reset should
be coordinated.  The driver is expected to be used by reset core
framework for ad-hoc reset controllers."

Key point is *GPIOs are shared and reset should be coordinated*. That is not the case here.

Or maybe I'm missing the point...

Having said the above, I would be up for some kind of helper in gpiolib. I still see way too
often people misinterpreting the meaning of GPIOD_OUT_HIGH and that the value in
gpiod_set_value_cansleep() means assert/deassert.

- Nuno Sá
 
> 
> ...
> 
> The rest LGTM.

Re: [PATCH v3 2/3] iio: adc: Initial support for AD4134

[Andy Shevchenko]

On Wed, Dec 03, 2025 at 11:02:45AM +0000, Nuno Sá wrote:
> On Tue, 2025-12-02 at 23:26 +0200, Andy Shevchenko wrote:
> > On Tue, Dec 2, 2025 at 10:55 PM Marcelo Schmitt
> > <marcelo.schmitt@analog.com> wrote:

Nuno, may you please remove unrelated context when replying?

...

> > > I tried using the reset-gpio driver to handle AD4134 reset GPIO. I had changed
> > > the device tree to set a reset-controller node and had referenced that from the
> > > ADC node. I also updated the ad4134 driver to use a reset controller to handle
> > > its reset GPIO. Though, after those changes, the AD4134 driver would defer
> > > device initialization forever because it missed a reset-controller. To make the
> > > reset-gpio driver probe and instantiate a reset controller, it would take a
> > > platform device to be set within a machine-specific, hardcoded platform data.
> > > AD4134 is not bound to any specific platform, so it doesn't make much sense to
> > > have a reset-gpio platform device for that. Thanks for mentioning reset-gpio. It
> > > was interesting looking into the reset-gpio driver and the reset framework. It
> > > looks cool. But I don't think the reset-gpio driver suits the AD4134 reset use
> > > case.
> > 
> > Bart converted it to be an aux driver and it should work. Please, give
> > a try after v6.19-rc1 is out.

...

> > > +       reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> > > +       if (IS_ERR(reset_gpio))
> > > +               return dev_err_probe(dev, PTR_ERR(reset_gpio),
> > > +                                    "failed to find reset GPIO\n");
> > > +
> > > +       if (reset_gpio) {
> > > +               fsleep(AD4134_RESET_TIME_US);
> > > +               gpiod_set_value_cansleep(reset_gpio, 0);
> > > +       }
> > 
> > I still think that reset-gpio driver is the right way to go (after
> > Bart's changes, which should be part of v6.19-rc1).
> 
> Hmm, can you share why we should have a reset controller for the above? 

My point here is to have a standard way of handling "reset" pin independently
of what's beneath in the HW — GPIO or other means to assert/deassert it.

> Unless I'm missing something, even with the aux device, you'll need the code to
> optionally add it which (I think) will already force you to check the existence for
> the pin (which would be a bit odd IMO).

If this is the case, it needs to be fixed, but reset framework provides
_optional() API, that's what should be used for the cases where reset is
optional. Let reset framework to handle that.

> But more importantly, for things like the above I'm failing to see the
> benefit in registering a reset controller. In fact, I think it would be
> dangerous to "allow" other potential consumers to randomly reset the device.

Again, same here, reset framework provides _shared() API which we do not want to
use here, we want to use _exclusive() one.

> If you look at Krzysztof's log when adding the driver, you see:

It's unrelated, because from the user's perspective it communicates via reset
framework.

> "Add a simple driver to control GPIO-based resets using the reset
> controller API for the cases when the GPIOs are shared and reset should
> be coordinated.  The driver is expected to be used by reset core
> framework for ad-hoc reset controllers."
> 
> Key point is *GPIOs are shared and reset should be coordinated*. That is not
> the case here.

And it has no direct relation to this driver in such a case. Of course the
generic reset controller driver should try to cover as many users as possible.
Then on the framework level they can decide how they want to use it.

It's more flexible that way, so I expect most of the reset controller drivers
to allow shared accesses. And it's orthogonal to the discussion here.

> Or maybe I'm missing the point...

I think so, see above.

> Having said the above, I would be up for some kind of helper in gpiolib.
> I still see way too often people misinterpreting the meaning of
> GPIOD_OUT_HIGH and that the value in gpiod_set_value_cansleep() means
> assert/deassert.

Consider this as a helper :-)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/3] iio: adc: Initial support for AD4134

[Nuno Sá]

On Wed, 2025-12-03 at 14:59 +0200, Andy Shevchenko wrote:
> On Wed, Dec 03, 2025 at 11:02:45AM +0000, Nuno Sá wrote:
> > On Tue, 2025-12-02 at 23:26 +0200, Andy Shevchenko wrote:
> > > On Tue, Dec 2, 2025 at 10:55 PM Marcelo Schmitt
> > > <marcelo.schmitt@analog.com> wrote:
> 
> Nuno, may you please remove unrelated context when replying?

It was not that much. That is why I did not bothered :)

...

> 
> > 
> > Hmm, can you share why we should have a reset controller for the above? 
> 
> My point here is to have a standard way of handling "reset" pin independently
> of what's beneath in the HW — GPIO or other means to assert/deassert it.

That makes sense.

> 
> > Unless I'm missing something, even with the aux device, you'll need the code to
> > optionally add it which (I think) will already force you to check the existence for
> > the pin (which would be a bit odd IMO).
> 
> If this is the case, it needs to be fixed, but reset framework provides
> _optional() API, that's what should be used for the cases where reset is
> optional. Let reset framework to handle that.

Ok, I think I was also misunderstanding you. So you mean that instead of doing 
devm_gpiod_get_optional() we should use one of the devm_reset_control_get_*() calls? 

Ok, I went to check the reset core implementation and with [1] I take back my comment. I can see now
that the framework will automatically handle creating the auxdevice. So while I still think most of
the times we'll still see reset-gpios in bindings, it makes sense to have this HW abstraction in the
code.

One thing to note is that the reset framework always enforces reset-gpios and we do have places
where reset pins have different ids (just because that's how the datasheet defines them).

...

> 
> > Having said the above, I would be up for some kind of helper in gpiolib.
> > I still see way too often people misinterpreting the meaning of
> > GPIOD_OUT_HIGH and that the value in gpiod_set_value_cansleep() means
> > assert/deassert.
> 
> Consider this as a helper :-)

Indeed!

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/reset/core.c#n1038

- Nuno Sá

Re: [PATCH v3 2/3] iio: adc: Initial support for AD4134

[Andy Shevchenko]

On Wed, Dec 03, 2025 at 02:48:44PM +0000, Nuno Sá wrote:
> On Wed, 2025-12-03 at 14:59 +0200, Andy Shevchenko wrote:
> > On Wed, Dec 03, 2025 at 11:02:45AM +0000, Nuno Sá wrote:
> > > On Tue, 2025-12-02 at 23:26 +0200, Andy Shevchenko wrote:
> > > > On Tue, Dec 2, 2025 at 10:55 PM Marcelo Schmitt
> > > > <marcelo.schmitt@analog.com> wrote:

...

> > Nuno, may you please remove unrelated context when replying?
> 
> It was not that much. That is why I did not bothered :)

2 pages of text on my screen... We definitely have different metrics
for "too much" :-)

...

> > > Hmm, can you share why we should have a reset controller for the above? 
> > 
> > My point here is to have a standard way of handling "reset" pin independently
> > of what's beneath in the HW — GPIO or other means to assert/deassert it.
> 
> That makes sense.
> 
> > > Unless I'm missing something, even with the aux device, you'll need the code to
> > > optionally add it which (I think) will already force you to check the existence for
> > > the pin (which would be a bit odd IMO).
> > 
> > If this is the case, it needs to be fixed, but reset framework provides
> > _optional() API, that's what should be used for the cases where reset is
> > optional. Let reset framework to handle that.
> 
> Ok, I think I was also misunderstanding you. So you mean that instead of doing 
> devm_gpiod_get_optional() we should use one of the devm_reset_control_get_*() calls? 

Yep!

> Ok, I went to check the reset core implementation and with [1] I take back my comment. I can see now
> that the framework will automatically handle creating the auxdevice. So while I still think most of
> the times we'll still see reset-gpios in bindings, it makes sense to have this HW abstraction in the
> code.

...and standardisation.

> One thing to note is that the reset framework always enforces reset-gpios and we do have places
> where reset pins have different ids (just because that's how the datasheet defines them).

This might affect the decision. In any case, please consider using it and we
can see if it makes sense over open-coded approach.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/3] iio: adc: Initial support for AD4134

[Marcelo Schmitt]

On 12/02, Andy Shevchenko wrote:
> On Tue, Dec 2, 2025 at 10:55 PM Marcelo Schmitt
> <marcelo.schmitt@analog.com> wrote:
> >
> > AD4134 is a 24-bit, 4-channel, simultaneous sampling, precision
> > analog-to-digital converter (ADC). The device can be managed through SPI or
> > direct control of pin logical levels (pin control mode). The AD4134 design
> > also features a dedicated bus for ADC sample data output. Though, this
> > initial driver for AD4134 only supports usual SPI connections.
> >
> > Add basic support for AD4134 that enables single-shot ADC sample read.
> 
...
> 
> Bart converted it to be an aux driver and it should work. Please, give
> a try after v6.19-rc1 is out.
> 
Ok, from yesterday's disscussion I see I should give rest framework another try.
For now, I'll rebase on top of linux-next to experiment with the new reset gpio aux device.

...
> 
> > +        * interface. Now we read data from all channels but keep only the bits
> > +        * from the requested one.
> > +        */
> > +       for (i = 0; i < ARRAY_SIZE(ad4134_chan_set); i++) {
> > +               ret = spi_write_then_read(st->spi, NULL, 0, st->rx_buf,
> > +                                         BITS_TO_BYTES(AD4134_CHAN_PRECISION_BITS));
> > +               if (ret)
> > +                       return ret;
> > +
> > +               if (i != AD4134_VREG_CH(reg))
> > +                       continue;
> > +               *val = get_unaligned_be24(st->rx_buf);
> 
> Hmm...
> 
> In this case it might be better to  use
> 
>   if (i == ...)
>     *val = ...
> 
> but it's still unclear on how many times the conditional can be true
> in the loop.

The if != ... condition was true on three iterations of the loop and false
in only one iteration. Updated to if (i == ...), the conditional is now true
only on one iteration. 
The AD4134 has a built-in feature that flags when data trasnfers don't run
enough clock cycles to read the entire data frame (i.e. the data from all 4
channels in this case).
Since this is not expected to be a time critical method for acquiring data,
I coded it to avoid that error flag. I added a comment about that for v4.

Thanks,
Marcelo

Re: [PATCH v3 2/3] iio: adc: Initial support for AD4134

[Jonathan Cameron]

On Tue, 2 Dec 2025 17:55:21 -0300
Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:

> AD4134 is a 24-bit, 4-channel, simultaneous sampling, precision
> analog-to-digital converter (ADC). The device can be managed through SPI or
> direct control of pin logical levels (pin control mode). The AD4134 design
> also features a dedicated bus for ADC sample data output. Though, this
> initial driver for AD4134 only supports usual SPI connections.
> 
> Add basic support for AD4134 that enables single-shot ADC sample read.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
Hi Marcelo

Nice and clean which makes for pleasant reviewing :)
A few minor comments inline.

Jonathan

> diff --git a/drivers/iio/adc/ad4134.c b/drivers/iio/adc/ad4134.c
> new file mode 100644
> index 000000000000..7158eefcd877
> --- /dev/null
> +++ b/drivers/iio/adc/ad4134.c


> +static int ad4134_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long info)
> +{
> +	struct ad4134_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (info) {
> +	case IIO_CHAN_INFO_RAW:
> +		gpiod_set_value_cansleep(st->odr_gpio, 1);
> +		fsleep(1);
Any section reference for required pulse width that you can add as a comment here?

It's useful if people end up wondering if the pulse is long enough if they have
problems with a board.

> +		gpiod_set_value_cansleep(st->odr_gpio, 0);
> +		ret = regmap_read(st->regmap, AD4134_CH_VREG(chan->channel), val);
> +		if (ret)
> +			return ret;
> +
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = st->refin_mv;
> +		*val2 = AD4134_CHAN_PRECISION_BITS - 1;
> +
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +	default:
> +		return -EINVAL;
> +	}
> +}

> +
> +static const char * const ad4143_regulator_names[] = {
> +	"avdd5", "dvdd5", "iovdd", "refin",
> +	"avdd1v8", "dvdd1v8", "clkvdd",	"ldoin",
> +};
> +
> +static int ad4134_regulator_setup(struct ad4134_state *st)
> +{
> +	struct device *dev = &st->spi->dev;
> +	bool use_internal_ldo_regulator;
> +	int ret;
> +
> +	/* Required regulators */
> +	ret = devm_regulator_bulk_get_enable(dev, 3, ad4143_regulator_names);

Why list names of regulators in that array that you don't use? I'd call it
ad4143_required_regulator_names and then you can use ARRAY_SIZE() to replace
that 3.

> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to enable power supplies\n");
> +
> +	/* Required regulator that we need to read the voltage */
> +	ret = devm_regulator_get_enable_read_voltage(dev, "refin");
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to get REFIN voltage.\n");
> +
> +	st->refin_mv = ret / (MICRO / MILLI);
> +
> +	/*
> +	 * If ldoin is not provided, then avdd1v8, dvdd1v8, and clkvdd are
> +	 * required.
> +	 */
> +	ret = devm_regulator_get_enable_optional(dev, "ldoin");
> +	if (ret < 0 && ret != -ENODEV)
> +		return dev_err_probe(dev, ret, "failed to enable ldoin supply\n");
> +
> +	use_internal_ldo_regulator = ret == 0;
> +
> +	if (use_internal_ldo_regulator)
> +		return 0;
> +

For these 3 you can use a second array of names and devm_regulator_bulk_get_enable()
Finding a short descriptive name for that array might be tricky however.

> +	ret = devm_regulator_get_enable(dev, "avdd1v8");
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to enable avdd1v8 supply\n");
> +
> +	ret = devm_regulator_get_enable(dev, "dvdd1v8");
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to enable dvdd1v8 supply\n");
> +
> +	ret = devm_regulator_get_enable(dev, "clkvdd");
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to enable clkvdd supply\n");
> +
> +	return 0;
> +}
> +
> +static int ad4134_clock_select(struct ad4134_state *st)
> +{
> +	struct device *dev = &st->spi->dev;
> +	struct clk *sys_clk;
> +
> +	/*
> +	 * AD4134 requires one external clock source and only one external clock
> +	 * source can be provided at a time. Try get a crystal provided clock.
> +	 * If that fails, try to get a CMOS clock.
> +	 */
> +	sys_clk = devm_clk_get_optional_enabled(dev, "xtal1-xtal2");

I should have noticed this in the binding review.  Why do we need to call out which
xtal pins?  Previous cases of this have just used the name "xtal" for the clock
type selection.  Maybe there was some discussion I missed.

> +	if (IS_ERR_OR_NULL(sys_clk)) {
> +		if (PTR_ERR(sys_clk) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +
> +		/* Try the CMOS clock */
> +		sys_clk = devm_clk_get_enabled(dev, "clkin");
> +		if (IS_ERR(sys_clk)) {
> +			if (PTR_ERR(sys_clk) == -EPROBE_DEFER)
> +				return -EPROBE_DEFER;
> +
> +			return dev_err_probe(dev, PTR_ERR(sys_clk),
> +					     "failed to get external clock\n");
> +		}
> +	}
> +
> +	st->sys_clk_hz = clk_get_rate(sys_clk);
> +	if (st->sys_clk_hz != AD4134_EXT_CLOCK_MHZ)
> +		dev_warn(dev, "invalid external clock frequency %lu\n",
> +			 st->sys_clk_hz);
> +
> +	return 0;
> +}

[PATCH v3 3/3] Docs: iio: Add AD4134

[Marcelo Schmitt]

Add initial documentation for the ad4134 IIO driver.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
---
No changes from v2 to v3.

 Documentation/iio/ad4134.rst | 58 ++++++++++++++++++++++++++++++++++++
 Documentation/iio/index.rst  |  1 +
 MAINTAINERS                  |  1 +
 3 files changed, 60 insertions(+)
 create mode 100644 Documentation/iio/ad4134.rst

diff --git a/Documentation/iio/ad4134.rst b/Documentation/iio/ad4134.rst
new file mode 100644
index 000000000000..fa44a05e6793
--- /dev/null
+++ b/Documentation/iio/ad4134.rst
@@ -0,0 +1,58 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+=============
+AD4134 driver
+=============
+
+Device driver for Analog Devices Inc. AD4134 and similar ADCs.
+
+Supported devices
+=================
+
+* `AD4134 <https://www.analog.com/AD4134>`_
+* `AD7134 <https://www.analog.com/AD7134>`_
+
+Wiring connections
+------------------
+
+AD4134 and similar ADCs can operate in a few different wiring configurations.
+
+Minimum I/O mode (SPI control mode)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The minimum I/O mode wiring allows AD4134 register and data access with the
+conventional set of SPI bus lines. The hardware configuration settings for using
+AD4134 in minimum I/O mode are:
+
++----------------------------+----------------------+--------------------+
+| Pin Function               |         Level        | Description        |
++============================+======================+====================+
+| PIN/SPI                    |         High         | SPI control mode   |
++----------------------------+----------------------+--------------------+
+| MODE                       |         Low          | ASRC slave mode    |
++----------------------------+----------------------+--------------------+
+| DCLKIO                     |         Low          | DCLK input         |
++----------------------------+----------------------+--------------------+
+| DCLKMODE                   |         Low          | Gated DCLK         |
++----------------------------+----------------------+--------------------+
+
+A possible connection schema that sets AD4134 digital interface for minimum I/O
+mode is:
+
+::
+
+                                 IOVDD
+      +------------------------+   |
+      |                PIN/SPI |<--+     +-------------+
+      |                        |         |     HOST    |
+      |                   DCLK |<--+     |             |
+      |           FORMAT1/SCLK |<--+---- | SCLK        |
+      |   AD4134      DEC2/SDI |<--------| SDO         |
+      |               DEC3/SDO |-------->| SDI         |
+      |                    ODR |<--------| GPIO        |
+      |             FORMAT0/CS |<--+     |             |
+      |                   MODE |<--+     +-------------+
+      |            DEC0/DCLKIO |<--+
+      |          DEC1/DCLKMODE |<--+
+      +------------------------+   |
+                                  GND
diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
index 315ae37d6fd4..d4ed782c93a6 100644
--- a/Documentation/iio/index.rst
+++ b/Documentation/iio/index.rst
@@ -22,6 +22,7 @@ Industrial I/O Kernel Drivers
    ad3552r
    ad4000
    ad4030
+   ad4134
    ad4695
    ad7191
    ad7380
diff --git a/MAINTAINERS b/MAINTAINERS
index a1541cf3967b..b2c101598636 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1455,6 +1455,7 @@ L:	linux-iio@vger.kernel.org
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml
+F:	Documentation/iio/ad4134.rst
 F:	drivers/iio/adc/ad4134.c
 
 ANALOG DEVICES INC AD4170-4 DRIVER
-- 
2.51.0

Re: [PATCH v3 3/3] Docs: iio: Add AD4134

[Tomas Melin]

Hi,

On 02/12/2025 22:55, Marcelo Schmitt wrote:
> Add initial documentation for the ad4134 IIO driver.

I wonder is there some information in here that is not readily available
in the device datasheet? After all, isn't idea with this file to
document peculiarities that are not easily found elsewhere?

Thanks,
Tomas


> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> ---
> No changes from v2 to v3.
> 
>  Documentation/iio/ad4134.rst | 58 ++++++++++++++++++++++++++++++++++++
>  Documentation/iio/index.rst  |  1 +
>  MAINTAINERS                  |  1 +
>  3 files changed, 60 insertions(+)
>  create mode 100644 Documentation/iio/ad4134.rst
> 
> diff --git a/Documentation/iio/ad4134.rst b/Documentation/iio/ad4134.rst
> new file mode 100644
> index 000000000000..fa44a05e6793
> --- /dev/null
> +++ b/Documentation/iio/ad4134.rst
> @@ -0,0 +1,58 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +=============
> +AD4134 driver
> +=============
> +
> +Device driver for Analog Devices Inc. AD4134 and similar ADCs.
> +
> +Supported devices
> +=================
> +
> +* `AD4134 <https://www.analog.com/AD4134>`_
> +* `AD7134 <https://www.analog.com/AD7134>`_
> +
> +Wiring connections
> +------------------
> +
> +AD4134 and similar ADCs can operate in a few different wiring configurations.
> +
> +Minimum I/O mode (SPI control mode)
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +The minimum I/O mode wiring allows AD4134 register and data access with the
> +conventional set of SPI bus lines. The hardware configuration settings for using
> +AD4134 in minimum I/O mode are:
> +
> ++----------------------------+----------------------+--------------------+
> +| Pin Function               |         Level        | Description        |
> ++============================+======================+====================+
> +| PIN/SPI                    |         High         | SPI control mode   |
> ++----------------------------+----------------------+--------------------+
> +| MODE                       |         Low          | ASRC slave mode    |
> ++----------------------------+----------------------+--------------------+
> +| DCLKIO                     |         Low          | DCLK input         |
> ++----------------------------+----------------------+--------------------+
> +| DCLKMODE                   |         Low          | Gated DCLK         |
> ++----------------------------+----------------------+--------------------+
> +
> +A possible connection schema that sets AD4134 digital interface for minimum I/O
> +mode is:
> +
> +::
> +
> +                                 IOVDD
> +      +------------------------+   |
> +      |                PIN/SPI |<--+     +-------------+
> +      |                        |         |     HOST    |
> +      |                   DCLK |<--+     |             |
> +      |           FORMAT1/SCLK |<--+---- | SCLK        |
> +      |   AD4134      DEC2/SDI |<--------| SDO         |
> +      |               DEC3/SDO |-------->| SDI         |
> +      |                    ODR |<--------| GPIO        |
> +      |             FORMAT0/CS |<--+     |             |
> +      |                   MODE |<--+     +-------------+
> +      |            DEC0/DCLKIO |<--+
> +      |          DEC1/DCLKMODE |<--+
> +      +------------------------+   |
> +                                  GND
> diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
> index 315ae37d6fd4..d4ed782c93a6 100644
> --- a/Documentation/iio/index.rst
> +++ b/Documentation/iio/index.rst
> @@ -22,6 +22,7 @@ Industrial I/O Kernel Drivers
>     ad3552r
>     ad4000
>     ad4030
> +   ad4134
>     ad4695
>     ad7191
>     ad7380
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a1541cf3967b..b2c101598636 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1455,6 +1455,7 @@ L:	linux-iio@vger.kernel.org
>  S:	Supported
>  W:	https://ez.analog.com/linux-software-drivers
>  F:	Documentation/devicetree/bindings/iio/adc/adi,ad4134.yaml
> +F:	Documentation/iio/ad4134.rst
>  F:	drivers/iio/adc/ad4134.c
>  
>  ANALOG DEVICES INC AD4170-4 DRIVER

Re: [PATCH v3 3/3] Docs: iio: Add AD4134

[Marcelo Schmitt]

On 12/03, Tomas Melin wrote:
> Hi,
> 
> On 02/12/2025 22:55, Marcelo Schmitt wrote:
> > Add initial documentation for the ad4134 IIO driver.
> 
> I wonder is there some information in here that is not readily available
> in the device datasheet? After all, isn't idea with this file to
> document peculiarities that are not easily found elsewhere?

You are correct, these docs are mostly from data sheet info.
The main idea of having the doc is to make clear what peripheral connection
schema is currently supported.
Because AD4134 is both flexible and somewhat extensible in the way it can be
connected to the host, we could have different wiring configurations, for
example

                                                              +-------------+
  +----------------------+                                    |  DATA HOST  |
  |       AD4134         |                                    |             |
  |                      |                                    |             |
  |Data interface  DOUT0 |----------------------------------->| GPI0        |
  |for ADC data    DOUT1 |----------------------------------->| GPI1        |
  |read back       DOUT2 |----------------------------------->| GPI2        |
  |                DOUT3 |----------------------------------->| GPI3        |
  |                DCLK  |<--------------+        +---------->| GPI4        |
  |                ODR   |<------------+ |        | +-------->| GPI5        |
  |                      |             | |        | | +------>| GPI6        |
  |                      |             | |        | | | +---->| GPI7        |
  | SPI interface   CS   |<-------+    | +--------|-|-|-|-----| DCLK        |
  | for register    SCLK |<-----+ |    |          | | | |     |             |
  | access          SDI  |<---+ | |    |          | | | |     | TRIGGER     |
  |                 SDO  |--+ | | |    |          | | | |     +-------------+
  +----------------------+  | | | |    +----------|-|-|-|---------+
                            | | | +---------------+ | | |
                            | | +-------------------+ | |
                            | +-----------------------+ |
                            +---------------------------+

or even with two devices [1].
[1]: https://lore.kernel.org/linux-iio/aRIIDTUR5Pyz1Rxi@debian-BULLSEYE-live-builder-AMD64/

That is not a current use case but it could be possible. I think it's likely we
will need extra software to support those cases and, when we get to that, we
would be adding more diagrams to this doc. Still, no strong opinion, we can
alternatively add the doc only when we get to those more complex cases.
I'll follow reviewers' preferences if you or anybody else express any.

Best regards,
Marcelo

Re: [PATCH v3 3/3] Docs: iio: Add AD4134

[Tomas Melin]

Hi,

On 04/12/2025 17:32, Marcelo Schmitt wrote:
> On 12/03, Tomas Melin wrote:
>> Hi,
>>
>> On 02/12/2025 22:55, Marcelo Schmitt wrote:
>>> Add initial documentation for the ad4134 IIO driver.
>>
>> I wonder is there some information in here that is not readily available
>> in the device datasheet? After all, isn't idea with this file to
>> document peculiarities that are not easily found elsewhere?
> 
> You are correct, these docs are mostly from data sheet info.
> The main idea of having the doc is to make clear what peripheral connection
> schema is currently supported.
> Because AD4134 is both flexible and somewhat extensible in the way it can be
> connected to the host, we could have different wiring configurations, for
> example

Thanks for your explanation. My humble opinion is that it would be
enough to mention in the commit message for the driver being added, or
in the device-tree bindings that basic I/O mode is only configuration
that is currently supported.

Thanks,
Tomas


> 
>                                                               +-------------+
>   +----------------------+                                    |  DATA HOST  |
>   |       AD4134         |                                    |             |
>   |                      |                                    |             |
>   |Data interface  DOUT0 |----------------------------------->| GPI0        |
>   |for ADC data    DOUT1 |----------------------------------->| GPI1        |
>   |read back       DOUT2 |----------------------------------->| GPI2        |
>   |                DOUT3 |----------------------------------->| GPI3        |
>   |                DCLK  |<--------------+        +---------->| GPI4        |
>   |                ODR   |<------------+ |        | +-------->| GPI5        |
>   |                      |             | |        | | +------>| GPI6        |
>   |                      |             | |        | | | +---->| GPI7        |
>   | SPI interface   CS   |<-------+    | +--------|-|-|-|-----| DCLK        |
>   | for register    SCLK |<-----+ |    |          | | | |     |             |
>   | access          SDI  |<---+ | |    |          | | | |     | TRIGGER     |
>   |                 SDO  |--+ | | |    |          | | | |     +-------------+
>   +----------------------+  | | | |    +----------|-|-|-|---------+
>                             | | | +---------------+ | | |
>                             | | +-------------------+ | |
>                             | +-----------------------+ |
>                             +---------------------------+
> 
> or even with two devices [1].
> [1]: https://lore.kernel.org/linux-iio/aRIIDTUR5Pyz1Rxi@debian-BULLSEYE-live-builder-AMD64/
> 
> That is not a current use case but it could be possible. I think it's likely we
> will need extra software to support those cases and, when we get to that, we
> would be adding more diagrams to this doc. Still, no strong opinion, we can
> alternatively add the doc only when we get to those more complex cases.
> I'll follow reviewers' preferences if you or anybody else express any.
> 
> Best regards,
> Marcelo

Re: [PATCH v3 3/3] Docs: iio: Add AD4134

[Marcelo Schmitt]

On 12/05, Tomas Melin wrote:
> Hi,
> 
> On 04/12/2025 17:32, Marcelo Schmitt wrote:
> > On 12/03, Tomas Melin wrote:
> >> Hi,
> >>
> >> On 02/12/2025 22:55, Marcelo Schmitt wrote:
> >>> Add initial documentation for the ad4134 IIO driver.
> >>
> >> I wonder is there some information in here that is not readily available
> >> in the device datasheet? After all, isn't idea with this file to
> >> document peculiarities that are not easily found elsewhere?
> > 
> > You are correct, these docs are mostly from data sheet info.
> > The main idea of having the doc is to make clear what peripheral connection
> > schema is currently supported.
> > Because AD4134 is both flexible and somewhat extensible in the way it can be
> > connected to the host, we could have different wiring configurations, for
> > example
> 
> Thanks for your explanation. My humble opinion is that it would be
> enough to mention in the commit message for the driver being added, or
> in the device-tree bindings that basic I/O mode is only configuration
> that is currently supported.

Okay, I'll drop the docs from v4.

Thanks,
Marcelo

Re: [PATCH v3 3/3] Docs: iio: Add AD4134

[Jonathan Cameron]

On Fri, 5 Dec 2025 09:32:04 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 12/05, Tomas Melin wrote:
> > Hi,
> > 
> > On 04/12/2025 17:32, Marcelo Schmitt wrote:  
> > > On 12/03, Tomas Melin wrote:  
> > >> Hi,
> > >>
> > >> On 02/12/2025 22:55, Marcelo Schmitt wrote:  
> > >>> Add initial documentation for the ad4134 IIO driver.  
> > >>
> > >> I wonder is there some information in here that is not readily available
> > >> in the device datasheet? After all, isn't idea with this file to
> > >> document peculiarities that are not easily found elsewhere?  
> > > 
> > > You are correct, these docs are mostly from data sheet info.
> > > The main idea of having the doc is to make clear what peripheral connection
> > > schema is currently supported.
> > > Because AD4134 is both flexible and somewhat extensible in the way it can be
> > > connected to the host, we could have different wiring configurations, for
> > > example  
> > 
> > Thanks for your explanation. My humble opinion is that it would be
> > enough to mention in the commit message for the driver being added, or
> > in the device-tree bindings that basic I/O mode is only configuration
> > that is currently supported.  
> 
> Okay, I'll drop the docs from v4.
> 
On this: I only rarely request docs for a particular driver when
it gets really complex - particularly when the ABI might not obviously
align with the terminology in a datasheet.

For other drivers I've always taken the view that if the documentation
is clean and potentially useful I don't mind adding it.

I get Tomas' point here though as this is very light so maybe a 'not yet'.

Jonathan


> Thanks,
> Marcelo