Re: [PATCH v1 14/15] iio: adc: ad7768-1: add support for Synchronization over SPI

[David Lechner <dlechner@baylibre.com>]

On 1/7/25 9:27 AM, Jonathan Santos wrote:
> The synchronization method using GPIO requires the generated pulse to be
> truly synchronous with the base MCLK signal. When it is not possible to
> do that in hardware, the datasheet recommends using synchronization over
> SPI, where the generated pulse is already synchronous with MCLK. This
> requires the SYNC_OUT pin to be connected to SYNC_IN pin.
> 
> Add the option to handle device synchronization over SPI.
> 
> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> ---

...

>  static int ad7768_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
> @@ -697,11 +708,21 @@ static int ad7768_setup(struct ad7768_state *st)
>  	if (ret)
>  		return ret;
>  
> -	st->gpio_sync_in = devm_gpiod_get(&st->spi->dev, "adi,sync-in",
> -					  GPIOD_OUT_LOW);
> +	st->gpio_sync_in = devm_gpiod_get_optional(&st->spi->dev, "adi,sync-in",
> +						   GPIOD_OUT_LOW);
>  	if (IS_ERR(st->gpio_sync_in))
>  		return PTR_ERR(st->gpio_sync_in);
>  
> +	if (device_property_present(&st->spi->dev, "adi,sync-in-spi"))
> +		st->en_spi_sync = true;
> +
> +	/*
> +	 * GPIO and SPI Synchronization are mutually exclusive.
> +	 * Return error if both are enabled

Should it also be an error if we have neither? Otherwise it sounds like
decimation won't work correctly since there is a comment that says we have
to toggle this after updating the decimation rate register.

> +	 */
> +	if (st->gpio_sync_in && st->en_spi_sync)
> +		return -EINVAL;

A dev_err_probe() message would be helpful here when creating a new DT and
bringing up a new system since it is easy to forget a property or make a typo
that could lead to this error.

> +
>  	ret = ad7768_gpio_init(st);
>  	if (ret < 0)
>  		return ret;