2021-11-10 11:08:55

by Alexandru Tachici

Subject: [PATCH 2/5] iio: adc: ad7192: Add update_scan_mode

From: Alexandru Tachici <[email protected]>

In continuous mode neither sigma_delta.c nor ad7192.c
will disable previously enabled channels.

Before this patch a channel stayed enabled indefinetly,
even when one another one was supposed to be sampled.
This causes mixed samples in continuous mode to be delivered
to the host.

By adding an update_scan_mode callback, every time the
continuous mode is activated, channels will be enabled/disabled
accordingly.

Fixes: 3f7c3306cf38 ("staging:iio:ad7192: Use common Sigma Delta library")
Signed-off-by: Alexandru Tachici <[email protected]>
---
drivers/iio/adc/ad7192.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
index 2121a812b0c3..1fc0f4eb858e 100644
--- a/drivers/iio/adc/ad7192.c
+++ b/drivers/iio/adc/ad7192.c
@@ -782,6 +782,20 @@ static int ad7192_read_avail(struct iio_dev *indio_dev,
return -EINVAL;
}

+static int ad7192_update_scan_mode(struct iio_dev *indio_dev, const unsigned long *scan_mask)
+{
+ struct ad7192_state *st = iio_priv(indio_dev);
+ int i;
+
+ st->conf &= ~AD7192_CONF_CHAN_MASK;
+ for (i = 0; i < 8; i++) {
+ if (test_bit(i, scan_mask))
+ st->conf |= AD7192_CONF_CHAN(i);
+ }
+
+ return ad_sd_write_reg(&st->sd, AD7192_REG_CONF, 3, st->conf);
+}
+
static const struct iio_info ad7192_info = {
.read_raw = ad7192_read_raw,
.write_raw = ad7192_write_raw,
@@ -789,6 +803,7 @@ static const struct iio_info ad7192_info = {
.read_avail = ad7192_read_avail,
.attrs = &ad7192_attribute_group,
.validate_trigger = ad_sd_validate_trigger,
+ .update_scan_mode = ad7192_update_scan_mode,
};

static const struct iio_info ad7195_info = {
@@ -798,6 +813,7 @@ static const struct iio_info ad7195_info = {
.read_avail = ad7192_read_avail,
.attrs = &ad7195_attribute_group,
.validate_trigger = ad_sd_validate_trigger,
+ .update_scan_mode = ad7192_update_scan_mode,
};

#define __AD719x_CHANNEL(_si, _channel1, _channel2, _address, _extend_name, \
--
2.25.1

2021-11-12 16:53:13

by Jonathan Cameron

Subject: Re: [PATCH 2/5] iio: adc: ad7192: Add update_scan_mode

On Wed, 10 Nov 2021 13:17:47 +0200
<[email protected]> wrote:

> From: Alexandru Tachici <[email protected]>
>
> In continuous mode neither sigma_delta.c nor ad7192.c
> will disable previously enabled channels.
>
> Before this patch a channel stayed enabled indefinetly,
> even when one another one was supposed to be sampled.
> This causes mixed samples in continuous mode to be delivered
> to the host.

Can you expand a bit on the path that leads to this. As far as I can tell
in both continuous mode enable and single channel reading set_channel()
callback is called and will overwrite the channels enabled previously.

Perhaps I'm missing a path in which that call isn't made?
>
> By adding an update_scan_mode callback, every time the
> continuous mode is activated, channels will be enabled/disabled
> accordingly.
>
> Fixes: 3f7c3306cf38 ("staging:iio:ad7192: Use common Sigma Delta library")
> Signed-off-by: Alexandru Tachici <[email protected]>
> ---
> drivers/iio/adc/ad7192.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c
> index 2121a812b0c3..1fc0f4eb858e 100644
> --- a/drivers/iio/adc/ad7192.c
> +++ b/drivers/iio/adc/ad7192.c
> @@ -782,6 +782,20 @@ static int ad7192_read_avail(struct iio_dev *indio_dev,
> return -EINVAL;
> }
>
> +static int ad7192_update_scan_mode(struct iio_dev *indio_dev, const unsigned long *scan_mask)
> +{
> + struct ad7192_state *st = iio_priv(indio_dev);
> + int i;
> +
> + st->conf &= ~AD7192_CONF_CHAN_MASK;
> + for (i = 0; i < 8; i++) {
> + if (test_bit(i, scan_mask))

for_each_set_bit()

> + st->conf |= AD7192_CONF_CHAN(i);
> + }
> +
> + return ad_sd_write_reg(&st->sd, AD7192_REG_CONF, 3, st->conf);
> +}
> +
> static const struct iio_info ad7192_info = {
> .read_raw = ad7192_read_raw,
> .write_raw = ad7192_write_raw,
> @@ -789,6 +803,7 @@ static const struct iio_info ad7192_info = {
> .read_avail = ad7192_read_avail,
> .attrs = &ad7192_attribute_group,
> .validate_trigger = ad_sd_validate_trigger,
> + .update_scan_mode = ad7192_update_scan_mode,
> };
>
> static const struct iio_info ad7195_info = {
> @@ -798,6 +813,7 @@ static const struct iio_info ad7195_info = {
> .read_avail = ad7192_read_avail,
> .attrs = &ad7195_attribute_group,
> .validate_trigger = ad_sd_validate_trigger,
> + .update_scan_mode = ad7192_update_scan_mode,
> };
>
> #define __AD719x_CHANNEL(_si, _channel1, _channel2, _address, _extend_name, \