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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241130183839.1fd5884f@jic23-huawei>
Date: Sat, 30 Nov 2024 18:38:39 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Alisa-Dariana Roman <alisadariana@...il.com>
Cc: Alisa-Dariana Roman <alisa.roman@...log.com>, Jonathan Cameron
 <Jonathan.Cameron@...wei.com>, Michael Hennerich
 <michael.hennerich@...log.com>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, Lars-Peter
 Clausen <lars@...afoo.de>, Rob Herring <robh@...nel.org>, Krzysztof
 Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>
Subject: Re: [PATCH v1 3/3] iio: adc: ad7192: Add sync gpio

On Thu, 28 Nov 2024 14:55:03 +0200
Alisa-Dariana Roman <alisadariana@...il.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@...log.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;
> +}