[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5bdc6802bc10b71774fb6d839a1a45d94463f4cf.camel@gmail.com>
Date: Sun, 04 May 2025 09:37:12 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: David Lechner <dlechner@...libre.com>, Michael Hennerich
<Michael.Hennerich@...log.com>, Jonathan Cameron <jic23@...nel.org>, Nuno
Sá
<nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>, Angelo Dureghello
<adureghello@...libre.com>
Cc: Jonathan Cameron <Jonathan.Cameron@...wei.com>,
linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] iio: adc: ad7606_spi: add offload scan mask check
On Fri, 2025-05-02 at 10:42 -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.
>
> Fixes: e96d35faf357 ("iio: adc: ad7606: add SPI offload support")
> Signed-off-by: David Lechner <dlechner@...libre.com>
> ---
Reviewed-by: Nuno Sá <nuno.sa@...log.com>
> Changes in v2:
> - Use bitmap_weight() instead of bitmap_equal().
> - Link to v1:
> https://lore.kernel.org/r/20250430-iio-adc-ad7606_spi-fix-offload-scan-mask-check-v1-1-8b165d9d6c0e@baylibre.com
>
> And in case it isn't obvious, the patch this fixes is fairly recent, so
> this goes in togreg rather than fixes-togreg.
> ---
> drivers/iio/adc/ad7606_spi.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
> index
> 997be483ebb93293481b922e13ece4edb47e940a..5b5b4677273b15956f1da73da41b16c5ee64e818
> 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,44 @@ 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) {
> + unsigned int num_adc_ch = st->chip_info->num_adc_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_weight(scan_mask, num_adc_ch) != num_adc_ch)
> + 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 +376,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 +386,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 +396,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: a9b169746d2e299159d4dde190552ae620982bbd
> change-id: 20250430-iio-adc-ad7606_spi-fix-offload-scan-mask-check-1d330400c014
>
> Best regards,
Powered by blists - more mailing lists