Re: [PATCH v4 07/13] iio: adc: ad4134: Support buffered data read

[Nuno Sá]

On Tue, 06 Oct 2026 15:17:56 -0300, Marcelo Schmitt <marcelo.schmitt@xxxxxxxxxx> wrote:
> diff --git a/drivers/iio/adc/ad4134.c b/drivers/iio/adc/ad4134.c
> index 3048e61a3cb1..b5207f16054e 100644
> --- a/drivers/iio/adc/ad4134.c
> +++ b/drivers/iio/adc/ad4134.c
> @@ -149,7 +160,7 @@ struct ad4134_state {
>  	 * DMA (thus cache coherency maintenance) requires the transfer buffers
>  	 * to live in their own cache lines.
>  	 */
> -	u8 rx_buf[AD4134_SPI_MAX_XFER_LEN] __aligned(IIO_DMA_MINALIGN);
> +	u8 rx_buf[AD4134_SPI_MAX_XFER_LEN * AD4134_NUM_CHANNELS] __aligned(IIO_DMA_MINALIGN);
>  	u8 tx_buf[AD4134_SPI_MAX_XFER_LEN];
>  };

The above is indeed a possible issue! Not just about dma but also about
concurrency on the buffers.

> @@ -333,6 +344,44 @@ static const struct regmap_config ad4134_regmap_config = {
> [ ... skip 17 lines ... ]
> +	gpiod_set_value_cansleep(st->odr_gpio, 1);
> +	fsleep(1);
> +	gpiod_set_value_cansleep(st->odr_gpio, 0);
> +
> +	/* Run enough DCLK cycles to match the data frame length. */
> +	ret = spi_sync_transfer(st->spi, &xfer, 1);

Yeah the above needs to be sync with debugfs. Either by not allowing
debugfs when buffering (which can be odd tbh) or just by using the sync
lock around both places or even by just using a dedicated buffer (that
can even allocated with devm during probe - or on the setup buffer op
depending on the scan_mask) so that no locking is needed.

> +	if (ret)
> +		goto out;
> +
> +	for (int ch = 0, i = 0; ch < AD4134_NUM_CHANNELS; ch++) {
> +		if (!test_bit(ch, indio_dev->active_scan_mask))
> +			continue;

As you already noted, please use the proper IIO helper.

-- 
Nuno Sá <nuno.sa@xxxxxxxxxx>