Add support for AD4000 series

Add device tree documentation for AD4000 family of ADC devices.

Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf

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

---
 .../bindings/iio/adc/adi,ad4000.yaml          | 201 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 208 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml new file +45 -0

new file mode 100644
index 000000000000..ca06afb5149e
@@ -0,0 +1,201 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD4000 and similar Analog to Digital Converters
+
+maintainers:
+  - Marcelo Schmitt <[email protected]>
+
+description: |
+  Analog Devices AD4000 family of Analog to Digital Converters with SPI support.
+  Specifications can be found at:
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
+
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - adi,ad4000
+      - adi,ad4001
+      - adi,ad4002
+      - adi,ad4003
+      - adi,ad4004
+      - adi,ad4005
+      - adi,ad4006
+      - adi,ad4007
+      - adi,ad4008
+      - adi,ad4010
+      - adi,ad4011
+      - adi,ad4020
+      - adi,ad4021
+      - adi,ad4022
+      - adi,adaq4001
+      - adi,adaq4003
+
+  reg:

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml new file +36 -0

+    description: |
+      This property indicates the SPI wiring configuration.
+
+      When this property is omitted, it is assumed that the device is using what
+      the datasheet calls "4-wire mode". This is the conventional SPI mode used
+      when there are multiple devices on the same bus. In this mode, the CNV
+      line is used to initiate the conversion and the SDI line is connected to
+      CS on the SPI controller.
+
+      When this property is present, it indicates that the device is using one
+      of the following alternative wiring configurations:
+
+      * single: The datasheet calls this "3-wire mode". (NOTE: The datasheet's
+        definition of 3-wire mode is NOT at all related to the standard
+        spi-3wire property!) This mode is often used when the ADC is the only
+        device on the bus. In this mode, SDI is tied to VIO, and the CNV line
+        can be connected to the CS line of the SPI controller or to a GPIO, in
+        which case the CS line of the controller is unused.
+      * chain: The datasheet calls this "chain mode". This mode is used to save
+        on wiring when multiple ADCs are used. In this mode, the SDI line of
+        one chip is tied to the SDO of the next chip in the chain and the SDI of
+        the last chip in the chain is tied to GND. Only the first chip in the
+        chain is connected to the SPI bus. The CNV line of all chips are tied
+        together. The CS line of the SPI controller can be used as the CNV line
+        only if it is active high.
+
+  '#daisy-chained-devices': true
+
+  vdd-supply:
+    description: A 1.8V supply that powers the chip (VDD).
+
+  vio-supply:
+    description:
+      A 1.8V to 5.5V supply for the digital inputs and outputs (VIO).
+
+  ref-supply:

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml new file +8 -0

+    maxItems: 1
+
+  adi,high-z-input:
+    type: boolean
+    description:
+      High-Z mode allows the amplifier and RC filter in front of the ADC to be
+      chosen based on the signal bandwidth of interest, rather than the settling
+      requirements of the switched capacitor SAR ADC inputs.

Quoted 11 lines · Marcelo Schmitt

Add device tree documentation for AD4000 family of ADC devices.

Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf

Suggested-by David Lechner <[email protected]>

(if you still use mostly my suggestions in the end)

Quoted 59 lines

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

---
 .../bindings/iio/adc/adi,ad4000.yaml          | 201 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 208 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml new file +45 -0

new file mode 100644
index 000000000000..ca06afb5149e
@@ -0,0 +1,201 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD4000 and similar Analog to Digital Converters
+
+maintainers:
+  - Marcelo Schmitt <[email protected]>
+
+description: |
+  Analog Devices AD4000 family of Analog to Digital Converters with SPI support.
+  Specifications can be found at:
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
+
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - adi,ad4000
+      - adi,ad4001
+      - adi,ad4002
+      - adi,ad4003
+      - adi,ad4004
+      - adi,ad4005
+      - adi,ad4006
+      - adi,ad4007
+      - adi,ad4008
+      - adi,ad4010
+      - adi,ad4011
+      - adi,ad4020
+      - adi,ad4021
+      - adi,ad4022
+      - adi,adaq4001
+      - adi,adaq4003
+
+  reg:

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml line 55

Quoted 10 lines

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

+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 102040816 # for VIO > 2.7 V, 81300813 for VIO > 1.7 V
+
+  spi-cpha: true
+
+  adi,spi-mode:
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [ single, chain ]

It sounds like there are more possible wiring configurations for these chips that I thought when suggesting reusing this binding from AD7944 so we probably need more options here. (see my reply to the cover letter for the complete context of these remarks)

We identified A) an additional wiring configuration where SDI of the ADC chip is wired to SDO of the SPI controller and B) a potential need to pin mux between wiring modes to work around SPI controller limitations perhaps we could omit the adi,spi-mode property and just use the standard pinctrl properties.

pinctrl-names:
description: |
Names for possible ways the SDI line of the controller is wired.

* default: The SDI line of the ADC is connected to the SDO line of the SPI controller. CNV line of the ADC is connected to CS of the SPI controller.

* single: The datasheet calls this "3-wire mode". (NOTE: The datasheet's definition of 3-wire mode is NOT at all related to the standard spi-3wire property!) In this mode, SDI is tied to VIO, and the CNV line can be connected to the CS line of the SPI controller (typical) or to a GPIO, in which case the CS line of the controller is unused. The SDO line of the SPI controller is not connected.
* multi: The datasheet calls this "4-wire mode" and is used when multiple chips are connected in parallel. In this mode, the ADC SDI line is tied to the CS line on the SPI controller and the CNV line is connected to a GPIO. The SDO line of the SPI controller is not connected.
* chain: The datasheet calls this "chain mode". This mode is used to save on wiring when multiple ADCs are used. In this mode, the SDI line of one chip is tied to the SDO of the next chip in the chain and the SDI of the last chip in the chain is tied to GND. Only the first chip in the chain is connected to the SPI bus. The CNV line of all chips are tied together. The CS line of the SPI controller can be used as the CNV line only if it is active high.

If one name is specified, it is assumed the chip is hard-wired in this configuration.

If two names are specified, it is assumed that a pinmux can switch between the two wiring configurations. The first is the default mode for reading and writing registers on the chip and the second is the mode for reading the conversion data from the chip.
oneOf:
- items:
- enum:
- default
- single
- multi
- chain
- items:
- const: default
- enum:
- single
- multi
- chain

pinctrl-0:
maxItems: 1

pinctrl-1:
maxItems: 1

Quoted 42 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml new file +42 -0

+    description: |
+      This property indicates the SPI wiring configuration.
+
+      When this property is omitted, it is assumed that the device is using what
+      the datasheet calls "4-wire mode". This is the conventional SPI mode used
+      when there are multiple devices on the same bus. In this mode, the CNV
+      line is used to initiate the conversion and the SDI line is connected to
+      CS on the SPI controller.
+
+      When this property is present, it indicates that the device is using one
+      of the following alternative wiring configurations:
+
+      * single: The datasheet calls this "3-wire mode". (NOTE: The datasheet's
+        definition of 3-wire mode is NOT at all related to the standard
+        spi-3wire property!) This mode is often used when the ADC is the only
+        device on the bus. In this mode, SDI is tied to VIO, and the CNV line
+        can be connected to the CS line of the SPI controller or to a GPIO, in
+        which case the CS line of the controller is unused.
+      * chain: The datasheet calls this "chain mode". This mode is used to save
+        on wiring when multiple ADCs are used. In this mode, the SDI line of
+        one chip is tied to the SDO of the next chip in the chain and the SDI of
+        the last chip in the chain is tied to GND. Only the first chip in the
+        chain is connected to the SPI bus. The CNV line of all chips are tied
+        together. The CS line of the SPI controller can be used as the CNV line
+        only if it is active high.
+
+  '#daisy-chained-devices': true
+
+  vdd-supply:
+    description: A 1.8V supply that powers the chip (VDD).
+
+  vio-supply:
+    description:
+      A 1.8V to 5.5V supply for the digital inputs and outputs (VIO).
+
+  ref-supply:
+    description:
+      A 2.5 to 5V supply for the external reference voltage (REF).
+
+  cnv-gpios:
+    description:
+      The Convert Input (CNV). This input has multiple functions. It initiates

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml line 101

Quoted 4 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml new file +4 -0

+      the conversions and selects the SPI mode of the device (chain or CS). In
+      'single' mode, this property is omitted if the CNV pin is connected to the
+      CS line of the SPI controller. If 'single' mode is selected and this GPIO
+      is provided, it must be active low.

Since the conversion is triggered on the low to high transition of CNV, I think it only makes sense to have it active high and not active low.

Quoted 9 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml new file +8 -0

+    maxItems: 1
+
+  adi,high-z-input:
+    type: boolean
+    description:
+      High-Z mode allows the amplifier and RC filter in front of the ADC to be
+      chosen based on the signal bandwidth of interest, rather than the settling
+      requirements of the switched capacitor SAR ADC inputs.

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml line 119

Quoted 10 lines

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

+
+  adi,gain-milli:
+    description: |
+      The hardware gain applied to the ADC input (in milli units).
+      The gain provided by the ADC input scaler is defined by the hardware
+      connections between chip pins OUT+, R1K-, R1K1-, R1K+, R1K1+, and OUT-.
+      If not present, default to 1000 (no actual gain applied).
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [454, 909, 1000, 1900]
+    default: 1000

Same suggestion as in V1 - we should make it clear that this property only applies to ADAQ chips (in the description and also a -if: for the bindings validator). Also, looking at the datasheet, it looks like there are a lot more pins on the ADAQ chips, so I think there are more properties missing here.

Some trivial ones:

vs-pos-supply (VS+ pin, 0 to 11V supply) and vs-neg-supply (VS- pin, -11 to 0V supply)

pd-amp-gpios (active low) and pd-ref-gpios (active low) for optional runtime power management.

Also the datasheet says the ADAQ chips supports "Single-ended to differential conversion". So it seems like we might need some extra properties to describe that case (a flag for indicating single-ended wiring and an optional voltage supply to describe what is connected to the negative input if it isn't tied to GND)

Quoted 14 lines · David Lechner

Quoted 11 lines · Marcelo Schmitt

Add device tree documentation for AD4000 family of ADC devices.

Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf

Quoted 3 lines

Suggested-by David Lechner <[email protected]>

(if you still use mostly my suggestions in the end)

Yes, it's been of great help. Will include the tag in future ad4000 DT patches.

Quoted 82 lines

Quoted 59 lines

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

---
 .../bindings/iio/adc/adi,ad4000.yaml          | 201 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 208 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml new file +45 -0

new file mode 100644
index 000000000000..ca06afb5149e
@@ -0,0 +1,201 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices AD4000 and similar Analog to Digital Converters
+
+maintainers:
+  - Marcelo Schmitt <[email protected]>
+
+description: |
+  Analog Devices AD4000 family of Analog to Digital Converters with SPI support.
+  Specifications can be found at:
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4000-4004-4008.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4001-4005.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4003-4007-4011.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
+    https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
+
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - adi,ad4000
+      - adi,ad4001
+      - adi,ad4002
+      - adi,ad4003
+      - adi,ad4004
+      - adi,ad4005
+      - adi,ad4006
+      - adi,ad4007
+      - adi,ad4008
+      - adi,ad4010
+      - adi,ad4011
+      - adi,ad4020
+      - adi,ad4021
+      - adi,ad4022
+      - adi,adaq4001
+      - adi,adaq4003
+
+  reg:

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml line 55

Quoted 10 lines

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

+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 102040816 # for VIO > 2.7 V, 81300813 for VIO > 1.7 V
+
+  spi-cpha: true
+
+  adi,spi-mode:
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [ single, chain ]  

It sounds like there are more possible wiring configurations for these chips that I thought when suggesting reusing this binding from AD7944 so we probably need more options here. (see my reply to the cover letter for the complete context of these remarks)

We identified A) an additional wiring configuration where SDI of the ADC chip is wired to SDO of the SPI controller and B) a potential need to pin mux between wiring modes to work around SPI controller limitations perhaps we could omit the adi,spi-mode property and just use the standard pinctrl properties.

pinctrl-names:

Quoted 6 lines

description: |
Names for possible ways the SDI line of the controller is wired.

* default: The SDI line of the ADC is connected to the SDO line of the SPI controller. CNV line of the ADC is connected to CS of the SPI controller.

Not sure if should be DT, but maybe also point out that in default mode the SPI controller must be capable of keeping ADC SDI (controller SDO) line high during ADC conversions.

Quoted 44 lines

* single: The datasheet calls this "3-wire mode". (NOTE: The datasheet's definition of 3-wire mode is NOT at all related to the standard spi-3wire property!) In this mode, SDI is tied to VIO, and the CNV line can be connected to the CS line of the SPI controller (typical) or to a GPIO, in which case the CS line of the controller is unused. The SDO line of the SPI controller is not connected.
* multi: The datasheet calls this "4-wire mode" and is used when multiple chips are connected in parallel. In this mode, the ADC SDI line is tied to the CS line on the SPI controller and the CNV line is connected to a GPIO. The SDO line of the SPI controller is not connected.
* chain: The datasheet calls this "chain mode". This mode is used to save on wiring when multiple ADCs are used. In this mode, the SDI line of one chip is tied to the SDO of the next chip in the chain and the SDI of the last chip in the chain is tied to GND. Only the first chip in the chain is connected to the SPI bus. The CNV line of all chips are tied together. The CS line of the SPI controller can be used as the CNV line only if it is active high.

If one name is specified, it is assumed the chip is hard-wired in this configuration.

If two names are specified, it is assumed that a pinmux can switch between the two wiring configurations. The first is the default mode for reading and writing registers on the chip and the second is the mode for reading the conversion data from the chip.
oneOf:
- items:
- enum:
- default
- single
- multi
- chain
- items:
- const: default
- enum:
- single
- multi
- chain

pinctrl-0:
maxItems: 1

pinctrl-1:
maxItems: 1

Quoted 4 lines · On Tue, 9 Apr 2024 12:30:09 -0300 Marcelo Schmitt <[email protected]>…

Quoted 3 lines · David Lechner

Quoted 1 line · Marcelo Schmitt

Quoted 10 lines

Quoted 10 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml line 55

Quoted 4 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml new file +4 -0

+
+  adi,spi-mode:
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [ single, chain ]  

It sounds like there are more possible wiring configurations for these chips that I thought when suggesting reusing this binding from AD7944 so we probably need more options here. (see my reply to the cover letter for the complete context of these remarks)

Quoted 7 lines

Quoted 7 lines

We identified A) an additional wiring configuration where SDI of the ADC chip is wired to SDO of the SPI controller and B) a potential need to pin mux between wiring modes to work around SPI controller limitations perhaps we could omit the adi,spi-mode property and just use the standard pinctrl properties.

