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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 05/03/2026 13:23, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
> 
> Add YAML bindings and dt-bindings header for the Analog Devices AD4691

There are no such thing as YAML bindings. DT bindings.


> family of multichannel SAR ADCs (AD4691, AD4692, AD4693, AD4694).
> 
> The binding describes five operating modes selectable via the

No, describe the hardware, not binding.


A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18


> adi,spi-mode property, optional PWM/clock for CNV Clock and CNV Burst
> modes, GPIO pins, voltage supplies and the trigger-source interface for
> SPI Engine offload operation.
> 
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
>  .../devicetree/bindings/iio/adc/adi,ad4691.yaml    | 278 +++++++++++++++++++++
>  MAINTAINERS                                        |   8 +
>  include/dt-bindings/iio/adc/adi,ad4691.h           |  13 +
>  3 files changed, 299 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..b0d8036184b0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4691.yaml
> @@ -0,0 +1,278 @@
> +# 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@analog.com>
> +
> +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 family supports multiple operating
> +  modes including CNV Clock Mode, CNV Burst Mode, Autonomous Mode, SPI Burst
> +  Mode, and Manual Mode.
> +
> +  The driver supports both standard SPI and SPI Engine (offload) operation.

Driver is irrelevant. If you change mode, you change bindings?

> +
> +  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
> +
> +  adi,spi-mode:
> +    $ref: /schemas/types.yaml#/definitions/uint32

Nope. You already have such property, so you cannot redefine it. Look at
other sources.

...


> +  clocks:
> +    description: Reference clock for PWM timing in CNV Clock and CNV Burst modes.
> +    maxItems: 1
> +
> +  clock-names:

Drop clock-names. Not useful if you call it just ref.

> +    items:
> +      - const: ref_clk
> +
> +  pwms:
> +    description:
> +      PWM connected to the CNV pin. Required for CNV Clock Mode and CNV Burst
> +      Mode to control conversion timing.
> +    maxItems: 1
> +
> +  pwm-names:
> +    items:
> +      - const: cnv
> +
> +  interrupts:
> +    description:
> +      Interrupt from the GP0 pin configured as DATA_READY or BUSY. Required
> +      for non-offload operation in all modes except Manual Mode (mode 4),
> +      where CNV is tied to CS and no DATA_READY signal is generated.
> +    maxItems: 1
> +
> +  interrupt-names:
> +    items:
> +      - const: DRDY

Lowercase. Or actually drop names...

> +
> +  '#trigger-source-cells':
> +    description: |
> +      For SPI Engine offload operation, this node acts as a trigger source.
> +      Two cells are required:
> +        - First cell: Trigger event type (0 = BUSY, 1 = DATA_READY)
> +        - Second cell: GPIO pin number (only 0 = GP0 is supported)
> +
> +      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
> +  - adi,spi-mode
> +  - vio-supply
> +  - reset-gpios
> +
> +allOf:
> +  # vref-supply and vrefin-supply are mutually exclusive, one is required
> +  - oneOf:
> +      - required:
> +          - vref-supply
> +      - required:
> +          - vrefin-supply
> +
> +  # AD4694 (20-bit) does not support Manual Mode
> +  - if:
> +      properties:
> +        compatible:
> +          const: adi,ad4694
> +    then:
> +      properties:
> +        adi,spi-mode:
> +          enum: [0, 1, 2, 3]
> +
> +  # CNV Clock Mode and CNV Burst Mode require PWM and clock
> +  - if:
> +      properties:
> +        adi,spi-mode:
> +          enum: [0, 1]
> +    then:
> +      required:
> +        - clocks
> +        - clock-names
> +        - pwms
> +        - pwm-names
> +
> +  # Non-Manual modes (0-3) without SPI offload require a DRDY interrupt.
> +  # Offload configurations expose '#trigger-source-cells' instead.
> +  - if:
> +      properties:
> +        adi,spi-mode:
> +          enum: [0, 1, 2, 3]
> +      not:
> +        required:
> +          - '#trigger-source-cells'
> +    then:
> +      required:
> +        - interrupts
> +        - interrupt-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/iio/adc/adi,ad4691.h>
> +
> +    /* Example: AD4692 in CNV Clock Mode 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>;
> +
> +            adi,spi-mode = <0>; /* CNV Clock Mode */
> +
> +            vio-supply = <&vio_supply>;
> +            vref-supply = <&vref_5v>;
> +
> +            reset-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>;
> +            gp0-gpios = <&gpio 11 GPIO_ACTIVE_HIGH>;
> +
> +            clocks = <&ref_clk>;
> +            clock-names = "ref_clk";
> +
> +            pwms = <&pwm_gen 0 0>;
> +            pwm-names = "cnv";
> +
> +            interrupts = <12 4>;
> +            interrupt-names = "DRDY";
> +        };
> +    };
> +
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/iio/adc/adi,ad4691.h>
> +
> +    /* Example: AD4692 in Manual Mode 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>;
> +
> +            adi,spi-mode = <4>; /* Manual Mode */
> +
> +            vio-supply = <&vio_supply>;
> +            vrefin-supply = <&vrefin_supply>;
> +
> +            reset-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>;
> +        };
> +    };
> +
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/iio/adc/adi,ad4691.h>

Where do you use the header?

Anyway, drop example, two are enough.


Best regards,
Krzysztof