Re: [PATCH v1 3/3] iio: adc: ad7192: Add sync gpio

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 28 Nov 2024 14:55:03 +0200
Alisa-Dariana Roman <alisadariana@gmail.com> wrote:

> Add support for the SYNC pin of AD719x devices. This pin is controlled
> through a GPIO. The pin allows synchronization of digital filters and
> analog modulators when using multiple devices.
> 
> Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com>
Hi.

Like all userspace ABI, this needs documentation.

It's an unusual feature, so some usecases would help.

It is also cross multiple devices which makes this odd as only one device
can presumably acquire the gpio?

An alternative would be to look at how to do this with a 'wrapper' sort of device
so that we have one instance to which this applies.

I'm not sure that helps that much though as we'd still need some for of
'I'm setup for all channels, now you can go' ABI.

Jonathan

> ---
>  drivers/iio/adc/ad7192.c | 112 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 111 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> index 955e9eff0099..542db7280e99 100644
> --- a/drivers/iio/adc/ad7192.c
> +++ b/drivers/iio/adc/ad7192.c
> @@ -10,6 +10,7 @@
>  #include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/device.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/kernel.h>
>  #include <linux/slab.h>
>  #include <linux/sysfs.h>
> @@ -196,6 +197,7 @@ struct ad7192_chip_info {
>  	u8				num_channels;
>  	const struct ad_sigma_delta_info	*sigma_delta_info;
>  	const struct iio_info		*info;
> +	const struct iio_info		*info_sync;
>  	int (*parse_channels)(struct iio_dev *indio_dev);
>  };
>  
> @@ -216,6 +218,8 @@ struct ad7192_state {
>  	struct mutex			lock;	/* protect sensor state */
>  	u8				syscalib_mode[8];
>  
> +	struct gpio_desc		*sync_gpio;
> +
>  	struct ad_sigma_delta		sd;
>  };
>  
> @@ -783,6 +787,36 @@ static void ad7192_update_filter_freq_avail(struct ad7192_state *st)
>  	st->filter_freq_avail[3][0] = DIV_ROUND_CLOSEST(fadc * 272, 1024);
>  }
>  
> +static ssize_t sync_gpio_show(struct device *dev,
> +			      struct device_attribute *attr,
> +			      char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct ad7192_state *st = iio_priv(indio_dev);
> +
> +	return sysfs_emit(buf, "%d\n", gpiod_get_value(st->sync_gpio));
> +}
> +
> +static ssize_t sync_gpio_store(struct device *dev,
> +			       struct device_attribute *attr,
> +			       const char *buf,
> +			       size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct ad7192_state *st = iio_priv(indio_dev);
> +	int val;
> +	int ret;
> +
> +	ret = kstrtoint(buf, 0, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (st->sync_gpio)
> +		gpiod_set_value(st->sync_gpio, val);
> +
> +	return len;
> +}