pinctrl-names:

I'm lost on how pinctrl makes sense here.
Yes you are changing the modes of the pins, but not in a conventional sense of some register that is being updated to say now use them like this. The mode is dependent on the timing sequence of how the pins are used. Otherwise looking at it a different way it's an external wiring thing we aren't controlling it at all. Is pinctrl suitable for that?
I always thought of it as a way to change configurations of SoC pins.

A pointer to some precendence in another driver for using it like this would go some way towards convincing me.

Jonathan

Quoted 7 lines · Jonathan Cameron

Quoted 4 lines · On Tue, 9 Apr 2024 12:30:09 -0300 Marcelo Schmitt <[email protected]>…

Quoted 3 lines · David Lechner

Quoted 1 line · Marcelo Schmitt

...

Quoted 18 lines

Quoted 10 lines

Quoted 10 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml line 55

Quoted 4 lines

Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml new file +4 -0

+
+  adi,spi-mode:
+    $ref: /schemas/types.yaml#/definitions/string
+    enum: [ single, chain ]    

It sounds like there are more possible wiring configurations for these chips that I thought when suggesting reusing this binding from AD7944 so we probably need more options here. (see my reply to the cover letter for the complete context of these remarks)

Quoted 7 lines

Quoted 7 lines

We identified A) an additional wiring configuration where SDI of the ADC chip is wired to SDO of the SPI controller and B) a potential need to pin mux between wiring modes to work around SPI controller limitations perhaps we could omit the adi,spi-mode property and just use the standard pinctrl properties.

pinctrl-names:

Quoted 7 lines

I'm lost on how pinctrl makes sense here.
Yes you are changing the modes of the pins, but not in a conventional sense of some register that is being updated to say now use them like this. The mode is dependent on the timing sequence of how the pins are used. Otherwise looking at it a different way it's an external wiring thing we aren't controlling it at all. Is pinctrl suitable for that?
I always thought of it as a way to change configurations of SoC pins.

Yes, this is exactly what I think we need here.

To write to the register, the chip has to be wired like this ("default"):

                                         +-------------+
     +-----------------------------------| SDO         |
     |                                   |             |
     |              +--------------------| CS          |
     |              v                    |             |
     |    +--------------------+         |     HOST    |
     |    |        CNV         |         |             |
     +--->| SDI   AD7944   SDO |-------->| SDI         |
          |        SCK         |         |             |
          +--------------------+         |             |
                    ^                    |             |
                    +--------------------| SCLK        |
                                         +-------------+

But to read sample data, the chip has to be wired in one of these 3 configurations:

3-wire mode ("single"):

                                         +-------------+
                    +--------------------| CS          |
                    v                    |             |
    VIO   +--------------------+         |     HOST    |
     |    |        CNV         |         |             |
     +--->| SDI   AD7944   SDO |-------->| SDI         |
          |        SCK         |         |             |
          +--------------------+         |             |
                    ^                    |             |
                    +--------------------| SCLK        |
                                         +-------------+

4-wire mode ("multi"):

                                         +-------------+
     +-----------------------------------| CS          |
     |                                   |             |
     |              +--------------------| GPIO        |
     |              v                    |             |
     |    +--------------------+         |     HOST    |
     |    |        CNV         |         |             |
     +--->| SDI   AD7944   SDO |-------->| SDI         |
          |        SCK         |         |             |
          +--------------------+         |             |
                    ^                    |             |
                    +--------------------| SCLK        |
                                         +-------------+

Chain mode ("chain"):

                                         +-------------+
                    +--------------------| CS          |
                    v                    |             |
          +--------------------+         |     HOST    |
          |        CNV         |         |             |
     +--->| SDI   AD7944   SDO |-------->| SDI         |
     |    |        SCK         |         |             |
    GND   +--------------------+         |             |
                    ^                    |             |
                    +--------------------| SCLK        |
                                         +-------------+

If we want to be able to both write the register and read data, some reconfiguration is needed. It might be possible to read data using the register-write wiring configuration, but that only works if SDO can be set to the correct state *before* the CS line changes. This is not something that I think most SPI controllers can do (e.g. Marcelo mentioned in the cover letter that RPi always returns SDO to low after every xfer while the AXI SPI Engine leaves SDO wherever it was last).

Quoted 6 lines

A pointer to some precendence in another driver for using it like this would go some way towards convincing me.

Jonathan

I didn't find much precedence for something like this, but I found devicetree/bindings/net/mediatek-bluetooth.txt that uses pinctrl to pull a UART Rx pin low for a bootstrap mode which sounds very similar to what we need to do here (pull the SPI controller SDO pin high or low for 3-wire or chain mode).

For example, if we wanted to use 3-wire mode for reading data, we would set the pinctrl to "default" to write the register to configure the chip during driver probe. Then to read data, we would change the pinctrl to "single" before doing the SPI xfer to ensure that the ADC SDI pin is pulled high independent of what the SDO line of the SPI controller is currently doing.

Quoted 11 lines · On Sat, 13 Apr 2024 12:33:54 -0500 David Lechner <[email protected]> wrote:

Quoted 7 lines · Jonathan Cameron

Quoted 4 lines · On Tue, 9 Apr 2024 12:30:09 -0300 Marcelo Schmitt <[email protected]>…

Quoted 3 lines · David Lechner

Quoted 1 line · Marcelo Schmitt

...

Quoted 41 lines

Quoted 9 lines

Quoted 8 lines

Quoted 8 lines

We identified A) an additional wiring configuration where SDI of the ADC chip is wired to SDO of the SPI controller and B) a potential need to pin mux between wiring modes to work around SPI controller limitations perhaps we could omit the adi,spi-mode property and just use the standard pinctrl properties.

pinctrl-names:

Quoted 7 lines

I'm lost on how pinctrl makes sense here.
Yes you are changing the modes of the pins, but not in a conventional sense of some register that is being updated to say now use them like this. The mode is dependent on the timing sequence of how the pins are used. Otherwise looking at it a different way it's an external wiring thing we aren't controlling it at all. Is pinctrl suitable for that?
I always thought of it as a way to change configurations of SoC pins.

Yes, this is exactly what I think we need here.

To write to the register, the chip has to be wired like this ("default"):

                                         +-------------+
     +-----------------------------------| SDO         |
     |                                   |             |
     |              +--------------------| CS          |
     |              v                    |             |
     |    +--------------------+         |     HOST    |
     |    |        CNV         |         |             |
     +--->| SDI   AD7944   SDO |-------->| SDI         |
          |        SCK         |         |             |
          +--------------------+         |             |
                    ^                    |             |
                    +--------------------| SCLK        |
                                         +-------------+

But to read sample data, the chip has to be wired in one of these 3 configurations:

3-wire mode ("single"):

Quoted 12 lines

                                         +-------------+
                    +--------------------| CS          |
                    v                    |             |
    VIO   +--------------------+         |     HOST    |
     |    |        CNV         |         |             |
     +--->| SDI   AD7944   SDO |-------->| SDI         |
          |        SCK         |         |             |
          +--------------------+         |             |
                    ^                    |             |
                    +--------------------| SCLK        |
                                         +-------------+

Quoted 18 lines

If we want to be able to both write the register and read data, some reconfiguration is needed. It might be possible to read data using the register-write wiring configuration, but that only works if SDO can be set to the correct state *before* the CS line changes. This is not something that I think most SPI controllers can do (e.g. Marcelo mentioned in the cover letter that RPi always returns SDO to low after every xfer while the AXI SPI Engine leaves SDO wherever it was last).

Quoted 6 lines

A pointer to some precendence in another driver for using it like this would go some way towards convincing me.

Jonathan

Quoted 6 lines

I didn't find much precedence for something like this, but I found devicetree/bindings/net/mediatek-bluetooth.txt that uses pinctrl to pull a UART Rx pin low for a bootstrap mode which sounds very similar to what we need to do here (pull the SPI controller SDO pin high or low for 3-wire or chain mode).

Quoted 7 lines

For example, if we wanted to use 3-wire mode for reading data, we would set the pinctrl to "default" to write the register to configure the chip during driver probe. Then to read data, we would change the pinctrl to "single" before doing the SPI xfer to ensure that the ADC SDI pin is pulled high independent of what the SDO line of the SPI controller is currently doing.

Ah ok. This is implying that we are switching to a controllable mode to pull that pin high (or I guess one where it is always high). I'm not sure if that's more common than an SPI controller where you can set the 'don't' care state to high or low. I assume we can't get away with just setting the output buffer to all 1s because it won't hold that state between transfers?

Feels like that could be rolled into the SPI subsystem with any necessary transitions handled there (maybe)

Just to check, this isn't just a case of a missing pull up resistor to drag that line correctly when it isn't actively driven (combined with all 1s in the write buffer) etc?

Jonathan

So, I have been trying to make this work, though I haven't been successful so far, and I don't really think using pinctrl is a good solution for this either.

Comments inline.

Quoted 14 lines · Jonathan Cameron

Quoted 11 lines · On Sat, 13 Apr 2024 12:33:54 -0500 David Lechner <[email protected]> wrote:

Quoted 7 lines · Jonathan Cameron

Quoted 4 lines · On Tue, 9 Apr 2024 12:30:09 -0300 Marcelo Schmitt <[email protected]>…

Quoted 3 lines · David Lechner

Quoted 1 line · Marcelo Schmitt

...

...

Quoted 41 lines

Quoted 41 lines

Quoted 9 lines

Quoted 8 lines

Quoted 8 lines

We identified A) an additional wiring configuration where SDI of the ADC chip is wired to SDO of the SPI controller and B) a potential need to pin mux between wiring modes to work around SPI controller limitations perhaps we could omit the adi,spi-mode property and just use the standard pinctrl properties.

pinctrl-names:

Quoted 7 lines

I'm lost on how pinctrl makes sense here.
Yes you are changing the modes of the pins, but not in a conventional sense of some register that is being updated to say now use them like this. The mode is dependent on the timing sequence of how the pins are used. Otherwise looking at it a different way it's an external wiring thing we aren't controlling it at all. Is pinctrl suitable for that?
I always thought of it as a way to change configurations of SoC pins.

Yes, this is exactly what I think we need here.

To write to the register, the chip has to be wired like this ("default"):

                                         +-------------+
     +-----------------------------------| SDO         |
     |                                   |             |
     |              +--------------------| CS          |
     |              v                    |             |
     |    +--------------------+         |     HOST    |
     |    |        CNV         |         |             |
     +--->| SDI   AD7944   SDO |-------->| SDI         |
          |        SCK         |         |             |
          +--------------------+         |             |
                    ^                    |             |
                    +--------------------| SCLK        |
                                         +-------------+

But to read sample data, the chip has to be wired in one of these 3 configurations:

3-wire mode ("single"):

Quoted 12 lines

Quoted 12 lines

                                         +-------------+
                    +--------------------| CS          |
                    v                    |             |
    VIO   +--------------------+         |     HOST    |
     |    |        CNV         |         |             |
     +--->| SDI   AD7944   SDO |-------->| SDI         |
          |        SCK         |         |             |
          +--------------------+         |             |
                    ^                    |             |
                    +--------------------| SCLK        |
                                         +-------------+

3-wire mode like setup can be achieved if the SPI controller is capable of keeping the ADC SDI line high (at VIO level) during ADC sampling, but keeping controller SDO high throughout the entire transfer is not a thing all SPI controllers can do (RPi's can't).
If the ADC is hard wired connected as in the 3-wire diagram then the user can't write/read the configuration register. Same applies to "4-wire" mode where controller CS is connected to ADC SDI.
The whole point of having pinctrl configurations was to make it possible to both read/write config register and do ADC sampling if the SPI controller can't keep ADC SDI at VIO. I don't think pinctrl can solve this problem though.

...

Quoted 18 lines

Quoted 18 lines

If we want to be able to both write the register and read data, some reconfiguration is needed. It might be possible to read data using the register-write wiring configuration, but that only works if SDO can be set to the correct state *before* the CS line changes. This is not something that I think most SPI controllers can do (e.g. Marcelo mentioned in the cover letter that RPi always returns SDO to low after every xfer while the AXI SPI Engine leaves SDO wherever it was last).

Quoted 6 lines

A pointer to some precendence in another driver for using it like this would go some way towards convincing me.

Jonathan

Quoted 6 lines

Quoted 6 lines

I didn't find much precedence for something like this, but I found devicetree/bindings/net/mediatek-bluetooth.txt that uses pinctrl to pull a UART Rx pin low for a bootstrap mode which sounds very similar to what we need to do here (pull the SPI controller SDO pin high or low for 3-wire or chain mode).

The pinctrl configuration for this ADC would not be meant to change once after boot as it looks to be the usual use case for pinctrl (including mediatek-bluetooth.txt).

Also, no suitable mux for the "3-wire" mode in Documentation/devicetree/bindings/pinctrl/xlnx,pinctrl-zynq.yaml to do it like Documentation/devicetree/bindings/net/mediatek-bluetooth.txt. The zynq pinctrl driver (drivers/pinctrl/pinctrl-zynq.c) would have to be updated to add the new mux function in
static const struct zynq_pinmux_function zynq_pmux_functions[] = {
DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet0, 1),
...
DEFINE_ZYNQ_PINMUX_FUNCTION(axi_spi_single, 1), DEFINE_ZYNQ_PINMUX_FUNCTION(axi_spi_multi, 2),
though this is not really a thing that's on zynq, but one that is related to these ADCs so I'm not sure it should go there.

Quoted 8 lines

Quoted 8 lines

For example, if we wanted to use 3-wire mode for reading data, we would set the pinctrl to "default" to write the register to configure the chip during driver probe. Then to read data, we would change the pinctrl to "single" before doing the SPI xfer to ensure that the ADC SDI pin is pulled high independent of what the SDO line of the SPI controller is currently doing.

No, the pin configuration for this ADCs would be expected to change unrestricted amount of times. The pin configuration would have to change every time a sample read is made after a register access transfers and vice-versa. If we decide to support span compression, every change to _scale would lead to pinctrl configuration change.

At pin level, we would want to rise SPI controller SDO line to VIO but then the new SDO pin config would conflict with SPI pin group config.

I included pinctrl properties in my test dts to make use of pinctrl framework. Yet, that doesn't really alternate SPI line configuration we are using because the SPI function that is in the PS (FPGA's Processing System) is not what we are interfacing when using spi-engine. Copy of my test dts at end of email.

Currently, the SPI controller we are using to test with these exotic ADCs is the spi-engine which is implemented in the FPGA as an IP block which owns control of the bus lines (SPI, SDO, CS, ...). To alternate the configuration of SPI lines (pull SDO (ADC SDI) up to VIO, connect controller CS to ADC SDI, etc.) I think it should be done in the HDL project. I don't think it's a good idea to hijack spi-engine lines through pinctrl.

Quoted 8 lines

Ah ok. This is implying that we are switching to a controllable mode to pull that pin high (or I guess one where it is always high). I'm not sure if that's more common than an SPI controller where you can set the 'don't' care state to high or low. I assume we can't get away with just setting the output buffer to all 1s because it won't hold that state between transfers?

I tried sending the tx buffer filled with 1s when testing this with a RPi4 but it brought the controller SDO (ADC SDI) line low between each 8 bits of transfer (attaching an image (yellow line is SCLK, green lines is controller SDO)).

Anyway, can we have any guaranties with respect to controller SDO line behaviour when its not clocking out data?

Quoted 1 line

Quoted 2 lines

Feels like that could be rolled into the SPI subsystem with any necessary transitions handled there (maybe)

To me, this sounds more reasonable than pinctrl.
Yeah, if we can set a don't' care state for the SDO line then that should be enough for these ADCs.
Otherwise, can we really do anything if the controller can't keep SDO high?

Quoted 5 lines

Just to check, this isn't just a case of a missing pull up resistor to drag that line correctly when it isn't actively driven (combined with all 1s in the write buffer) etc?

When using spi-engine, the controller SDO is connected to ADC SDI, controller CS to ADC CNV and, for reg access, it works as conventional SPI. spi-engine leaves the SDO line the state it was after last transfer so it we can make it idle high during sample read. No hardware pull-up needed.

Marcelo

Quoted 4 lines

Jonathan

The device tree source file I was using for testing with the ADC with the changes for using pinctrl. Didn't really work.

// SPDX-License-Identifier: GPL-2.0
/*
* Analog Devices ADAQ4003
* https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad400x
* https://wiki.analog.com/resources/eval/user-guides/ad400x
*
* hdl_project: <pulsar_adc_pmdz/zed>
* board_revision: <>
*
* Copyright (C) 2016-2023 Analog Devices Inc.
*/
/dts-v1/;

#include "zynq-zed.dtsi"
#include "zynq-zed-adv7511.dtsi"
#include <dt-bindings/pinctrl/pinctrl-zynq.h>

/ {
	adc_vref: regulator-vref {
		compatible = "regulator-fixed";
		regulator-name = "EVAL 5V Vref";
		regulator-min-microvolt = <5000000>;
		regulator-max-microvolt = <5000000>;
		regulator-always-on;
	};

	adc_vdd: regulator-vdd {
		compatible = "regulator-fixed";
		regulator-name = "Eval VDD supply";
		regulator-min-microvolt = <1800000>;
		regulator-max-microvolt = <1800000>;
		regulator-always-on;
	};

	adc_vio: regulator-vio {
		compatible = "regulator-fixed";
		regulator-name = "Eval VIO supply";
		regulator-min-microvolt = <3300000>;
		regulator-max-microvolt = <3300000>;
		regulator-always-on;
	};
};

&pinctrl0 {
	/* Restore conventional SPI pin configuration */
	pinctrl_spi_default: default_config {
		mux {
			/* Are these the ones used by spi-engine? */
			groups = "spi0_0_grp";
			function = "spi0";
		};
		conf {
			groups = "spi0_0_grp";
			power-source = <IO_STANDARD_LVCMOS33>;
		};
		conf-spi-sdo {
			pins = "MIO17"; /* SPI0 SDO? */
			bias-disable;
		};
	};

	/* Pull-up SPI SDO (ADC SDI) to VIO */
	pinctrl_spi_single: single_config {
		conf-spi-sdo {
			pins = "MIO17"; /* Conflicts with SPI0 pin group */
			bias-pull-up;
		};
	};
};

&fpga_axi {
	rx_dma: rx-dmac@44a30000 {
		compatible = "adi,axi-dmac-1.00.a";
		reg = <0x44a30000 0x1000>;
		#dma-cells = <1>;
		interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
		clocks = <&clkc 17>;

		adi,channels {
			#size-cells = <0>;
			#address-cells = <1>;

			dma-channel@0 {
				reg = <0>;
				adi,source-bus-width = <32>;
				adi,source-bus-type = <1>;
				adi,destination-bus-width = <64>;
				adi,destination-bus-type = <0>;
			};
		};
	};

	axi_pwm_gen: axi-pwm-gen@ {
		compatible = "adi,axi-pwmgen";
		reg = <0x44b00000 0x1000>;
		label = "cnv";
		#pwm-cells = <2>;
		clocks = <&spi_clk>;
	};

	spi_clk: axi-clkgen@44a70000 {
		compatible = "adi,axi-clkgen-2.00.a";
		reg = <0x44a70000 0x10000>;
		#clock-cells = <0>;
		clocks = <&clkc 15>, <&clkc 15>;
		clock-names = "s_axi_aclk", "clkin1";
		clock-output-names = "spi_clk";
	};

	axi_spi_engine_0: spi@44a00000 {
		compatible = "adi,axi-spi-engine-1.00.a";
		reg = <0x44a00000 0x1000>;
		interrupt-parent = <&intc>;
		interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
		clocks = <&clkc 15 &spi_clk>;
		clock-names = "s_axi_aclk", "spi_clk";
		num-cs = <1>;

		#address-cells = <0x1>;
		#size-cells = <0x0>;

		adaq4003: adc@0 {
			compatible = "adi,adaq4003";
			reg = <0>;
			spi-max-frequency = <102000000>;
			spi-cpha;
			pinctrl-names = "default", "single";
			pinctrl-0 = <&pinctrl_spi_default>;
			pinctrl-1 = <&pinctrl_spi_single>;
			vdd-supply = <&adc_vdd>;
			vio-supply = <&adc_vio>;
			ref-supply = <&adc_vref>;
			adi,high-z-input;
			adi,gain-milli = <454>;
		};
	};
};

Quoted 23 lines · On Tue, 16 Apr 2024 18:46:11 -0300 Marcelo Schmitt…

So, I have been trying to make this work, though I haven't been successful so far, and I don't really think using pinctrl is a good solution for this either.

Comments inline.

Quoted 14 lines · Jonathan Cameron

Quoted 11 lines · On Sat, 13 Apr 2024 12:33:54 -0500 David Lechner <[email protected]> wrote:

Quoted 7 lines · Jonathan Cameron

Quoted 4 lines · On Tue, 9 Apr 2024 12:30:09 -0300 Marcelo Schmitt <[email protected]>…

Quoted 3 lines · David Lechner

Quoted 1 line · Marcelo Schmitt

...

...

Quoted 4 lines

The pinctrl configuration for this ADC would not be meant to change once after boot as it looks to be the usual use case for pinctrl (including mediatek-bluetooth.txt).

Quoted 12 lines

Also, no suitable mux for the "3-wire" mode in Documentation/devicetree/bindings/pinctrl/xlnx,pinctrl-zynq.yaml to do it like Documentation/devicetree/bindings/net/mediatek-bluetooth.txt. The zynq pinctrl driver (drivers/pinctrl/pinctrl-zynq.c) would have to be updated to add the new mux function in
static const struct zynq_pinmux_function zynq_pmux_functions[] = {
DEFINE_ZYNQ_PINMUX_FUNCTION(ethernet0, 1),
...
DEFINE_ZYNQ_PINMUX_FUNCTION(axi_spi_single, 1), DEFINE_ZYNQ_PINMUX_FUNCTION(axi_spi_multi, 2),
though this is not really a thing that's on zynq, but one that is related to these ADCs so I'm not sure it should go there.

I'd argue we are after a specific SPI controller setup for this. A controller driver would need modifying to make it work.

Quoted 24 lines

Quoted 9 lines

Quoted 9 lines

For example, if we wanted to use 3-wire mode for reading data, we would set the pinctrl to "default" to write the register to configure the chip during driver probe. Then to read data, we would change the pinctrl to "single" before doing the SPI xfer to ensure that the ADC SDI pin is pulled high independent of what the SDO line of the SPI controller is currently doing.

No, the pin configuration for this ADCs would be expected to change unrestricted amount of times. The pin configuration would have to change every time a sample read is made after a register access transfers and vice-versa. If we decide to support span compression, every change to _scale would lead to pinctrl configuration change.

At pin level, we would want to rise SPI controller SDO line to VIO but then the new SDO pin config would conflict with SPI pin group config.

I included pinctrl properties in my test dts to make use of pinctrl framework. Yet, that doesn't really alternate SPI line configuration we are using because the SPI function that is in the PS (FPGA's Processing System) is not what we are interfacing when using spi-engine. Copy of my test dts at end of email.

Quoted 6 lines

Currently, the SPI controller we are using to test with these exotic ADCs is the spi-engine which is implemented in the FPGA as an IP block which owns control of the bus lines (SPI, SDO, CS, ...). To alternate the configuration of SPI lines (pull SDO (ADC SDI) up to VIO, connect controller CS to ADC SDI, etc.) I think it should be done in the HDL project. I don't think it's a good idea to hijack spi-engine lines through pinctrl.

Such functionality would need to be pushed to the spi controller driver which could know if there was any need to do anything like this, or if there was simply a register to set.

Quoted 9 lines

Quoted 8 lines

Ah ok. This is implying that we are switching to a controllable mode to pull that pin high (or I guess one where it is always high). I'm not sure if that's more common than an SPI controller where you can set the 'don't' care state to high or low. I assume we can't get away with just setting the output buffer to all 1s because it won't hold that state between transfers?

Quoted 3 lines

I tried sending the tx buffer filled with 1s when testing this with a RPi4 but it brought the controller SDO (ADC SDI) line low between each 8 bits of transfer (attaching an image (yellow line is SCLK, green lines is controller SDO)).

Pity - thought that was overly optimistic.

Quoted 6 lines

Anyway, can we have any guaranties with respect to controller SDO line behaviour when its not clocking out data?

Quoted 1 line

Quoted 7 lines

Quoted 2 lines

Feels like that could be rolled into the SPI subsystem with any necessary transitions handled there (maybe)

To me, this sounds more reasonable than pinctrl.
Yeah, if we can set a don't' care state for the SDO line then that should be enough for these ADCs.
Otherwise, can we really do anything if the controller can't keep SDO high?

There is one similar (ish) entry already.
https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/spi/spi.h#L29
#define SPI_3WIRE_HIZ _BITUL(15) /* high impedance turnaround */
in that it is controlling state in what I think would normally be a don't care state.

I think we could have an
SPI_SDO_DONT_CARE_HIGH (naming to be improved upon ;)
that a driver could advertise support for and the spi device could request.

https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/panel/panel-tpo-tpg110.c#L429

Implement that in the spi-gpio driver as a PoC probably and in your SPI copntoller driver.

Ultimately if the controller really isn't capable (including dances through pin mode changes if necessary) then the ADC won't work in this wiring with that host controller.

I'd propose something along these lines and see whether Mark + any other active SPI folk think it is reasonable or not?

Quoted 7 lines

Quoted 6 lines

Just to check, this isn't just a case of a missing pull up resistor to drag that line correctly when it isn't actively driven (combined with all 1s in the write buffer) etc?

Quoted 4 lines

When using spi-engine, the controller SDO is connected to ADC SDI, controller CS to ADC CNV and, for reg access, it works as conventional SPI. spi-engine leaves the SDO line the state it was after last transfer so it we can make it idle high during sample read. No hardware pull-up needed.

Fair enough. No multi master support I guess (that is a bit obscure for SPI). A little ugly that it's dependent on the last access - so you would need to do a dummy access if the normal last bit was wrong level?

Jonathan

Quoted 148 lines

Marcelo

Quoted 4 lines

Jonathan

The device tree source file I was using for testing with the ADC with the changes for using pinctrl. Didn't really work.

// SPDX-License-Identifier: GPL-2.0
/*
* Analog Devices ADAQ4003
* https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/ad400x
* https://wiki.analog.com/resources/eval/user-guides/ad400x
*
* hdl_project: <pulsar_adc_pmdz/zed>
* board_revision: <>
*
* Copyright (C) 2016-2023 Analog Devices Inc.
*/
/dts-v1/;

#include "zynq-zed.dtsi"
#include "zynq-zed-adv7511.dtsi"
#include <dt-bindings/pinctrl/pinctrl-zynq.h>

/ {
	adc_vref: regulator-vref {
		compatible = "regulator-fixed";
		regulator-name = "EVAL 5V Vref";
		regulator-min-microvolt = <5000000>;
		regulator-max-microvolt = <5000000>;
		regulator-always-on;
	};

	adc_vdd: regulator-vdd {
		compatible = "regulator-fixed";
		regulator-name = "Eval VDD supply";
		regulator-min-microvolt = <1800000>;
		regulator-max-microvolt = <1800000>;
		regulator-always-on;
	};

	adc_vio: regulator-vio {
		compatible = "regulator-fixed";
		regulator-name = "Eval VIO supply";
		regulator-min-microvolt = <3300000>;
		regulator-max-microvolt = <3300000>;
		regulator-always-on;
	};
};

&pinctrl0 {
	/* Restore conventional SPI pin configuration */
	pinctrl_spi_default: default_config {
		mux {
			/* Are these the ones used by spi-engine? */
			groups = "spi0_0_grp";
			function = "spi0";
		};
		conf {
			groups = "spi0_0_grp";
			power-source = <IO_STANDARD_LVCMOS33>;
		};
		conf-spi-sdo {
			pins = "MIO17"; /* SPI0 SDO? */
			bias-disable;
		};
	};

	/* Pull-up SPI SDO (ADC SDI) to VIO */
	pinctrl_spi_single: single_config {
		conf-spi-sdo {
			pins = "MIO17"; /* Conflicts with SPI0 pin group */
			bias-pull-up;
		};
	};
};

&fpga_axi {
	rx_dma: rx-dmac@44a30000 {
		compatible = "adi,axi-dmac-1.00.a";
		reg = <0x44a30000 0x1000>;
		#dma-cells = <1>;
		interrupts = <0 57 IRQ_TYPE_LEVEL_HIGH>;
		clocks = <&clkc 17>;

		adi,channels {
			#size-cells = <0>;
			#address-cells = <1>;

			dma-channel@0 {
				reg = <0>;
				adi,source-bus-width = <32>;
				adi,source-bus-type = <1>;
				adi,destination-bus-width = <64>;
				adi,destination-bus-type = <0>;
			};
		};
	};

	axi_pwm_gen: axi-pwm-gen@ {
		compatible = "adi,axi-pwmgen";
		reg = <0x44b00000 0x1000>;
		label = "cnv";
		#pwm-cells = <2>;
		clocks = <&spi_clk>;
	};

	spi_clk: axi-clkgen@44a70000 {
		compatible = "adi,axi-clkgen-2.00.a";
		reg = <0x44a70000 0x10000>;
		#clock-cells = <0>;
		clocks = <&clkc 15>, <&clkc 15>;
		clock-names = "s_axi_aclk", "clkin1";
		clock-output-names = "spi_clk";
	};

	axi_spi_engine_0: spi@44a00000 {
		compatible = "adi,axi-spi-engine-1.00.a";
		reg = <0x44a00000 0x1000>;
		interrupt-parent = <&intc>;
		interrupts = <0 56 IRQ_TYPE_LEVEL_HIGH>;
		clocks = <&clkc 15 &spi_clk>;
		clock-names = "s_axi_aclk", "spi_clk";
		num-cs = <1>;

		#address-cells = <0x1>;
		#size-cells = <0x0>;

		adaq4003: adc@0 {
			compatible = "adi,adaq4003";
			reg = <0>;
			spi-max-frequency = <102000000>;
			spi-cpha;
			pinctrl-names = "default", "single";
			pinctrl-0 = <&pinctrl_spi_default>;
			pinctrl-1 = <&pinctrl_spi_single>;
			vdd-supply = <&adc_vdd>;
			vio-supply = <&adc_vio>;
			ref-supply = <&adc_vref>;
			adi,high-z-input;
			adi,gain-milli = <454>;
		};
	};
};

Add support for AD4000 family of low noise, low power, high speed, successive aproximation register (SAR) ADCs.

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

---
 MAINTAINERS              |   1 +
 drivers/iio/adc/Kconfig  |  12 +
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad4000.c | 649 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 663 insertions(+)
 create mode 100644 drivers/iio/adc/ad4000.c

MAINTAINERS +1 -0

index 5dfe118a5dd3..86aa96115f5a 100644
@@ -1165,6 +1165,7 @@ L:        [email protected]
 S:     Supported
 W:     https://ez.analog.com/linux-software-drivers
 F:     Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
+F:     drivers/iio/adc/ad4000.c
 
 ANALOG DEVICES INC AD4130 DRIVER
 M:     Cosmin Tanislav <[email protected]>

drivers/iio/adc/Kconfig +12 -0

index 8db68b80b391..9c9d13d4b74f 100644
@@ -21,6 +21,18 @@ config AD_SIGMA_DELTA
        select IIO_BUFFER
        select IIO_TRIGGERED_BUFFER
 
+config AD4000
+       tristate "Analog Devices AD4000 ADC Driver"
+       depends on SPI
+       select IIO_BUFFER
+       select IIO_TRIGGERED_BUFFER
+       help
+         Say yes here to build support for Analog Devices AD4000 high speed
+         SPI analog to digital converters (ADC).
+
+         To compile this driver as a module, choose M here: the module will be
+         called ad4000.
+
 config AD4130
        tristate "Analog Device AD4130 ADC Driver"
        depends on SPI

drivers/iio/adc/Makefile +1 -0

index edb32ce2af02..aa52068d864b 100644
@@ -6,6 +6,7 @@
 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
 obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
+obj-$(CONFIG_AD4000) += ad4000.o
 obj-$(CONFIG_AD4130) += ad4130.o
 obj-$(CONFIG_AD7091R) += ad7091r-base.o
 obj-$(CONFIG_AD7091R5) += ad7091r5.o

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

new file mode 100644
index 000000000000..7997d9d98743
@@ -0,0 +1,649 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD4000 SPI ADC driver
+ *
+ * Copyright 2024 Analog Devices Inc.
+ */
+#include <asm/unaligned.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+#include <linux/units.h>
+#include <linux/util_macros.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>

drivers/iio/adc/ad4000.c new file +32 -0

+
+#define AD4000_TQUIET2_NS              60
+
+#define AD4000_18BIT_MSK       GENMASK(31, 14)
+#define AD4000_20BIT_MSK       GENMASK(31, 12)
+
+#define AD4000_DIFF_CHANNEL(_sign, _real_bits)                         \
+       {                                                               \
+               .type = IIO_VOLTAGE,                                    \
+               .indexed = 1,                                           \
+               .differential = 1,                                      \
+               .channel = 0,                                           \
+               .channel2 = 1,                                          \
+               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |          \
+                                     BIT(IIO_CHAN_INFO_SCALE),         \
+               .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),\
+               .scan_type = {                                          \
+                       .sign = _sign,                                  \
+                       .realbits = _real_bits,                         \
+                       .storagebits = _real_bits > 16 ? 32 : 16,       \
+                       .shift = _real_bits > 16 ? 32 - _real_bits : 0, \
+                       .endianness = IIO_BE,                           \
+               },                                                      \
+       }                                                               \
+
+#define AD4000_PSEUDO_DIFF_CHANNEL(_sign, _real_bits)                  \
+       {                                                               \
+               .type = IIO_VOLTAGE,                                    \
+               .indexed = 1,                                           \
+               .channel = 0,                                           \
+               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |          \
+                                     BIT(IIO_CHAN_INFO_SCALE) |        \

drivers/iio/adc/ad4000.c new file +86 -0

+
+enum ad4000_ids {
+       ID_AD4000,
+       ID_AD4001,
+       ID_AD4002,
+       ID_AD4003,
+       ID_AD4004,
+       ID_AD4005,
+       ID_AD4006,
+       ID_AD4007,
+       ID_AD4008,
+       ID_AD4010,
+       ID_AD4011,
+       ID_AD4020,
+       ID_AD4021,
+       ID_AD4022,
+       ID_ADAQ4001,
+       ID_ADAQ4003,
+};
+
+struct ad4000_chip_info {
+       const char *dev_name;
+       struct iio_chan_spec chan_spec;
+};
+
+static const struct ad4000_chip_info ad4000_chips[] = {
+       [ID_AD4000] = {
+               .dev_name = "ad4000",
+               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
+       },
+       [ID_AD4001] = {
+               .dev_name = "ad4001",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
+       },
+       [ID_AD4002] = {
+               .dev_name = "ad4002",
+               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
+       },
+       [ID_AD4003] = {
+               .dev_name = "ad4003",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
+       },
+       [ID_AD4004] = {
+               .dev_name = "ad4004",
+               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
+       },
+       [ID_AD4005] = {
+               .dev_name = "ad4005",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
+       },
+       [ID_AD4006] = {
+               .dev_name = "ad4006",
+               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
+       },
+       [ID_AD4007] = {
+               .dev_name = "ad4007",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
+       },
+       [ID_AD4008] = {
+               .dev_name = "ad4008",
+               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
+       },
+       [ID_AD4010] = {
+               .dev_name = "ad4010",
+               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
+       },
+       [ID_AD4011] = {
+               .dev_name = "ad4011",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
+       },
+       [ID_AD4020] = {
+               .dev_name = "ad4020",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
+       },
+       [ID_AD4021] = {
+               .dev_name = "ad4021",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
+       },
+       [ID_AD4022] = {
+               .dev_name = "ad4022",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
+       },
+       [ID_ADAQ4001] = {
+               .dev_name = "adaq4001",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
+       },

drivers/iio/adc/ad4000.c new file +11 -0

+
+struct ad4000_state {
+       struct spi_device *spi;
+       struct gpio_desc *cnv_gpio;
+       int vref;
+       bool status_bits;
+       bool span_comp;
+       bool turbo_mode;
+       bool high_z_mode;
+
+       enum ad4000_gains pin_gain;

drivers/iio/adc/ad4000.c new file +37 -0

+               val = st->vref / 1000;
+               /* Multiply by MILLI here to avoid losing precision */
+               val = mult_frac(val, ad4000_gains_frac[i][1] * MILLI,
+                               ad4000_gains_frac[i][0]);
+               /* Would multiply by NANO here but we already multiplied by MILLI */
+               tmp2 = shift_right((u64)val * MICRO, val2);
+               tmp0 = (int)div_s64_rem(tmp2, NANO, &tmp1);
+               /* Store scale for when span compression is disabled */
+               st->scale_tbl[i][0][0] = tmp0; /* Integer part */
+               st->scale_tbl[i][0][1] = abs(tmp1); /* Fractional part */
+               /* Store scale for when span compression is enabled */
+               st->scale_tbl[i][1][0] = tmp0;
+               if (diff)
+                       st->scale_tbl[i][1][1] = DIV_ROUND_CLOSEST(abs(tmp1) * 4, 5);
+               else
+                       st->scale_tbl[i][1][1] = DIV_ROUND_CLOSEST(abs(tmp1) * 9, 10);
+       }
+}
+
+static int ad4000_write_reg(struct ad4000_state *st, uint8_t val)
+{
+       put_unaligned_be16(AD400X_WRITE_COMMAND << BITS_PER_BYTE | val,
+                          &st->tx_buf);
+       return spi_write(st->spi, &st->tx_buf, 2);
+}
+
+static int ad4000_read_reg(struct ad4000_state *st, unsigned int *val)
+{
+       struct spi_transfer t[] = {
+               {
+                       .tx_buf = &st->tx_buf,
+                       .rx_buf = &st->rx_buf,
+                       .len = 2,
+               },
+       };
+       int ret;
+

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

+               {
+                       .rx_buf = &st->scan.data,
+                       .len = BITS_TO_BYTES(chan->scan_type.storagebits),
+                       .delay = {
+                               .value = AD4000_TQUIET2_NS,
+                               .unit = SPI_DELAY_UNIT_NSECS,
+                       },
+               },
+       };
+       int ret;
+
+       ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
+       if (ret < 0)
+               return ret;
+
+       return 0;
+}

drivers/iio/adc/ad4000.c new file +66 -0

+
+       switch (chan->scan_type.realbits) {
+       case 16:
+               break;
+       case 18:
+               sample = FIELD_GET(AD4000_18BIT_MSK, sample);
+               break;
+       case 20:
+               sample = FIELD_GET(AD4000_20BIT_MSK, sample);
+               break;
+       default:
+               return -EINVAL;
+       }
+
+       if (chan->scan_type.sign == 's')
+               *val = sign_extend32(sample, chan->scan_type.realbits - 1);
+
+       return IIO_VAL_INT;
+}
+
+static int ad4000_read_raw(struct iio_dev *indio_dev,
+                          struct iio_chan_spec const *chan, int *val,
+                          int *val2, long info)
+{
+       struct ad4000_state *st = iio_priv(indio_dev);
+
+       switch (info) {
+       case IIO_CHAN_INFO_RAW:
+               iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
+                       return ad4000_single_conversion(indio_dev, chan, val);
+               unreachable();
+       case IIO_CHAN_INFO_SCALE:
+               *val = st->scale_tbl[st->pin_gain][st->span_comp][0];
+               *val2 = st->scale_tbl[st->pin_gain][st->span_comp][1];
+               return IIO_VAL_INT_PLUS_NANO;
+       case IIO_CHAN_INFO_OFFSET:
+               *val = 0;
+               if (st->span_comp)
+                       *val = mult_frac(st->vref / 1000, 1, 10);
+
+               return IIO_VAL_INT;
+       default:
+               break;
+       }
+
+       return -EINVAL;
+}
+
+static int ad4000_read_avail(struct iio_dev *indio_dev,
+                            struct iio_chan_spec const *chan,
+                            const int **vals, int *type, int *length,
+                            long info)
+{
+       struct ad4000_state *st = iio_priv(indio_dev);
+
+       switch (info) {
+       case IIO_CHAN_INFO_SCALE:
+               *vals = (int *)st->scale_tbl[st->pin_gain];
+               *length = 2 * 2;
+               *type = IIO_VAL_INT_PLUS_NANO;
+               return IIO_AVAIL_LIST;
+       default:
+               return -EINVAL;
+       }
+}
+

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

+}
+
+static int ad4000_write_raw(struct iio_dev *indio_dev,
+                           struct iio_chan_spec const *chan, int val, int val2,
+                           long mask)
+{
+       struct ad4000_state *st = iio_priv(indio_dev);
+       unsigned int reg_val;
+       bool span_comp_en;
+       int ret;
+
+       switch (mask) {
+       case IIO_CHAN_INFO_SCALE:
+               iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
+                       ret = ad4000_read_reg(st, &reg_val);
+                       if (ret < 0)
+                               return ret;
+
+                       span_comp_en = (val2 == st->scale_tbl[st->pin_gain][1][1]);
+                       reg_val &= ~AD4000_SPAN_COMP;

drivers/iio/adc/ad4000.c new file +33 -0

+       }
+
+       return -EINVAL;
+}
+
+static irqreturn_t ad4000_trigger_handler(int irq, void *p)
+{
+       struct iio_poll_func *pf = p;
+       struct iio_dev *indio_dev = pf->indio_dev;
+       struct ad4000_state *st = iio_priv(indio_dev);
+       int ret;
+
+       if (st->cnv_gpio)
+               gpiod_set_value(st->cnv_gpio, GPIOD_OUT_HIGH);
+
+       ret = ad4000_read_sample(st, &indio_dev->channels[0]);
+       if (ret < 0)
+               goto err_out;
+
+       if (st->cnv_gpio)
+               gpiod_set_value(st->cnv_gpio, GPIOD_OUT_LOW);
+
+       iio_push_to_buffers_with_timestamp(indio_dev, &st->scan,
+                                          iio_get_time_ns(indio_dev));
+
+err_out:
+       iio_trigger_notify_done(indio_dev->trig);
+       return IRQ_HANDLED;
+}
+
+static const struct iio_info ad4000_info = {
+       .read_raw = &ad4000_read_raw,
+       .read_avail = &ad4000_read_avail,

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

+}
+
+static void ad4000_regulator_disable(void *reg)
+{

drivers/iio/adc/ad4000.c new file +33 -0

+
+       indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+       if (!indio_dev)
+               return -ENOMEM;
+
+       chip = spi_get_device_match_data(spi);
+       if (!chip)
+               return -EINVAL;
+
+       st = iio_priv(indio_dev);
+       st->spi = spi;
+
+       ret = devm_regulator_get_enable(&spi->dev, "vdd");
+       if (ret)
+               return dev_err_probe(&spi->dev, ret, "Failed to enable VDD supply\n");
+
+       ret = devm_regulator_get_enable(&spi->dev, "vio");
+       if (ret)
+               return dev_err_probe(&spi->dev, ret, "Failed to enable VIO supply\n");
+
+       vref_reg = devm_regulator_get(&spi->dev, "ref");
+       if (IS_ERR(vref_reg))
+               return dev_err_probe(&spi->dev, PTR_ERR(vref_reg),
+                                    "Failed to get vref regulator\n");
+
+       ret = regulator_enable(vref_reg);
+       if (ret < 0)
+               return dev_err_probe(&spi->dev, ret,
+                                    "Failed to enable voltage regulator\n");
+
+       ret = devm_add_action_or_reset(&spi->dev, ad4000_regulator_disable, vref_reg);
+       if (ret)
+               return dev_err_probe(&spi->dev, ret,

drivers/iio/adc/ad4000.c new file +5 -0

+
+               return dev_err_probe(&spi->dev, PTR_ERR(st->cnv_gpio),
+                                    "Failed to get CNV GPIO");
+       }
+

drivers/iio/adc/ad4000.c new file +11 -0

+
+               ret = device_property_read_u32(&spi->dev, "adi,gain-milli", &val);
+               if (ret)
+                       return ret;
+
+               switch (val) {
+               case 454:
+                       st->pin_gain = AD4000_0454_GAIN;
+                       break;
+               case 909:
+                       st->pin_gain = AD4000_0909_GAIN;

drivers/iio/adc/ad4000.c new file +79 -1

+               }
+       }
+
+       /*
+        * ADCs that output twos complement code have one less bit to express
+        * voltage magnitude.
+        */
+       if (chip->chan_spec.scan_type.sign == 's')
+               ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits - 1,
+                                     chip);
+       else
+               ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits,
+                                     chip);
+
+       ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
+                                             &iio_pollfunc_store_time,
+                                             &ad4000_trigger_handler, NULL);
+       if (ret)
+               return ret;
+
+       return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id ad4000_id[] = {
+       { "ad4000", (kernel_ulong_t)&ad4000_chips[ID_AD4000] },
+       { "ad4001", (kernel_ulong_t)&ad4000_chips[ID_AD4001] },
+       { "ad4002", (kernel_ulong_t)&ad4000_chips[ID_AD4002] },
+       { "ad4003", (kernel_ulong_t)&ad4000_chips[ID_AD4003] },
+       { "ad4004", (kernel_ulong_t)&ad4000_chips[ID_AD4004] },
+       { "ad4005", (kernel_ulong_t)&ad4000_chips[ID_AD4005] },
+       { "ad4006", (kernel_ulong_t)&ad4000_chips[ID_AD4006] },
+       { "ad4007", (kernel_ulong_t)&ad4000_chips[ID_AD4007] },
+       { "ad4008", (kernel_ulong_t)&ad4000_chips[ID_AD4008] },
+       { "ad4010", (kernel_ulong_t)&ad4000_chips[ID_AD4010] },
+       { "ad4011", (kernel_ulong_t)&ad4000_chips[ID_AD4011] },
+       { "ad4020", (kernel_ulong_t)&ad4000_chips[ID_AD4020] },
+       { "ad4021", (kernel_ulong_t)&ad4000_chips[ID_AD4021] },
+       { "ad4022", (kernel_ulong_t)&ad4000_chips[ID_AD4022] },
+       { "adaq4001", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4001] },
+       { "adaq4003", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4003] },
+       { }
+};
+MODULE_DEVICE_TABLE(spi, ad4000_id);
+
+static const struct of_device_id ad4000_of_match[] = {
+       { .compatible = "adi,ad4000", .data = &ad4000_chips[ID_AD4000] },
+       { .compatible = "adi,ad4001", .data = &ad4000_chips[ID_AD4001] },
+       { .compatible = "adi,ad4002", .data = &ad4000_chips[ID_AD4002] },
+       { .compatible = "adi,ad4003", .data = &ad4000_chips[ID_AD4003] },
+       { .compatible = "adi,ad4004", .data = &ad4000_chips[ID_AD4004] },
+       { .compatible = "adi,ad4005", .data = &ad4000_chips[ID_AD4005] },
+       { .compatible = "adi,ad4006", .data = &ad4000_chips[ID_AD4006] },
+       { .compatible = "adi,ad4007", .data = &ad4000_chips[ID_AD4007] },
+       { .compatible = "adi,ad4008", .data = &ad4000_chips[ID_AD4008] },
+       { .compatible = "adi,ad4010", .data = &ad4000_chips[ID_AD4010] },
+       { .compatible = "adi,ad4011", .data = &ad4000_chips[ID_AD4011] },
+       { .compatible = "adi,ad4020", .data = &ad4000_chips[ID_AD4020] },
+       { .compatible = "adi,ad4021", .data = &ad4000_chips[ID_AD4021] },
+       { .compatible = "adi,ad4022", .data = &ad4000_chips[ID_AD4022] },
+       { .compatible = "adi,adaq4001", .data = &ad4000_chips[ID_ADAQ4001] },
+       { .compatible = "adi,adaq4003", .data = &ad4000_chips[ID_ADAQ4003] },
+       { }
+};
+MODULE_DEVICE_TABLE(of, ad4000_of_match);
+
+static struct spi_driver ad4000_driver = {
+       .driver = {
+               .name   = "ad4000",
+               .of_match_table = ad4000_of_match,
+       },
+       .probe          = ad4000_probe,
+       .id_table       = ad4000_id,
+};
+module_spi_driver(ad4000_driver);
+
+MODULE_AUTHOR("Mircea Caprioru <[email protected]>");
+MODULE_AUTHOR("Marcelo Schmitt <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices AD4000 ADC driver");
+MODULE_LICENSE("GPL");
--

2.43.0

Quoted 96 lines · Marcelo Schmitt

Add support for AD4000 family of low noise, low power, high speed, successive aproximation register (SAR) ADCs.

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

---
 MAINTAINERS              |   1 +
 drivers/iio/adc/Kconfig  |  12 +
 drivers/iio/adc/Makefile |   1 +
 drivers/iio/adc/ad4000.c | 649 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 663 insertions(+)
 create mode 100644 drivers/iio/adc/ad4000.c

MAINTAINERS +1 -0

index 5dfe118a5dd3..86aa96115f5a 100644
@@ -1165,6 +1165,7 @@ L:        [email protected]
 S:     Supported
 W:     https://ez.analog.com/linux-software-drivers
 F:     Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
+F:     drivers/iio/adc/ad4000.c
 
 ANALOG DEVICES INC AD4130 DRIVER
 M:     Cosmin Tanislav <[email protected]>

drivers/iio/adc/Kconfig +12 -0

index 8db68b80b391..9c9d13d4b74f 100644
@@ -21,6 +21,18 @@ config AD_SIGMA_DELTA
        select IIO_BUFFER
        select IIO_TRIGGERED_BUFFER
 
+config AD4000
+       tristate "Analog Devices AD4000 ADC Driver"
+       depends on SPI
+       select IIO_BUFFER
+       select IIO_TRIGGERED_BUFFER
+       help
+         Say yes here to build support for Analog Devices AD4000 high speed
+         SPI analog to digital converters (ADC).
+
+         To compile this driver as a module, choose M here: the module will be
+         called ad4000.
+
 config AD4130
        tristate "Analog Device AD4130 ADC Driver"
        depends on SPI

drivers/iio/adc/Makefile +1 -0

index edb32ce2af02..aa52068d864b 100644
@@ -6,6 +6,7 @@
 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_AB8500_GPADC) += ab8500-gpadc.o
 obj-$(CONFIG_AD_SIGMA_DELTA) += ad_sigma_delta.o
+obj-$(CONFIG_AD4000) += ad4000.o
 obj-$(CONFIG_AD4130) += ad4130.o
 obj-$(CONFIG_AD7091R) += ad7091r-base.o
 obj-$(CONFIG_AD7091R5) += ad7091r5.o

drivers/iio/adc/ad4000.c new file +30 -0

new file mode 100644
index 000000000000..7997d9d98743
@@ -0,0 +1,649 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * AD4000 SPI ADC driver
+ *
+ * Copyright 2024 Analog Devices Inc.
+ */
+#include <asm/unaligned.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/sysfs.h>
+#include <linux/units.h>
+#include <linux/util_macros.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+
+#define AD400X_READ_COMMAND    0x54
+#define AD400X_WRITE_COMMAND   0x14
+

drivers/iio/adc/ad4000.c line 35

Quoted 5 lines

drivers/iio/adc/ad4000.c new file +5 -0

+/* AD4000 Configuration Register programmable bits */
+#define AD4000_STATUS          BIT(4) /* Status bits output */
+#define AD4000_SPAN_COMP       BIT(3) /* Input span compression  */
+#define AD4000_HIGHZ           BIT(2) /* High impedance mode  */
+#define AD4000_TURBO           BIT(1) /* Turbo mode */

Usually bits of the same register share a similar prefix, e.g. AD4000_CFG_TURBO, AD4000_CFG_HIGHZ, etc.

Quoted 39 lines

drivers/iio/adc/ad4000.c new file +38 -0

+
+#define AD4000_TQUIET2_NS              60
+
+#define AD4000_18BIT_MSK       GENMASK(31, 14)
+#define AD4000_20BIT_MSK       GENMASK(31, 12)
+
+#define AD4000_DIFF_CHANNEL(_sign, _real_bits)                         \
+       {                                                               \
+               .type = IIO_VOLTAGE,                                    \
+               .indexed = 1,                                           \
+               .differential = 1,                                      \
+               .channel = 0,                                           \
+               .channel2 = 1,                                          \
+               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |          \
+                                     BIT(IIO_CHAN_INFO_SCALE),         \
+               .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),\
+               .scan_type = {                                          \
+                       .sign = _sign,                                  \
+                       .realbits = _real_bits,                         \
+                       .storagebits = _real_bits > 16 ? 32 : 16,       \
+                       .shift = _real_bits > 16 ? 32 - _real_bits : 0, \
+                       .endianness = IIO_BE,                           \
+               },                                                      \
+       }                                                               \
+
+#define AD4000_PSEUDO_DIFF_CHANNEL(_sign, _real_bits)                  \
+       {                                                               \
+               .type = IIO_VOLTAGE,                                    \
+               .indexed = 1,                                           \
+               .channel = 0,                                           \
+               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |          \
+                                     BIT(IIO_CHAN_INFO_SCALE) |        \
+                                     BIT(IIO_CHAN_INFO_OFFSET),        \
+               .info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),\
+               .scan_type = {                                          \
+                       .sign = _sign,                                  \
+                       .realbits = _real_bits,                         \
+                       .storagebits = _real_bits > 16 ? 32 : 16,       \

drivers/iio/adc/ad4000.c line 59

Quoted 4 lines

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

+                       .shift = _real_bits > 16 ? 32 - _real_bits : 0, \
+                       .endianness = IIO_BE,                           \
+               },                                                      \
+       }                                                               \

It looks like all differential chips are signed and all pseduo-differential chips are unsigned, so I don't think we need the _sign parameter in these macros.

I also still have doubts about using IIO_BE and 8-bit xfers when it comes to adding support later to achieve max sample rate with a SPI offload. For example to get 2MSPS with an 18-bit chip, it will require an approx 33% faster SPI clock than the actual slowest clock possible because it will have to read 6 extra bits per sample. I didn't check the specs, but this may not even be physically possible without exceeding the datasheet max SPI clock rate. Also errors could be reduced if we could actually use the slowest allowable SPI clock rate. Furthermore, the offload hardware would have to be capable of adding an extra byte per sample for 18 and 20-bit chips when piping the data to DMA in order to get the 32-bit alignment in the buffer required by IIO scan_type and the natural alignment requirements of IIO buffers in general.

Quoted 91 lines

drivers/iio/adc/ad4000.c new file +90 -0

+
+enum ad4000_ids {
+       ID_AD4000,
+       ID_AD4001,
+       ID_AD4002,
+       ID_AD4003,
+       ID_AD4004,
+       ID_AD4005,
+       ID_AD4006,
+       ID_AD4007,
+       ID_AD4008,
+       ID_AD4010,
+       ID_AD4011,
+       ID_AD4020,
+       ID_AD4021,
+       ID_AD4022,
+       ID_ADAQ4001,
+       ID_ADAQ4003,
+};
+
+struct ad4000_chip_info {
+       const char *dev_name;
+       struct iio_chan_spec chan_spec;
+};
+
+static const struct ad4000_chip_info ad4000_chips[] = {
+       [ID_AD4000] = {
+               .dev_name = "ad4000",
+               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
+       },
+       [ID_AD4001] = {
+               .dev_name = "ad4001",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
+       },
+       [ID_AD4002] = {
+               .dev_name = "ad4002",
+               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
+       },
+       [ID_AD4003] = {
+               .dev_name = "ad4003",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
+       },
+       [ID_AD4004] = {
+               .dev_name = "ad4004",
+               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
+       },
+       [ID_AD4005] = {
+               .dev_name = "ad4005",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
+       },
+       [ID_AD4006] = {
+               .dev_name = "ad4006",
+               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
+       },
+       [ID_AD4007] = {
+               .dev_name = "ad4007",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
+       },
+       [ID_AD4008] = {
+               .dev_name = "ad4008",
+               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 16),
+       },
+       [ID_AD4010] = {
+               .dev_name = "ad4010",
+               .chan_spec = AD4000_PSEUDO_DIFF_CHANNEL('u', 18),
+       },
+       [ID_AD4011] = {
+               .dev_name = "ad4011",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
+       },
+       [ID_AD4020] = {
+               .dev_name = "ad4020",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
+       },
+       [ID_AD4021] = {
+               .dev_name = "ad4021",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
+       },
+       [ID_AD4022] = {
+               .dev_name = "ad4022",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 20),
+       },
+       [ID_ADAQ4001] = {
+               .dev_name = "adaq4001",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 16),
+       },
+       [ID_ADAQ4003] = {
+               .dev_name = "adaq4003",
+               .chan_spec = AD4000_DIFF_CHANNEL('s', 18),
+       },

drivers/iio/adc/ad4000.c line 173

Quoted 6 lines

drivers/iio/adc/ad4000.c new file +6 -0

+};
+
+enum ad4000_gains {
+       AD4000_0454_GAIN = 0,
+       AD4000_0909_GAIN = 1,
+       AD4000_1_GAIN = 2,

AD4000_1000_GAIN would be more consistent with the others.

Quoted 12 lines

drivers/iio/adc/ad4000.c new file +11 -0

+       AD4000_1900_GAIN = 3,
+       AD4000_GAIN_LEN
+};
+
+/*
+ * Gains stored and computed as fractions to avoid introducing rounding errors.
+ */
+static const int ad4000_gains_frac[AD4000_GAIN_LEN][2] = {
+       [AD4000_0454_GAIN] = { 227, 500 },
+       [AD4000_0909_GAIN] = { 909, 1000 },
+       [AD4000_1_GAIN] = { 1, 1 },

drivers/iio/adc/ad4000.c line 186

Quoted 2 lines

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

+       [AD4000_1900_GAIN] = { 19, 10 },
+};

