[PATCH] iio: adc: ad7606_spi: add offload scan mask check

David Lechner posted 1 patch 9 months, 2 weeks ago
There is a newer version of this series
drivers/iio/adc/ad7606_spi.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
[PATCH] iio: adc: ad7606_spi: add offload scan mask check
Posted by David Lechner 9 months, 2 weeks ago
Validate the scan mask when SPI offloading is being used.

Since this family of ADCs is simultaneous sampling, there isn't a way
to selectively disable channels when reading sample data. (Technically,
AD7616 has a sequencer so could have some control, but that is for
another day).

For "regular" IIO triggered buffer reads, this isn't a problem and the
IIO core will demux the data and ignore data from disabled channels.
However, since SPI offloading is done completely in hardware, we don't
have a way to do the same. So before this patch, if less than all
channels were enabled, the data would be misplaced in the buffer.

By adding a check in update_scan_mode, we can fail to enable the buffer
instead of having bad data returned to userspace.

Fixes: e96d35faf357 ("iio: adc: ad7606: add SPI offload support")
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad7606_spi.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
index 997be483ebb93293481b922e13ece4edb47e940a..69287c0c231bc57a108cc782a0d98db83b7612f8 100644
--- a/drivers/iio/adc/ad7606_spi.c
+++ b/drivers/iio/adc/ad7606_spi.c
@@ -5,6 +5,7 @@
  * Copyright 2011 Analog Devices Inc.
  */
 
+#include <linux/bitmap.h>
 #include <linux/err.h>
 #include <linux/math.h>
 #include <linux/module.h>
@@ -329,19 +330,47 @@ static int ad7606_spi_offload_probe(struct device *dev,
 	return 0;
 }
 
+static int ad7606_spi_update_scan_mode(struct iio_dev *indio_dev,
+				       const unsigned long *scan_mask)
+{
+	struct ad7606_state *st = iio_priv(indio_dev);
+
+	if (st->offload_en) {
+		DECLARE_BITMAP(bitmap, AD760X_MAX_CHANNELS);
+
+		bitmap_fill(bitmap, AD760X_MAX_CHANNELS);
+
+		/*
+		 * SPI offload requires that all channels are enabled since
+		 * there isn't a way to selectively disable channels that get
+		 * read (this is simultaneous sampling ADC) and the DMA buffer
+		 * has no way of demuxing the data to filter out unwanted
+		 * channels.
+		 */
+		if (!bitmap_equal(bitmap, scan_mask,
+				  st->chip_info->num_adc_channels))
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
 static const struct ad7606_bus_ops ad7606_spi_bops = {
 	.offload_config = ad7606_spi_offload_probe,
 	.read_block = ad7606_spi_read_block,
+	.update_scan_mode = ad7606_spi_update_scan_mode,
 };
 
 static const struct ad7606_bus_ops ad7607_spi_bops = {
 	.offload_config = ad7606_spi_offload_probe,
 	.read_block = ad7606_spi_read_block14to16,
+	.update_scan_mode = ad7606_spi_update_scan_mode,
 };
 
 static const struct ad7606_bus_ops ad7608_spi_bops = {
 	.offload_config = ad7606_spi_offload_probe,
 	.read_block = ad7606_spi_read_block18to32,
+	.update_scan_mode = ad7606_spi_update_scan_mode,
 };
 
 static const struct ad7606_bus_ops ad7616_spi_bops = {
@@ -350,6 +379,7 @@ static const struct ad7606_bus_ops ad7616_spi_bops = {
 	.reg_read = ad7606_spi_reg_read,
 	.reg_write = ad7606_spi_reg_write,
 	.rd_wr_cmd = ad7616_spi_rd_wr_cmd,
+	.update_scan_mode = ad7606_spi_update_scan_mode,
 };
 
 static const struct ad7606_bus_ops ad7606b_spi_bops = {
@@ -359,6 +389,7 @@ static const struct ad7606_bus_ops ad7606b_spi_bops = {
 	.reg_write = ad7606_spi_reg_write,
 	.rd_wr_cmd = ad7606b_spi_rd_wr_cmd,
 	.sw_mode_config = ad7606b_sw_mode_config,
+	.update_scan_mode = ad7606_spi_update_scan_mode,
 };
 
 static const struct ad7606_bus_ops ad7606c_18_spi_bops = {
@@ -368,6 +399,7 @@ static const struct ad7606_bus_ops ad7606c_18_spi_bops = {
 	.reg_write = ad7606_spi_reg_write,
 	.rd_wr_cmd = ad7606b_spi_rd_wr_cmd,
 	.sw_mode_config = ad7606b_sw_mode_config,
+	.update_scan_mode = ad7606_spi_update_scan_mode,
 };
 
 static const struct ad7606_bus_info ad7605_4_bus_info = {

---
base-commit: 350224bdb9725aab5b90d72fc3db7618ebd232ae
change-id: 20250430-iio-adc-ad7606_spi-fix-offload-scan-mask-check-1d330400c014

Best regards,
-- 
David Lechner <dlechner@baylibre.com>
Re: [PATCH] iio: adc: ad7606_spi: add offload scan mask check
Posted by Andy Shevchenko 9 months, 1 week ago
On Wed, Apr 30, 2025 at 04:41:58PM -0500, David Lechner wrote:
> Validate the scan mask when SPI offloading is being used.
> 
> Since this family of ADCs is simultaneous sampling, there isn't a way
> to selectively disable channels when reading sample data. (Technically,
> AD7616 has a sequencer so could have some control, but that is for
> another day).
> 
> For "regular" IIO triggered buffer reads, this isn't a problem and the
> IIO core will demux the data and ignore data from disabled channels.
> However, since SPI offloading is done completely in hardware, we don't
> have a way to do the same. So before this patch, if less than all
> channels were enabled, the data would be misplaced in the buffer.
> 
> By adding a check in update_scan_mode, we can fail to enable the buffer
> instead of having bad data returned to userspace.

...

> +static int ad7606_spi_update_scan_mode(struct iio_dev *indio_dev,
> +				       const unsigned long *scan_mask)
> +{
> +	struct ad7606_state *st = iio_priv(indio_dev);
> +
> +	if (st->offload_en) {
> +		DECLARE_BITMAP(bitmap, AD760X_MAX_CHANNELS);
> +
> +		bitmap_fill(bitmap, AD760X_MAX_CHANNELS);
> +
> +		/*
> +		 * SPI offload requires that all channels are enabled since
> +		 * there isn't a way to selectively disable channels that get
> +		 * read (this is simultaneous sampling ADC) and the DMA buffer
> +		 * has no way of demuxing the data to filter out unwanted
> +		 * channels.
> +		 */
> +		if (!bitmap_equal(bitmap, scan_mask,
> +				  st->chip_info->num_adc_channels))
> +			return -EINVAL;


No need to have the local variable, just make sure that bitmap_weigth(FOO) == FOO.

	if (... && bitmap_weight(FOO) != FOO)
		return -EINVAL;

> +	}
> +
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko