Re: [PATCH v3 2/8] dt-bindings: iio: adc: Add adi,ad4052

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 10 Jun 2025 09:34:35 +0200
Jorge Marques <jorge.marques@analog.com> wrote:

> Add dt-bindings for AD4052 family, devices AD4050/AD4052/AD4056/AD4058,
> low-power with monitor capabilities SAR ADCs. Each variant of the family
> differs in speed and resolution, resulting in different scan types and
> spi word sizes, that are matched by the compatible with the chip_info.

The bit about what the drive does with this doesn't really belong in a DT
binding patch description. Stick to just something like.

Each variant of the family differs in speed and resolution, reflected
in word size for SPI messages.

> The device contains one input (cnv) and two outputs (gp0, gp1).
> The outputs can be configured for range of options, such as threshold
> and data ready.
> The spi-max-frequency refers to the configuration mode maximum access
> speed. The ADC mode speed depends on the vio input voltage.
> 
> Signed-off-by: Jorge Marques <jorge.marques@analog.com>
> ---
>  .../devicetree/bindings/iio/adc/adi,ad4052.yaml    | 167 +++++++++++++++++++++
>  MAINTAINERS                                        |   6 +
>  include/dt-bindings/iio/adc/adi,ad4052.h           |  17 +++
>  3 files changed, 190 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4052.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4052.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..2cf197e2d872d9a3d4f7210121a1e38f784f92dc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4052.yaml
> @@ -0,0 +1,167 @@
> +
> +  interrupts:
> +    items:
> +      - description: Signal coming from the GP0 pin.
Description would be better in interrupt-names than here.
> +      - description: Signal coming from the GP1 pin.

Also minItems should be specified to allow for just one of these
being wired I think.

> +
> +  interrupt-names:
> +    items:
> +      - const: gp0
> +      - const: gp1
> +
> +  cnv-gpios:
> +    description: The Convert Input (CNV). If omitted, CNV is tied to SPI CS.
> +    maxItems: 1
> +
> +  pwms:
> +    maxItems: 1
> +    description: PWM connected to the CNV pin.
> +
> +  trigger-sources:
> +    minItems: 1
> +    maxItems: 2
> +    description:
> +      Describes the output pin and event associated.
> +
> +  "#trigger-source-cells":
> +    const: 2
> +    description: |
> +      Output pins used as trigger source.
> +
> +      Cell 0 defines the event:
> +      * 0 = Data ready
> +      * 1 = Min threshold
> +      * 2 = Max threshold
> +      * 3 = Either threshold
> +      * 4 = CHOP control
> +      * 5 = Device enable
> +      * 6 = Device ready (only GP1)

Hmm. I'm a bit dubious on why 'what the offload trigger is'
is a DT thing?  Is that because the IP needs to comprehend
this?  I guess only data ready is actually supported in
practice? 

What would the use of Device enable or device ready or chop
control actually be?

The thresholds are unusual but those I can sort of understand.

Jonathan

> +
> +      Cell 1 defines which pin:
> +      * 0 = GP0
> +      * 1 = GP1
> +
> +      For convenience, macros for these values are available in
> +      dt-bindings/iio/adc/adi,ad4052.h.
> +
> +  spi-max-frequency:
> +    maximum: 83333333
> +
> +  vdd-supply:
> +    description: Analog power supply.
> +
> +  vio-supply:
> +    description: Digital interface logic power supply.
> +
> +  ref-supply:
> +    description: |

Don't need the | as no need to preserve anything about formatting of
a single paragraph like this.