Why not just store the numerator in milli units and always use 1000 for the denominator? It seems like it would simplify the code and make it easier to read and understand. Also, these values are coming from the adi,gain-milli property already, so we could avoid the enum and the lookup table entirely and simplify things even more.

Quoted 17 lines

drivers/iio/adc/ad4000.c new file +16 -0

+
+struct ad4000_state {
+       struct spi_device *spi;
+       struct gpio_desc *cnv_gpio;
+       int vref;
+       bool status_bits;
+       bool span_comp;
+       bool turbo_mode;
+       bool high_z_mode;
+
+       enum ad4000_gains pin_gain;
+       int scale_tbl[AD4000_GAIN_LEN][2][2];
+
+       /*
+        * DMA (thus cache coherency maintenance) requires the
+        * transfer buffers to live in their own cache lines.

drivers/iio/adc/ad4000.c line 207

Quoted 5 lines

drivers/iio/adc/ad4000.c new file +5 -0

+        */
+       struct {
+               union {
+                       u16 sample_buf16;
+                       u32 sample_buf32;

Technically, these are holding big-endian data, so __be16 and __be32 would be more correct.

drivers/iio/adc/ad4000.c line 213

Quoted 7 lines

drivers/iio/adc/ad4000.c new file +6 -0

+               } data;
+               s64 timestamp __aligned(8);
+       } scan;
+       __be16 tx_buf __aligned(IIO_DMA_MINALIGN);
+       __be16 rx_buf;
+};

scan.data is used as SPI rx_buf so __aligned(IIO_DMA_MINALIGN); needs to be moved to the scan field.

drivers/iio/adc/ad4000.c line 223

Quoted 11 lines

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

+
+static void ad4000_fill_scale_tbl(struct ad4000_state *st, int scale_bits,
+                                 const struct ad4000_chip_info *chip)
+{
+       int diff = chip->chan_spec.differential;
+       int val, val2, tmp0, tmp1, i;
+       u64 tmp2;
+
+       val2 = scale_bits;
+       for (i = 0; i < AD4000_GAIN_LEN; i++) {

Only one gain is selected by the devicetree, so why do we need to do this for all 4 gains?

Quoted 43 lines

drivers/iio/adc/ad4000.c new file +43 -0

+               val = st->vref / 1000;
+               /* Multiply by MILLI here to avoid losing precision */
+               val = mult_frac(val, ad4000_gains_frac[i][1] * MILLI,
+                               ad4000_gains_frac[i][0]);
+               /* Would multiply by NANO here but we already multiplied by MILLI */
+               tmp2 = shift_right((u64)val * MICRO, val2);
+               tmp0 = (int)div_s64_rem(tmp2, NANO, &tmp1);
+               /* Store scale for when span compression is disabled */
+               st->scale_tbl[i][0][0] = tmp0; /* Integer part */
+               st->scale_tbl[i][0][1] = abs(tmp1); /* Fractional part */
+               /* Store scale for when span compression is enabled */
+               st->scale_tbl[i][1][0] = tmp0;
+               if (diff)
+                       st->scale_tbl[i][1][1] = DIV_ROUND_CLOSEST(abs(tmp1) * 4, 5);
+               else
+                       st->scale_tbl[i][1][1] = DIV_ROUND_CLOSEST(abs(tmp1) * 9, 10);
+       }
+}
+
+static int ad4000_write_reg(struct ad4000_state *st, uint8_t val)
+{
+       put_unaligned_be16(AD400X_WRITE_COMMAND << BITS_PER_BYTE | val,
+                          &st->tx_buf);
+       return spi_write(st->spi, &st->tx_buf, 2);
+}
+
+static int ad4000_read_reg(struct ad4000_state *st, unsigned int *val)
+{
+       struct spi_transfer t[] = {
+               {
+                       .tx_buf = &st->tx_buf,
+                       .rx_buf = &st->rx_buf,
+                       .len = 2,
+               },
+       };
+       int ret;
+
+       put_unaligned_be16(AD400X_READ_COMMAND << BITS_PER_BYTE, &st->tx_buf);
+       ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
+       if (ret < 0)
+               return ret;
+
+       *val = get_unaligned_be16(&st->rx_buf);

drivers/iio/adc/ad4000.c line 270

Quoted 4 lines

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

+
+       return ret;
+}
+

It would be very helpful to have comments here explaining the exact expected wiring configuration and signal timing here since there are so many possibilities for this chip.

drivers/iio/adc/ad4000.c line 274

Quoted 4 lines

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

+static int ad4000_read_sample(struct ad4000_state *st,
+                             const struct iio_chan_spec *chan)
+{
+       struct spi_transfer t[] = {

Don't really need [] here since there is only one xfer.

Quoted 17 lines

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

+               {
+                       .rx_buf = &st->scan.data,
+                       .len = BITS_TO_BYTES(chan->scan_type.storagebits),
+                       .delay = {
+                               .value = AD4000_TQUIET2_NS,
+                               .unit = SPI_DELAY_UNIT_NSECS,
+                       },
+               },
+       };
+       int ret;
+
+       ret = spi_sync_transfer(st->spi, t, ARRAY_SIZE(t));
+       if (ret < 0)
+               return ret;
+
+       return 0;
+}

drivers/iio/adc/ad4000.c line 301

Quoted 10 lines

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

+
+static int ad4000_single_conversion(struct iio_dev *indio_dev,
+                                   const struct iio_chan_spec *chan, int *val)
+{
+       struct ad4000_state *st = iio_priv(indio_dev);
+       u32 sample;
+       int ret;
+
+       if (st->cnv_gpio)
+               gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_HIGH);

It would make more sense and be less redundant to move the gpio code into ad4000_read_sample().

Also, gpiod_set_value_cansleep() checks for NULL, so the if () is redundant.

Quoted 7 lines

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

+
+       ret = ad4000_read_sample(st, chan);
+       if (ret)
+               return ret;
+
+       if (st->cnv_gpio)
+               gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_LOW);

drivers/iio/adc/ad4000.c line 313

Quoted 5 lines

drivers/iio/adc/ad4000.c new file +5 -0

+
+       if (chan->scan_type.storagebits > 16)
+               sample = get_unaligned_be32(&st->scan.data);
+       else
+               sample = get_unaligned_be16(&st->scan.data);

data is aligned, so be32/16_to_cpu() should be fine. Also, Jonathan will suggest to use &st->scan.data.sample_b32/16 here too. :-)

Quoted 71 lines

drivers/iio/adc/ad4000.c new file +70 -0

+
+       switch (chan->scan_type.realbits) {
+       case 16:
+               break;
+       case 18:
+               sample = FIELD_GET(AD4000_18BIT_MSK, sample);
+               break;
+       case 20:
+               sample = FIELD_GET(AD4000_20BIT_MSK, sample);
+               break;
+       default:
+               return -EINVAL;
+       }
+
+       if (chan->scan_type.sign == 's')
+               *val = sign_extend32(sample, chan->scan_type.realbits - 1);
+
+       return IIO_VAL_INT;
+}
+
+static int ad4000_read_raw(struct iio_dev *indio_dev,
+                          struct iio_chan_spec const *chan, int *val,
+                          int *val2, long info)
+{
+       struct ad4000_state *st = iio_priv(indio_dev);
+
+       switch (info) {
+       case IIO_CHAN_INFO_RAW:
+               iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
+                       return ad4000_single_conversion(indio_dev, chan, val);
+               unreachable();
+       case IIO_CHAN_INFO_SCALE:
+               *val = st->scale_tbl[st->pin_gain][st->span_comp][0];
+               *val2 = st->scale_tbl[st->pin_gain][st->span_comp][1];
+               return IIO_VAL_INT_PLUS_NANO;
+       case IIO_CHAN_INFO_OFFSET:
+               *val = 0;
+               if (st->span_comp)
+                       *val = mult_frac(st->vref / 1000, 1, 10);
+
+               return IIO_VAL_INT;
+       default:
+               break;
+       }
+
+       return -EINVAL;
+}
+
+static int ad4000_read_avail(struct iio_dev *indio_dev,
+                            struct iio_chan_spec const *chan,
+                            const int **vals, int *type, int *length,
+                            long info)
+{
+       struct ad4000_state *st = iio_priv(indio_dev);
+
+       switch (info) {
+       case IIO_CHAN_INFO_SCALE:
+               *vals = (int *)st->scale_tbl[st->pin_gain];
+               *length = 2 * 2;
+               *type = IIO_VAL_INT_PLUS_NANO;
+               return IIO_AVAIL_LIST;
+       default:
+               return -EINVAL;
+       }
+}
+
+static int ad4000_write_raw_get_fmt(struct iio_dev *indio_dev,
+                                   struct iio_chan_spec const *chan, long mask)
+{
+       switch (mask) {

drivers/iio/adc/ad4000.c line 389

Quoted 6 lines

drivers/iio/adc/ad4000.c new file +6 -0

+       case IIO_CHAN_INFO_SCALE:
+               return IIO_VAL_INT_PLUS_NANO;
+       default:
+               return IIO_VAL_INT_PLUS_MICRO;
+       }
+       return -EINVAL;

not reachable because of default, so can be left out

Quoted 24 lines

drivers/iio/adc/ad4000.c new file +24 -0

+}
+
+static int ad4000_write_raw(struct iio_dev *indio_dev,
+                           struct iio_chan_spec const *chan, int val, int val2,
+                           long mask)
+{
+       struct ad4000_state *st = iio_priv(indio_dev);
+       unsigned int reg_val;
+       bool span_comp_en;
+       int ret;
+
+       switch (mask) {
+       case IIO_CHAN_INFO_SCALE:
+               iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
+                       ret = ad4000_read_reg(st, &reg_val);
+                       if (ret < 0)
+                               return ret;
+
+                       span_comp_en = (val2 == st->scale_tbl[st->pin_gain][1][1]);
+                       reg_val &= ~AD4000_SPAN_COMP;
+                       reg_val |= FIELD_PREP(AD4000_SPAN_COMP, span_comp_en);
+
+                       ret = ad4000_write_reg(st, reg_val);
+                       if (ret < 0)

drivers/iio/adc/ad4000.c line 419

Quoted 6 lines

drivers/iio/adc/ad4000.c new file +6 -0

+                               return ret;
+
+                       st->span_comp = span_comp_en;
+                       return 0;
+               }
+               unreachable();

Can bring out the return 0 to avoid unreachable.

drivers/iio/adc/ad4000.c line 421

Quoted 3 lines

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

+       default:
+               break;

Can return -EINVAL to avoid break;

Quoted 39 lines

drivers/iio/adc/ad4000.c new file +39 -0

+       }
+
+       return -EINVAL;
+}
+
+static irqreturn_t ad4000_trigger_handler(int irq, void *p)
+{
+       struct iio_poll_func *pf = p;
+       struct iio_dev *indio_dev = pf->indio_dev;
+       struct ad4000_state *st = iio_priv(indio_dev);
+       int ret;
+
+       if (st->cnv_gpio)
+               gpiod_set_value(st->cnv_gpio, GPIOD_OUT_HIGH);
+
+       ret = ad4000_read_sample(st, &indio_dev->channels[0]);
+       if (ret < 0)
+               goto err_out;
+
+       if (st->cnv_gpio)
+               gpiod_set_value(st->cnv_gpio, GPIOD_OUT_LOW);
+
+       iio_push_to_buffers_with_timestamp(indio_dev, &st->scan,
+                                          iio_get_time_ns(indio_dev));
+
+err_out:
+       iio_trigger_notify_done(indio_dev->trig);
+       return IRQ_HANDLED;
+}
+
+static const struct iio_info ad4000_info = {
+       .read_raw = &ad4000_read_raw,
+       .read_avail = &ad4000_read_avail,
+       .write_raw = &ad4000_write_raw,
+       .write_raw_get_fmt = &ad4000_write_raw_get_fmt,
+};
+
+static void ad4000_config(struct ad4000_state *st)
+{

drivers/iio/adc/ad4000.c line 464

Quoted 4 lines

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

+       unsigned int reg_val;
+       int ret;
+
+       reg_val = FIELD_PREP(AD4000_TURBO, 1);

Since the driver in it's current state can get anywhere near the max sample rate of ~1MSPS, I don't think it makes sense to enable turbo at this point.

Quoted 10 lines

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

+
+       if (device_property_present(&st->spi->dev, "adi,high-z-input"))
+               reg_val |= FIELD_PREP(AD4000_HIGHZ, 1);
+
+       /*
+        * The ADC SDI pin might be connected to controller CS line in which
+        * case the write might fail. This, however, does not prevent the device
+        * from functioning even though in a configuration other than the
+        * requested one.
+        */

drivers/iio/adc/ad4000.c line 477

Quoted 3 lines

drivers/iio/adc/ad4000.c new file +3 -0

+       ret = ad4000_write_reg(st, reg_val);
+       if (ret < 0)
+               dev_dbg(&st->spi->dev, "Failed to config device\n");

If writing fails because there is no CS line wired up, we won't get an error returned here. The SPI controller has no way of knowing this happened, so it can only assume the write was successful and return 0. So this should return ret.

Ideally, the devicetree should tell us if CS is wired up or not.

Quoted 10 lines

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

+}
+
+static void ad4000_regulator_disable(void *reg)
+{
+       regulator_disable(reg);
+}
+
+static int ad4000_probe(struct spi_device *spi)
+{
+       const struct ad4000_chip_info *chip;

drivers/iio/adc/ad4000.c line 491

Quoted 4 lines

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

+       struct regulator *vref_reg;
+       struct iio_dev *indio_dev;
+       struct ad4000_state *st;
+       int ret;

We need a check somewhere in here to make sure that adi,spi-mode is in a supported configuration. E.g. chain mode is not currently implemented.

Quoted 38 lines

drivers/iio/adc/ad4000.c new file +37 -0

+
+       indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+       if (!indio_dev)
+               return -ENOMEM;
+
+       chip = spi_get_device_match_data(spi);
+       if (!chip)
+               return -EINVAL;
+
+       st = iio_priv(indio_dev);
+       st->spi = spi;
+
+       ret = devm_regulator_get_enable(&spi->dev, "vdd");
+       if (ret)
+               return dev_err_probe(&spi->dev, ret, "Failed to enable VDD supply\n");
+
+       ret = devm_regulator_get_enable(&spi->dev, "vio");
+       if (ret)
+               return dev_err_probe(&spi->dev, ret, "Failed to enable VIO supply\n");
+
+       vref_reg = devm_regulator_get(&spi->dev, "ref");
+       if (IS_ERR(vref_reg))
+               return dev_err_probe(&spi->dev, PTR_ERR(vref_reg),
+                                    "Failed to get vref regulator\n");
+
+       ret = regulator_enable(vref_reg);
+       if (ret < 0)
+               return dev_err_probe(&spi->dev, ret,
+                                    "Failed to enable voltage regulator\n");
+
+       ret = devm_add_action_or_reset(&spi->dev, ad4000_regulator_disable, vref_reg);
+       if (ret)
+               return dev_err_probe(&spi->dev, ret,
+                                    "Failed to add regulator disable action\n");
+
+       st->vref = regulator_get_voltage(vref_reg);
+       if (st->vref < 0)

drivers/iio/adc/ad4000.c line 534

Quoted 6 lines

drivers/iio/adc/ad4000.c new file +6 -0

+               return dev_err_probe(&spi->dev, st->vref, "Failed to get vref\n");
+
+       st->cnv_gpio = devm_gpiod_get_optional(&spi->dev, "cnv", GPIOD_OUT_HIGH);
+       if (IS_ERR(st->cnv_gpio)) {
+               if (PTR_ERR(st->cnv_gpio) == -EPROBE_DEFER)
+                       return -EPROBE_DEFER;

EPROBE_DEFER check is not needed with dev_err_probe();, it already does that.

Quoted 11 lines

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

+
+               return dev_err_probe(&spi->dev, PTR_ERR(st->cnv_gpio),
+                                    "Failed to get CNV GPIO");
+       }
+
+       ad4000_config(st);
+
+       indio_dev->name = chip->dev_name;
+       indio_dev->info = &ad4000_info;

drivers/iio/adc/ad4000.c line 549

Quoted 6 lines

drivers/iio/adc/ad4000.c new file +6 -0

+       indio_dev->channels = &chip->chan_spec;
+       indio_dev->num_channels = 1;
+
+       st->pin_gain = AD4000_1_GAIN;
+       if (device_property_present(&spi->dev, "adi,gain-milli")) {
+               u32 val;

Should it be an error if adi,gain-milli is set on non-adaq chips?

Quoted 16 lines

drivers/iio/adc/ad4000.c new file +15 -0

+
+               ret = device_property_read_u32(&spi->dev, "adi,gain-milli", &val);
+               if (ret)
+                       return ret;
+
+               switch (val) {
+               case 454:
+                       st->pin_gain = AD4000_0454_GAIN;
+                       break;
+               case 909:
+                       st->pin_gain = AD4000_0909_GAIN;
+                       break;
+               case 1000:
+                       st->pin_gain = AD4000_1_GAIN;
+                       break;

drivers/iio/adc/ad4000.c line 570

Quoted 6 lines

drivers/iio/adc/ad4000.c new file +6 -0

+               case 1900:
+                       st->pin_gain = AD4000_1900_GAIN;
+                       break;
+               default:
+                       return dev_err_probe(&spi->dev, -EINVAL,
+                                            "Invalid firmware provided gain\n");

Could help debugging if val is included in the error message.

Quoted 85 lines

drivers/iio/adc/ad4000.c new file +79 -1

+               }
+       }
+
+       /*
+        * ADCs that output twos complement code have one less bit to express
+        * voltage magnitude.
+        */
+       if (chip->chan_spec.scan_type.sign == 's')
+               ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits - 1,
+                                     chip);
+       else
+               ad4000_fill_scale_tbl(st, chip->chan_spec.scan_type.realbits,
+                                     chip);
+
+       ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
+                                             &iio_pollfunc_store_time,
+                                             &ad4000_trigger_handler, NULL);
+       if (ret)
+               return ret;
+
+       return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct spi_device_id ad4000_id[] = {
+       { "ad4000", (kernel_ulong_t)&ad4000_chips[ID_AD4000] },
+       { "ad4001", (kernel_ulong_t)&ad4000_chips[ID_AD4001] },
+       { "ad4002", (kernel_ulong_t)&ad4000_chips[ID_AD4002] },
+       { "ad4003", (kernel_ulong_t)&ad4000_chips[ID_AD4003] },
+       { "ad4004", (kernel_ulong_t)&ad4000_chips[ID_AD4004] },
+       { "ad4005", (kernel_ulong_t)&ad4000_chips[ID_AD4005] },
+       { "ad4006", (kernel_ulong_t)&ad4000_chips[ID_AD4006] },
+       { "ad4007", (kernel_ulong_t)&ad4000_chips[ID_AD4007] },
+       { "ad4008", (kernel_ulong_t)&ad4000_chips[ID_AD4008] },
+       { "ad4010", (kernel_ulong_t)&ad4000_chips[ID_AD4010] },
+       { "ad4011", (kernel_ulong_t)&ad4000_chips[ID_AD4011] },
+       { "ad4020", (kernel_ulong_t)&ad4000_chips[ID_AD4020] },
+       { "ad4021", (kernel_ulong_t)&ad4000_chips[ID_AD4021] },
+       { "ad4022", (kernel_ulong_t)&ad4000_chips[ID_AD4022] },
+       { "adaq4001", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4001] },
+       { "adaq4003", (kernel_ulong_t)&ad4000_chips[ID_ADAQ4003] },
+       { }
+};
+MODULE_DEVICE_TABLE(spi, ad4000_id);
+
+static const struct of_device_id ad4000_of_match[] = {
+       { .compatible = "adi,ad4000", .data = &ad4000_chips[ID_AD4000] },
+       { .compatible = "adi,ad4001", .data = &ad4000_chips[ID_AD4001] },
+       { .compatible = "adi,ad4002", .data = &ad4000_chips[ID_AD4002] },
+       { .compatible = "adi,ad4003", .data = &ad4000_chips[ID_AD4003] },
+       { .compatible = "adi,ad4004", .data = &ad4000_chips[ID_AD4004] },
+       { .compatible = "adi,ad4005", .data = &ad4000_chips[ID_AD4005] },
+       { .compatible = "adi,ad4006", .data = &ad4000_chips[ID_AD4006] },
+       { .compatible = "adi,ad4007", .data = &ad4000_chips[ID_AD4007] },
+       { .compatible = "adi,ad4008", .data = &ad4000_chips[ID_AD4008] },
+       { .compatible = "adi,ad4010", .data = &ad4000_chips[ID_AD4010] },
+       { .compatible = "adi,ad4011", .data = &ad4000_chips[ID_AD4011] },
+       { .compatible = "adi,ad4020", .data = &ad4000_chips[ID_AD4020] },
+       { .compatible = "adi,ad4021", .data = &ad4000_chips[ID_AD4021] },
+       { .compatible = "adi,ad4022", .data = &ad4000_chips[ID_AD4022] },
+       { .compatible = "adi,adaq4001", .data = &ad4000_chips[ID_ADAQ4001] },
+       { .compatible = "adi,adaq4003", .data = &ad4000_chips[ID_ADAQ4003] },
+       { }
+};
+MODULE_DEVICE_TABLE(of, ad4000_of_match);
+
+static struct spi_driver ad4000_driver = {
+       .driver = {
+               .name   = "ad4000",
+               .of_match_table = ad4000_of_match,
+       },
+       .probe          = ad4000_probe,
+       .id_table       = ad4000_id,
+};
+module_spi_driver(ad4000_driver);
+
+MODULE_AUTHOR("Mircea Caprioru <[email protected]>");
+MODULE_AUTHOR("Marcelo Schmitt <[email protected]>");
+MODULE_DESCRIPTION("Analog Devices AD4000 ADC driver");
+MODULE_LICENSE("GPL");
--

