Re: [PATCH v1 2/3] dt-bindings: iio: adc: ad7192: Add sync gpio

[David Lechner <dlechner@baylibre.com>]

On 11/28/24 6:55 AM, Alisa-Dariana Roman wrote:
> Add support for the SYNC pin GPIO specification in the devicetree
> bindings. This pin allows synchronization of digital filters and analog
> modulators when using multiple devices. Update the examples to
> demonstrate the usage of the new property.
> 
> Also update the interrupt type in the examples to use the proper
> IRQ_TYPE_EDGE_FALLING macro instead of the raw value.
> 
> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
> ---
>  .../bindings/iio/adc/adi,ad7192.yaml          | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> index f70caefdace7..1cd0fd13bc42 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7192.yaml
> @@ -59,6 +59,17 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  sync-gpios:
> +    description: |
> +      Optional GPIO spec for the SYNC pin. The SYNC pin allows synchronization
> +      of the digital filters and analog modulators when using multiple AD7192
> +      devices. When SYNC is pulled low, it resets the digital filter nodes,
> +      filter control logic, calibration control logic, and holds the analog
> +      modulator in reset state. Only specify this property if you need to
> +      actively control SYNC for multi-device synchronization, otherwise it
> +      defaults to HIGH.

The datasheet says that the /SYNC pin can also be used to trigger a conversion.
But I'm not sure we really need all this explanation here. The important parts
are that this is a GPIO that is connected to the /SYNC pin and in the
synchronization use case, the same GPIO might be connected to multiple ADC
chips.

> +    maxItems: 1
> +
>    aincom-supply:
>      description: |
>        AINCOM voltage supply. Analog inputs AINx are referenced to this input
> @@ -182,6 +193,8 @@ unevaluatedProperties: false
>  
>  examples:
>    - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
>      spi {
>          #address-cells = <1>;
>          #size-cells = <0>;
> @@ -194,8 +207,9 @@ examples:
>              spi-cpha;
>              clocks = <&ad7192_mclk>;
>              clock-names = "mclk";
> -            interrupts = <25 0x2>;
> +            interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
>              interrupt-parent = <&gpio>;
> +            sync-gpios = <&gpio0 26 GPIO_ACTIVE_HIGH>;

On the schematic, the SYNC pin is marked as active low, so I would
expect GPIO_ACTIVE_LOW here.

>              aincom-supply = <&aincom>;
>              dvdd-supply = <&dvdd>;
>              avdd-supply = <&avdd>;
> @@ -208,6 +222,7 @@ examples:
>          };
>      };
>    - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
>      spi {
>          #address-cells = <1>;
>          #size-cells = <0>;
> @@ -223,7 +238,7 @@ examples:
>              spi-cpol;
>              spi-cpha;
>              #clock-cells = <0>;
> -            interrupts = <25 0x2>;
> +            interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
>              interrupt-parent = <&gpio>;
>              aincom-supply = <&aincom>;
>              dvdd-supply = <&dvdd>;