> +      Reference voltage to set the ADC full-scale range. If not present,
> +      vdd-supply is used as the reference voltage.
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +  - vio-supply
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/iio/adc/adi,ad4052.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +            compatible = "adi,ad4052";
> +            reg = <0>;
> +            vdd-supply = <&vdd>;
> +            vio-supply = <&vio>;
> +            ref-supply = <&ref>;
> +            spi-max-frequency = <83333333>;
> +
> +            #trigger-source-cells = <2>;
> +            trigger-sources = <&adc AD4052_TRIGGER_EVENT_EITHER_THRESH
> +                                    AD4052_TRIGGER_PIN_GP0
> +                               &adc AD4052_TRIGGER_EVENT_DATA_READY
> +                                    AD4052_TRIGGER_PIN_GP1>;
> +            interrupt-parent = <&gpio>;
> +            interrupts = <0 0 IRQ_TYPE_EDGE_RISING>,
> +                         <0 1 IRQ_TYPE_EDGE_FALLING>;
> +            interrupt-names = "gp0", "gp1";
> +            cnv-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
> +        };
> +    };
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/iio/adc/adi,ad4052.h>
> +
> +    rx_dma {
> +            #dma-cells = <1>;
> +    };
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        dmas = <&rx_dma 0>;
> +        dma-names = "offload0-rx";
> +        trigger-sources = <&adc AD4052_TRIGGER_EVENT_DATA_READY
> +                                AD4052_TRIGGER_PIN_GP1>;
> +
> +        adc@0 {
> +            compatible = "adi,ad4052";
> +            reg = <0>;
> +            vdd-supply = <&vdd>;
> +            vio-supply = <&vio>;
> +            spi-max-frequency = <83333333>;
> +            pwms = <&adc_trigger 0 10000 0>;
> +
> +            #trigger-source-cells = <2>;
> +            trigger-sources = <&adc AD4052_TRIGGER_EVENT_EITHER_THRESH
> +                                    AD4052_TRIGGER_PIN_GP0
> +                               &adc AD4052_TRIGGER_EVENT_DATA_READY
> +                                    AD4052_TRIGGER_PIN_GP1>;
> +            interrupt-parent = <&gpio>;
> +            interrupts = <0 0 IRQ_TYPE_EDGE_RISING>,
> +                         <0 1 IRQ_TYPE_EDGE_FALLING>;
> +            interrupt-names = "gp0", "gp1";
> +            cnv-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c02d83560058f7ea75e24509b4d87ef293df6773..d000c7de7ff9eba390f87593bc2b1847f966f48b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1337,6 +1337,12 @@ F:	Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml
>  F:	Documentation/iio/ad4030.rst
>  F:	drivers/iio/adc/ad4030.c
>  
> +ANALOG DEVICES INC AD4052 DRIVER
> +M:	Jorge Marques <jorge.marques@analog.com>
> +S:	Supported
> +W:	https://ez.analog.com/linux-software-drivers
> +F:	Documentation/devicetree/bindings/iio/adc/adi,ad4052.yaml
> +
>  ANALOG DEVICES INC AD4080 DRIVER
>  M:	Antoniu Miclaus <antoniu.miclaus@analog.com>
>  L:	linux-iio@vger.kernel.org
> diff --git a/include/dt-bindings/iio/adc/adi,ad4052.h b/include/dt-bindings/iio/adc/adi,ad4052.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..37db5d9d10e788d5e7fb715c4ba9077e555131d5
> --- /dev/null
> +++ b/include/dt-bindings/iio/adc/adi,ad4052.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +
> +#ifndef _DT_BINDINGS_ADI_AD4052_H
> +#define _DT_BINDINGS_ADI_AD4052_H
> +
> +#define AD4052_TRIGGER_EVENT_DATA_READY		0
> +#define AD4052_TRIGGER_EVENT_MIN_THRESH		1
> +#define AD4052_TRIGGER_EVENT_MAX_THRESH		2
> +#define AD4052_TRIGGER_EVENT_EITHER_THRESH	3
> +#define AD4052_TRIGGER_EVENT_CHOP		4
> +#define AD4052_TRIGGER_EVENT_DEV_ENABLED	5
> +#define AD4052_TRIGGER_EVENT_DEV_READY		6
> +
> +#define AD4052_TRIGGER_PIN_GP0		0
> +#define AD4052_TRIGGER_PIN_GP1		1
> +
> +#endif /* _DT_BINDINGS_ADI_AD4052_H */
>