2.43.0

Quoted 3 lines · David Lechner

Quoted 1 line · Marcelo Schmitt

Quoted 14 lines

I also still have doubts about using IIO_BE and 8-bit xfers when it comes to adding support later to achieve max sample rate with a SPI offload. For example to get 2MSPS with an 18-bit chip, it will require an approx 33% faster SPI clock than the actual slowest clock possible because it will have to read 6 extra bits per sample. I didn't check the specs, but this may not even be physically possible without exceeding the datasheet max SPI clock rate. Also errors could be reduced if we could actually use the slowest allowable SPI clock rate. Furthermore, the offload hardware would have to be capable of adding an extra byte per sample for 18 and 20-bit chips when piping the data to DMA in order to get the 32-bit alignment in the buffer required by IIO scan_type and the natural alignment requirements of IIO buffers in general.

Maybe I should already implement support for reading with SPI offload rather than doing it after this set is merged?
So we can test what happens at faster sample rates before we commit to a solution.

Quoted 9 lines

Quoted 8 lines

drivers/iio/adc/ad4000.c new file +6 -0

+               } data;
+               s64 timestamp __aligned(8);
+       } scan;
+       __be16 tx_buf __aligned(IIO_DMA_MINALIGN);
+       __be16 rx_buf;
+};

Quoted 2 lines

scan.data is used as SPI rx_buf so __aligned(IIO_DMA_MINALIGN); needs to be moved to the scan field.

I have already tried it. Maybe I did something wrong besides buffer alignment at that time. Will give it another try.

Quoted 11 lines

drivers/iio/adc/ad4000.c line 464

Quoted 6 lines

drivers/iio/adc/ad4000.c new file +6 -0

+static void ad4000_config(struct ad4000_state *st)
+{
+       unsigned int reg_val;
+       int ret;
+
+       reg_val = FIELD_PREP(AD4000_TURBO, 1);

Since the driver in it's current state can get anywhere near the max sample rate of ~1MSPS, I don't think it makes sense to enable turbo at this point.

This is just enabling turbo at start up. If not enabling turbo during probe, we would want(need?) to provide some interface for that, which might not be much desired.

Quoted 6 lines

drivers/iio/adc/ad4000.c line 549

Quoted 4 lines

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

+
+       st->pin_gain = AD4000_1_GAIN;
+       if (device_property_present(&spi->dev, "adi,gain-milli")) {
+               u32 val;

Should it be an error if adi,gain-milli is set on non-adaq chips?

Maybe. We should not change the scale if it's a chip that don't have the amplifier in front of the ADC. I think the best handling would be to just ignore adi,gain-milli if it's not an ADAQ device. Maybe better add a DT constraint,
- if:
properties:
compatible:
contains:
enum:
- adi,adaq4001
- adi,adaq4003
then:
properties:
adi,gain-milli: false
?

Quoted 5 lines · Marcelo Schmitt

Quoted 3 lines · David Lechner

Quoted 1 line · Marcelo Schmitt

Quoted 14 lines

Quoted 13 lines

drivers/iio/adc/ad4000.c line 464

Quoted 8 lines

drivers/iio/adc/ad4000.c new file +6 -0

+static void ad4000_config(struct ad4000_state *st)
+{
+       unsigned int reg_val;
+       int ret;
+
+       reg_val = FIELD_PREP(AD4000_TURBO, 1);

Since the driver in it's current state can get anywhere near the max sample rate of ~1MSPS, I don't think it makes sense to enable turbo at this point.

Quoted 4 lines

This is just enabling turbo at start up. If not enabling turbo during probe, we would want(need?) to provide some interface for that, which might not be much desired.

TURBO is only needed to achieve the max sample rate of 500k/1M/2MSPS on the various chips by skipping powering down some circuitry between samples. We can't get anywhere close to that in Linux without some sort of SPI offloading. So, for now, we might as well leave it disabled and save some power.

Quoted 8 lines

Quoted 7 lines

drivers/iio/adc/ad4000.c line 549

Quoted 5 lines

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

+
+       st->pin_gain = AD4000_1_GAIN;
+       if (device_property_present(&spi->dev, "adi,gain-milli")) {
+               u32 val;

Should it be an error if adi,gain-milli is set on non-adaq chips?

Quoted 15 lines

Maybe. We should not change the scale if it's a chip that don't have the amplifier in front of the ADC. I think the best handling would be to just ignore adi,gain-milli if it's not an ADAQ device. Maybe better add a DT constraint,
- if:
properties:
compatible:
contains:
enum:
- adi,adaq4001
- adi,adaq4003
then:
properties:
adi,gain-milli: false
?

I think this is missing a not:, but otherwise yes this should be in the DT bindings.

Even with that though, I would still be helpful to readers of the driver to at least have a comment here pointing out that this property and related gain scaling only applies to ADAQ chips.

Quoted 5 lines · Marcelo Schmitt

Quoted 3 lines · David Lechner

Quoted 1 line · Marcelo Schmitt

...

Quoted 15 lines

Quoted 14 lines

I also still have doubts about using IIO_BE and 8-bit xfers when it comes to adding support later to achieve max sample rate with a SPI offload. For example to get 2MSPS with an 18-bit chip, it will require an approx 33% faster SPI clock than the actual slowest clock possible because it will have to read 6 extra bits per sample. I didn't check the specs, but this may not even be physically possible without exceeding the datasheet max SPI clock rate. Also errors could be reduced if we could actually use the slowest allowable SPI clock rate. Furthermore, the offload hardware would have to be capable of adding an extra byte per sample for 18 and 20-bit chips when piping the data to DMA in order to get the 32-bit alignment in the buffer required by IIO scan_type and the natural alignment requirements of IIO buffers in general.

Quoted 4 lines

Maybe I should already implement support for reading with SPI offload rather than doing it after this set is merged?
So we can test what happens at faster sample rates before we commit to a solution.

Yes, that sounds like a wise thing to do.

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

drivers/iio/adc/ad4000.c new file +6 -0

+               } data;
+               s64 timestamp __aligned(8);
+       } scan;
+       __be16 tx_buf __aligned(IIO_DMA_MINALIGN);
+       __be16 rx_buf;
+};  

Quoted 5 lines

Quoted 2 lines

scan.data is used as SPI rx_buf so __aligned(IIO_DMA_MINALIGN); needs to be moved to the scan field.

I have already tried it. Maybe I did something wrong besides buffer alignment at that time. Will give it another try.

This is the alignment for DMA cache coherency. So it should not have any affect on the actual data read, only performance.

Quoted 4 lines

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

+
+       ret = ad4000_read_sample(st, chan);

drivers/iio/adc/ad4000.c line 313

Quoted 10 lines

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

+       if (ret)
+               return ret;
+
+       if (st->cnv_gpio)
+               gpiod_set_value_cansleep(st->cnv_gpio, GPIOD_OUT_LOW);
+
+       if (chan->scan_type.storagebits > 16)
+               sample = get_unaligned_be32(&st->scan.data);
+       else
+               sample = get_unaligned_be16(&st->scan.data);  

data is aligned, so be32/16_to_cpu() should be fine. Also, Jonathan will suggest to use &st->scan.data.sample_b32/16 here too. :-)

