Re: [PATCH v6 02/11] dt-bindings: iio: adc: ad7768-1: add trigger-sources property

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 27 Apr 2025 21:12:16 -0300
Jonathan Santos <Jonathan.Santos@analog.com> wrote:

> In addition to GPIO synchronization, The AD7768-1 also supports
> synchronization over SPI, which use is recommended when the GPIO
> cannot provide a pulse synchronous with the base MCLK signal. It
> consists of looping back the SYNC_OUT to the SYNC_IN pin and send
> a command via SPI to trigger the synchronization.
> 
> Introduce the 'trigger-sources' property to enable SPI-based
> synchronization via SYNC_OUT pin, along with additional optional
> entries for GPIO3 and DRDY pins.
> 
> Also create #trigger-source-cells property to differentiate the trigger
> sources provided by the ADC. To improve readability, create a
> adi,ad7768-1.h header with the macros for the cell values.
> 
> While at it, add description to the interrupts property.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>

A few things inline, but minor stuff for if you need to do a respin.

> ---
> v6 Changes:
> * Removed references to offload.
> * Changed trigger-sources-cells description. Each cell value indicates
>   a gpio line from the ADC.
> * Added adi,ad7768-1.h header with macros for the trigger source cells.
> * Removed offload trigger entry from trigger-sources.
> 
> v5 Changes:
> * Include START pin and DRDY in the trigger-sources description.
> * Fixed "#trigger-source-cells" value and description.
> * sync-in-gpios is represented in the trigger-sources property.
> 
> v4 Changes:
> * none
> 
> v3 Changes:
> * Fixed dt-bindings errors.
> * Trigger-source is set as an alternative to sync-in-gpios, so we
>   don't break the previous ABI.
> * increased maxItems from trigger-sources to 2.
> 
> v2 Changes:
> * Patch added as replacement for adi,sync-in-spi patch.
> * addressed the request for a description to interrupts property.
> ---
>  .../bindings/iio/adc/adi,ad7768-1.yaml        | 31 ++++++++++++++++++-
>  include/dt-bindings/iio/adc/adi,ad7768-1.h    | 10 ++++++
>  2 files changed, 40 insertions(+), 1 deletion(-)
>  create mode 100644 include/dt-bindings/iio/adc/adi,ad7768-1.h
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> index 3ce59d4d065f..1f476aa15305 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7768-1.yaml
> @@ -26,7 +26,28 @@ properties:
>    clock-names:
>      const: mclk
>  
> +  trigger-sources:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    minItems: 1
> +    maxItems: 2
> +    description: |
> +      A list of phandles referencing trigger source providers. Each entry
> +      represents a trigger source for the ADC:
> +
> +        - First entry specifies the device responsible for driving the
> +          synchronization (SYNC_IN) pin, as an alternative to adi,sync-in-gpios.
> +          This can be a `gpio-trigger` or another `ad7768-1` device. If the
> +          device's own SYNC_OUT pin is internally connected to its SYNC_IN pin,
> +          reference the device itself or omit this property.
> +        - Second entry optionally defines a GPIO3 pin used as a START signal trigger.
> +
> +      Use the accompanying trigger source cell to identify the type of each entry.
> +
>    interrupts:
> +    description:
> +      Specifies the interrupt line associated with the ADC. This refers
> +      to the DRDY (Data Ready) pin, which signals when conversion results are
> +      available.

Trivial but this seems overly wordy. The only bit that matters is the datardy bit.

     DRDY (Data Ready) pin, which signals conversion results are available.

is probably enough.  Only bother if you are respining for other reasons however
as this really doesn't matter!

>      maxItems: 1
>  
>    '#address-cells':
> @@ -57,6 +78,15 @@ properties:
>    "#io-channel-cells":
>      const: 1
>  
> +  "#trigger-source-cells":
> +    description: |
> +      Cell indicates the trigger output signal: 0 = SYNC_OUT, 1 = GPIO3,
> +      2 = DRDY.
> +
> +      For better readability, macros for these values are available in
> +      dt-bindings/iio/adc/adi,ad7768-1.h.
> +    const: 1
> +
>  required:
>    - compatible
>    - reg
> @@ -65,7 +95,6 @@ required:
>    - vref-supply
>    - spi-cpol
>    - spi-cpha
> -  - adi,sync-in-gpios

Maybe worth requiring oneOf adi,sync-in-gpios or trigger-sources? 

>  
>  patternProperties:
>    "^channel@([0-9]|1[0-5])$":
> diff --git a/include/dt-bindings/iio/adc/adi,ad7768-1.h b/include/dt-bindings/iio/adc/adi,ad7768-1.h
> new file mode 100644
> index 000000000000..34d92856a50b
> --- /dev/null
> +++ b/include/dt-bindings/iio/adc/adi,ad7768-1.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +
> +#ifndef _DT_BINDINGS_ADI_AD7768_1_H
> +#define _DT_BINDINGS_ADI_AD7768_1_H
> +
> +#define AD7768_TRIGGER_SOURCE_SYNC_OUT  0
> +#define AD7768_TRIGGER_SOURCE_GPIO3     1
> +#define AD7768_TRIGGER_SOURCE_DRDY      2
> +
> +#endif /* _DT_BINDINGS_ADI_AD7768_1_H */