Re: [PATCH v3 1/4] dt-bindings: iio: adc: add bindings for AD4691 family

[David Lechner]

On 3/13/26 5:07 AM, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@xxxxxxxxxx>
> 
> Add DT bindings for the Analog Devices AD4691 family of multichannel
> SAR ADCs (AD4691, AD4692, AD4693, AD4694).
> 
> The binding describes the hardware connections: an optional PWM on
> the CNV pin selects CNV Clock Mode; when absent, Manual Mode is used
> with CNV tied to SPI CS. GPIO pins, voltage supplies, and the
> trigger-source interface for SPI Engine offload operation are also
> described.
> 
> Signed-off-by: Radu Sabau <radu.sabau@xxxxxxxxxx>
> ---
>  .../devicetree/bindings/iio/adc/adi,ad4691.yaml    | 180 +++++++++++++++++++++
>  MAINTAINERS                                        |   8 +
>  include/dt-bindings/iio/adc/adi,ad4691.h           |  13 ++
>  3 files changed, 201 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml
> new file mode 100644
> index 000000000000..a9301e0ca851
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml
> @@ -0,0 +1,180 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad4691.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD4691 Family Multichannel SAR ADCs
> +
> +maintainers:
> +  - Radu Sabau <radu.sabau@xxxxxxxxxx>
> +
> +description: |
> +  The AD4691 family are high-speed, low-power, multichannel successive
> +  approximation register (SAR) analog-to-digital converters (ADCs) with
> +  an SPI-compatible serial interface. The ADC supports CNV Clock Mode,
> +  where an external PWM drives the CNV pin, and Manual Mode, where CNV
> +  is directly tied to the SPI chip-select.
> +
> +  Datasheets:
> +    * https://www.analog.com/en/products/ad4692.html
> +    * https://www.analog.com/en/products/ad4691.html
> +    * https://www.analog.com/en/products/ad4694.html
> +    * https://www.analog.com/en/products/ad4693.html
> +
> +$ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad4691
> +      - adi,ad4692
> +      - adi,ad4693
> +      - adi,ad4694
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 40000000
> +
> +  spi-cpol: true
> +  spi-cpha: true
> +
> +  vio-supply:
> +    description: I/O voltage supply (1.71V to 1.89V or VDD).
> +

Missing avdd-supply and ldo-in-supply.


> +  vref-supply:
> +    description: External reference voltage supply (2.4V to 5.25V).
> +
> +  vrefin-supply:
> +    description: Internal reference buffer input supply.
> +

It always confuses me when "v" is added to the pin name. Generally, "VREF"
is an internal signal and "REF" is the actual pin name. So it makes more
sense to me to call these ref-supply and refin-supply.

> +  reset-gpios:
> +    description: GPIO connected to the RESET pin (active high).

Datasheet says "Active Low".

> +    maxItems: 1
> +
> +  clocks:
> +    description: Reference clock for PWM timing in CNV Clock Mode.
> +    maxItems: 1

I feel like I asked this already, but which pin is this clock connected to?
It sounds like it is the clock for the PWM, not the ADC. So it does not belong
here.

> +
> +  pwms:
> +    description:
> +      PWM connected to the CNV pin. When present, selects CNV Clock Mode where
> +      the PWM drives the conversion rate. When absent, Manual Mode is used
> +      (CNV tied to SPI CS).
> +    maxItems: 1
> +
> +  pwm-names:
> +    items:
> +      - const: cnv

Usually, we don't have a name when there is only one.

> +
> +  interrupts:
> +    description:
> +      Interrupt line connected to the ADC GP0 pin. GP0 must be physically
> +      wired to an interrupt-capable input on the SoC. The ADC asserts GP0 as
> +      DATA_READY at end of conversion, used both for non-offload CNV Clock Mode
> +      operation and for SPI Engine offload triggering via '#trigger-source-cells'.
> +      Not used in Manual Mode, where CNV is tied to SPI CS and no DATA_READY
> +      signal is generated.
> +    maxItems: 1
> +
> +  '#trigger-source-cells':
> +    description: |
> +      For SPI Engine offload operation, this node acts as a trigger source.

I don't think we need to call out SPI offload here. In theory, this could
also also be used for synchronization with something else.

> +      Two cells are required:
> +        - First cell: Trigger event type (0 = BUSY, 1 = DATA_READY)

I'm wondering if we really need to specify the event type. For interrupts,
we we just specify the pin and not the function when the pin has more than
one possible function.

I know that we have done something like this on some of the previous SPI
offload devices. So maybe there was a good reason for it. Or maybe I just
had tunnel vision at the time.