Add support for AD4000 family of low noise, low power, high speed, successive aproximation register (SAR) ADCs.

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

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

+#include <asm/unaligned.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/math.h>

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

+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
 
+
+static int ad4000_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan, int *val,
+			   int *val2, long info)
+{
+	struct ad4000_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_RAW:
+		iio_device_claim_direct_scoped(return -EBUSY, indio_dev)
+			return ad4000_single_conversion(indio_dev, chan, val);
+		unreachable();
+	case IIO_CHAN_INFO_SCALE:
+		*val = st->scale_tbl[st->pin_gain][st->span_comp][0];
+		*val2 = st->scale_tbl[st->pin_gain][st->span_comp][1];
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_OFFSET:

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

+}
+
+static int ad4000_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length,
+			     long info)
+{
+	struct ad4000_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (int *)st->scale_tbl[st->pin_gain];
+		*length = 2 * 2;
+		*type = IIO_VAL_INT_PLUS_NANO;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+

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

+}
+
+static int ad4000_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int val, int val2,
+			    long mask)
+{
+	struct ad4000_state *st = iio_priv(indio_dev);
+	unsigned int reg_val;
+	bool span_comp_en;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
+			ret = ad4000_read_reg(st, &reg_val);
+			if (ret < 0)
+				return ret;
+
+			span_comp_en = (val2 == st->scale_tbl[st->pin_gain][1][1]);
+			reg_val &= ~AD4000_SPAN_COMP;
+			reg_val |= FIELD_PREP(AD4000_SPAN_COMP, span_comp_en);
+
+			ret = ad4000_write_reg(st, reg_val);
+			if (ret < 0)
+				return ret;

+}

This is more like an RFC patch set since configuration read/write is currently buggy.

Change log v1 -> v2:
- Took device tree provided by David.
- Dropped ABI additions in favor of device tree properties.
- Set differential IIO channel subtype for differential ADCs.
- Set scan_type shift bits to mask out correct real bits from buffer.
- Added __aligned(8) to buffer timestamp.
- Used union to reduce buffer memory usage for 16-bit devices.
- Used SPI transfer functions rather than SPI message.
- Used c99 style structure initialization.
- Used iio_device_claim_direct_scoped().
- Removed unneeded pointer casts.
- Added other power supplies (VDD and VIO).

Link to v1: https://lore.kernel.org/linux-iio/[email protected]/

Additional topics:

- Why there is no different handling for the different SPI wiring modes? It looks like there is no need for different handling of "4-wire" and "3-wire" modes.
If in "4-wire" (dt default mode), SDI is connected to SPI controller CS and CNV is active high. We can activate the CNV GPIO then let the SPI controller bring CS (connected to SDI) down when starting the transfer.
If in "3-wire" (dt single mode), if we have a CNV (active low) GPIO we activate it and then proceed with with the transfer. If controller CS is connected to CNV it works the same way.
I'm thinking it's better if we can support these devices in similar way other SPI ADCs are supported. Does that make sense?

To me, the "3-wire" mode with controller CS to ADC CNV is what most resembles conventional SPI. The only important distinction is that the controller must be able to keep ADC SDI line high during conversions. Although, while the spi-engine implementation provided to me can keep SDI up during conversions, I'm not sure its a thing all SPI controllers can do. I tried a raspberry pi 4 some time ago and it was leaving the SDI line low if no tx buffer was provided. Even with a tx full of 1s the controller would bring SDI down between each 8 bits of transfer.

Anyway, single-shot and buffered reads work with the spi-engine controller with ADC in "3-wire"/single mode with controller CS line connected to ADC CNV pin which is how I've been testing it.

- Why did not make vref regulator optional?
Other SAR ADCs I've seen needed a voltage reference otherwise they simply could not provide any reasonable readings. Isn't it preferable to fail rather than having a device that can't provide reliable data?

- Why did not split into AD and ADAQ patches?
The main difference between AD and ADAQ is the amplifier in front of the ADC. If only supporting AD, we could probably avoid the scale table since it would only have two possible values per ADC. But then the handling of span compression scale would need refactoring to be in the scale table when adding ADAQ. I'm not excited to implement something knowing it will need rework in the following patch. Will do if required.

- Span compression and offset.
For non-differential ADCs, enabling the span compression requires an input offset.

Link https://www.analog.com/media/en/technical-documentation/data-sheets/AD4000-4004-4008.pdf

page 18
and

Link https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf

page 19
I updated the _offset attribute for those ADCs according to span compression being enabled or not. Is it okay to have an attribute update cause an update to another one?
Maybe also make the span compression a dt property and have it fixed after probe?

- Configuration register
Despite it doing single-shot and buffered captures, read and writes to the configuration register are currently buggy. It is as if the register was "floating". I tried setting up buffers like ad7768-1, adxl355_core, bma220_spi, bma400_core, and mcp3911.

Thanks,
Marcelo

Marcelo Schmitt (2):
dt-bindings: iio: adc: Add AD4000
iio: adc: Add support for AD4000

 .../bindings/iio/adc/adi,ad4000.yaml          | 201 ++++++
 MAINTAINERS                                   |   8 +
 drivers/iio/adc/Kconfig                       |  12 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ad4000.c                      | 649 ++++++++++++++++++
 5 files changed, 871 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
 create mode 100644 drivers/iio/adc/ad4000.c

--
2.43.0

Quoted 21 lines · Marcelo Schmitt

This is more like an RFC patch set since configuration read/write is currently buggy.

Change log v1 -> v2:
- Took device tree provided by David.
- Dropped ABI additions in favor of device tree properties.
- Set differential IIO channel subtype for differential ADCs.
- Set scan_type shift bits to mask out correct real bits from buffer.
- Added __aligned(8) to buffer timestamp.
- Used union to reduce buffer memory usage for 16-bit devices.
- Used SPI transfer functions rather than SPI message.
- Used c99 style structure initialization.
- Used iio_device_claim_direct_scoped().
- Removed unneeded pointer casts.
- Added other power supplies (VDD and VIO).

Link to v1: https://lore.kernel.org/linux-iio/[email protected]/

Additional topics:

Quoted 11 lines

- Why there is no different handling for the different SPI wiring modes? It looks like there is no need for different handling of "4-wire" and "3-wire" modes.
If in "4-wire" (dt default mode), SDI is connected to SPI controller CS and CNV is active high. We can activate the CNV GPIO then let the SPI controller bring CS (connected to SDI) down when starting the transfer.
If in "3-wire" (dt single mode), if we have a CNV (active low) GPIO we activate it and then proceed with with the transfer. If controller CS is connected to CNV it works the same way.
I'm thinking it's better if we can support these devices in similar way other SPI ADCs are supported. Does that make sense?

In the AD7944 driver, I handled the "3-wire" mode separately because the sample conversion is triggered on the rising edge of the CNV line. In "4-wire" mode, since we have a GPIO connected to CNV, we can just toggle the GPIO from low to high, wait for the conversion time (t_CONV) and then read the sample (SPI xfer) then toggle the CNV line low again. In 3-wire mode, the CS line is connected to the CNV pin, so in order to get an up-to-date sample, we need to to toggle the CS line from low to high to trigger a conversion (spi xfer with no data, only delay), then wait for the conversion time, then read the sample (2nd spi xfer). So in "4-wire" mode, the CS line is getting toggled once per sample, but in "3-wire" mode, it is getting toggled twice per sample. I didn't add support for "3-wire" mode where CNV is connected to GPIO because we can't get max sample rate that way and it is unusual to not have CS connected to something. But if we do that here, the timing has to be different from 4-wire mode in order to not get stale data.

Quoted 9 lines

To me, the "3-wire" mode with controller CS to ADC CNV is what most resembles conventional SPI. The only important distinction is that the controller must be able to keep ADC SDI line high during conversions. Although, while the spi-engine implementation provided to me can keep SDI up during conversions, I'm not sure its a thing all SPI controllers can do. I tried a raspberry pi 4 some time ago and it was leaving the SDI line low if no tx buffer was provided. Even with a tx full of 1s the controller would bring SDI down between each 8 bits of transfer.

This is a good point. It sounds like additional bindings are needed to describe the various wiring cases of the SDI line.

It sounds like possibilities are:

1. SDI is hard-wired high -> can't write to registers, CNV is connected to SPI controller CS, chip is in "3-wire" mode. Currently adi,spi-mode="single"
2. SDI is connected to SDO of another chip, SDI of last chip is hard-wired low -> can't write to registers, CNV is connected to SPI controller CS, chips are in daisy chain mode. Currently adi,spi-mode="chain"
3. SDI is connected to SPI controller CS -> can't write registers, chip can operate in 4-wire mode with CNV connected to GPIO, Currently adi,spi-mode omitted.
4. SDI is connected to SPI controller SDO -> can write registers, and support all writing modes (3-wire, 4-wire, daisy chain) as long as SPI controller SDO line can be kept high or low at the appropriate time. Currently not handled.
5. There could be a pin mux that switches between the one of the first three and the 4th option (needed to avoid the issue with SPI controller not being able to place the SDI pin in the correct state during conversion trigger as described above).

On AD7944, the proposed adi,spi-mode property was sufficient to describe what was wired to the SDI pin because we only had the first 3 options (the AD7944 doesn't have SPI registers to write to).

Also see related comments in my reply to the DT bindings patch.

(From the complete bindings point of view, we should probably also consider the possibility of variations of 1. and 2. where CS of the SPI controller is not wired and CNV is connected to a GPIO - this can be determined by the combination of the adi,spi-mode property and the presence or absence of the cnv-gpios property.)

Quoted 4 lines

Anyway, single-shot and buffered reads work with the spi-engine controller with ADC in "3-wire"/single mode with controller CS line connected to ADC CNV pin which is how I've been testing it.

Technically, yes data can be captured in "3-wire" mode with a single CS toggle, but then the data is stale and doesn't correspond to the soft timestamp because it is reading the data from the previous conversion triggered by the last SPI xfer, whenever that was. Since it is trivial to avoid this by adding the extra CS/CNV toggle I describe above, I don't see any reason not to.

But the way the driver is written now, it is actually only supporting the unnamed wiring option 4 from above, so now I understand the confusion about 3-wire vs. 4-wire mode in that context.

Quoted 5 lines

- Why did not make vref regulator optional?
Other SAR ADCs I've seen needed a voltage reference otherwise they simply could not provide any reasonable readings. Isn't it preferable to fail rather than having a device that can't provide reliable data?

In the device tree bindings, making vref-supply required makes sense since there is no internal reference. In the driver, as discussed in V1, it will fail if vref-supply in regulator_get_voltage() if vref-supply is missing and we use devm_regulator_get() instead of devm_regulator_get_optional(). So leaving it as-is is fine. We have a plan to clean this up later anyway.

Quoted 8 lines

- Why did not split into AD and ADAQ patches?
The main difference between AD and ADAQ is the amplifier in front of the ADC. If only supporting AD, we could probably avoid the scale table since it would only have two possible values per ADC. But then the handling of span compression scale would need refactoring to be in the scale table when adding ADAQ. I'm not excited to implement something knowing it will need rework in the following patch. Will do if required.

If it isn't that much work, it seems worth it to me. If the driver work is too much, maybe just split the DT patch?

Quoted 4 lines

- Span compression and offset.
For non-differential ADCs, enabling the span compression requires an input offset.

Link https://www.analog.com/media/en/technical-documentation/data-sheets/AD4000-4004-4008.pdf

Quoted 8 lines

page 18
and

Link https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf

page 19
I updated the _offset attribute for those ADCs according to span compression being enabled or not. Is it okay to have an attribute update cause an update to another one?
Maybe also make the span compression a dt property and have it fixed after probe?

This doesn't sound like something that belongs in DT since it doesn't depend on the physical properties of what is wired to the input.

But the fact that offset should not be read until after scale is set sounds like a quirk that would be worth documenting in some chip-specific docs.

Quoted 6 lines

- Configuration register
Despite it doing single-shot and buffered captures, read and writes to the configuration register are currently buggy. It is as if the register was "floating". I tried setting up buffers like ad7768-1, adxl355_core, bma220_spi, bma400_core, and mcp3911.

If the ADC CNV pin is connected to a GPIO and the ADC SDI pin is connected to SDO of the SPI controller, then nothing is connected to CS of the SPI controller, so that might be the problem.

Quoted 23 lines

Thanks,
Marcelo

Marcelo Schmitt (2):
dt-bindings: iio: adc: Add AD4000
iio: adc: Add support for AD4000

 .../bindings/iio/adc/adi,ad4000.yaml          | 201 ++++++
 MAINTAINERS                                   |   8 +
 drivers/iio/adc/Kconfig                       |  12 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/ad4000.c                      | 649 ++++++++++++++++++
 5 files changed, 871 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
 create mode 100644 drivers/iio/adc/ad4000.c

--
2.43.0

Quoted 3 lines · David Lechner

Quoted 1 line · Marcelo Schmitt

Quoted 6 lines

Quoted 5 lines

- Why did not make vref regulator optional?
Other SAR ADCs I've seen needed a voltage reference otherwise they simply could not provide any reasonable readings. Isn't it preferable to fail rather than having a device that can't provide reliable data?

Quoted 7 lines

In the device tree bindings, making vref-supply required makes sense since there is no internal reference. In the driver, as discussed in V1, it will fail if vref-supply in regulator_get_voltage() if vref-supply is missing and we use devm_regulator_get() instead of devm_regulator_get_optional(). So leaving it as-is is fine. We have a plan to clean this up later anyway.

Not sure I understand the idea here. Should the driver use devm_regulator_get_optional() instead of devm_regulator_get() because the optional call would fail immediately if no vref-supply while the regular call would only fail at regulator_get_voltage()? Why? This looks very counter intuitive to me.

Quoted 4 lines

- Span compression and offset.
For non-differential ADCs, enabling the span compression requires an input offset.

Quoted 8 lines

Link https://www.analog.com/media/en/technical-documentation/data-sheets/AD4000-4004-4008.pdf

page 18
and

Link https://www.analog.com/media/en/technical-documentation/data-sheets/ad4002-4006-4010.pdf

page 19
I updated the _offset attribute for those ADCs according to span compression being enabled or not. Is it okay to have an attribute update cause an update to another one?

Quoted 1 line

Maybe also make the span compression a dt property and have it fixed after probe?

This doesn't sound like something that belongs in DT since it doesn't depend on the physical properties of what is wired to the input.

But the fact that offset should not be read until after scale is set sounds like a quirk that would be worth documenting in some chip-specific docs.