I suggest we try implementing this with just one cell that specifies the
physical pin. In the driver, when SPI_OFFLOAD_TRIGGER_DATA_READY is
requested in the driver, we can use that to program the function of the
pin accordingly. 

> +        - Second cell: GPIO pin number (only 0 = GP0 is supported)

If GP0 is the only possible pin for an output, we should omit the cell. If
there are more possible pins, we should document them (even if the driver
doesn't support it).

> +
> +      Macros are available in dt-bindings/iio/adc/adi,ad4691.h:
> +        AD4691_TRIGGER_EVENT_BUSY, AD4691_TRIGGER_EVENT_DATA_READY
> +        AD4691_TRIGGER_PIN_GP0
> +    const: 2
> +
> +required:
> +  - compatible
> +  - reg
> +  - vio-supply
> +  - reset-gpios
> +
> +allOf:
> +  # vref-supply and vrefin-supply are mutually exclusive, one is required
> +  - oneOf:
> +      - required:
> +          - vref-supply
> +      - required:
> +          - vrefin-supply
> +
> +  # CNV Clock Mode requires a reference clock.
> +  - if:
> +      required:
> +        - pwms
> +    then:
> +      required:
> +        - clocks
> +
> +  # CNV Clock Mode (pwms present) without SPI offload requires a DRDY interrupt.
> +  # Offload configurations expose '#trigger-source-cells' instead.
> +  - if:
> +      required:
> +        - pwms
> +      not:
> +        required:
> +          - '#trigger-source-cells'
> +    then:
> +      required:
> +        - interrupts
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    /* Example: AD4692 in CNV Clock Mode (pwms present) with standard SPI */
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +            compatible = "adi,ad4692";
> +            reg = <0>;
> +            spi-cpol;
> +            spi-cpha;
> +            spi-max-frequency = <40000000>;
> +
> +            vio-supply = <&vio_supply>;
> +            vref-supply = <&vref_5v>;
> +
> +            reset-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>;

I would expect reset to be active low to match the hardware.

> +
> +            clocks = <&ref_clk>;
> +
> +            pwms = <&pwm_gen 0 0>;
> +            pwm-names = "cnv";

Should we also include the trigger in this example?

> +
> +            interrupts = <12 4>;
> +        };
> +    };
> +
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    /* Example: AD4692 in Manual Mode (no pwms) with SPI Engine offload */
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +            compatible = "adi,ad4692";
> +            reg = <0>;
> +            spi-cpol;
> +            spi-cpha;
> +            spi-max-frequency = <31250000>;
> +
> +            vio-supply = <&vio_supply>;
> +            vrefin-supply = <&vrefin_supply>;
> +
> +            reset-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>;
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 61bf550fd37c..9994d107d88d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1484,6 +1484,14 @@ W:	https://ez.analog.com/linux-software-drivers
>  F:	Documentation/devicetree/bindings/iio/adc/adi,ad4170-4.yaml
>  F:	drivers/iio/adc/ad4170-4.c
>  
> +ANALOG DEVICES INC AD4691 DRIVER
> +M:	Radu Sabau <radu.sabau@xxxxxxxxxx>
> +L:	linux-iio@xxxxxxxxxxxxxxx
> +S:	Supported
> +W:	https://ez.analog.com/linux-software-drivers
> +F:	Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml
> +F:	include/dt-bindings/iio/adc/adi,ad4691.h
> +
>  ANALOG DEVICES INC AD4695 DRIVER
>  M:	Michael Hennerich <michael.hennerich@xxxxxxxxxx>
>  M:	Nuno Sá <nuno.sa@xxxxxxxxxx>
> diff --git a/include/dt-bindings/iio/adc/adi,ad4691.h b/include/dt-bindings/iio/adc/adi,ad4691.h
> new file mode 100644
> index 000000000000..294b03974f48
> --- /dev/null
> +++ b/include/dt-bindings/iio/adc/adi,ad4691.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +
> +#ifndef _DT_BINDINGS_ADI_AD4691_H
> +#define _DT_BINDINGS_ADI_AD4691_H
> +
> +/* Trigger event types */
> +#define AD4691_TRIGGER_EVENT_BUSY	0
> +#define AD4691_TRIGGER_EVENT_DATA_READY	1
> +
> +/* Trigger GPIO pin selection */
> +#define AD4691_TRIGGER_PIN_GP0		0

Could probably do without a macro to give GP0/1/2/3 names since
it will be pretty obvious that 0 = GP0, etc.

> +
> +#endif /* _DT_BINDINGS_ADI_AD4691_H */
>