Re: [PATCH 5/9] iio: adc: rzt2h: implement DMA buffer support

[Jonathan Cameron]

On Sat, 29 Aug 2026 02:20:43 +0100
Jonathan Cameron <jic23@xxxxxxxxxx> wrote:

> > Implement buffered capture using a cyclic DMA transfer into a kfifo
> > buffer to support continuous high-rate sampling.
> > 
> > On buffer enable, switch the ADC to continuous conversion mode and start
> > a cyclic DMA transfer over the active channels.
> > 
> > Because the DMA controller does not support native scatter-gather, and
> > because of the cyclic DMA setup, transfers must be done in widths
> > covering all the enabled channels.
> > 
> > Since DMA transfer width must be a power of two and aligned to its size,
> > cover the smallest power-of-two-aligned group of channel registers
> > spanning the enabled channels.
> > 
> > Split the cyclic buffer into fixed-size periods. On each period
> > completion, bump a pending counter and wake a consumer kthread from the
> > DMA callback.
> > 
> > For every completed period, gather the enabled channels out of the DMA
> > layout into the scan layout the IIO core expects and push each scan
> > with iio_push_to_buffers().  
> 
> Could you instead use the available_scan_masks infrastructure.  bit annoying
> to specify the full list but isn't that long I think with 16 channels
> 
> 16 x single
> 8 x double,
> 4 x quads
> 2 x octect
> 1 x all of them.
> 
> The the IIO demux in (sits behind the push_to_buffers path if
> we have available_scan_masks set) will then deal with repacking
> the data if necessary.
> 
> > 
> > If the consumer kthread falls behind by a full buffer, drop the oldest
> > periods.
> > 
> > Because the DMA transfer must cover all channels between the first and
> > last enabled ones, skip disabled channels while compacting.
> > 
> > Also, the DMA controller transfers data in 32-bit words, but the ADC's
> > data registers are 16-bit wide, causing adjacent channel data to be
> > swapped. Swap consecutive channels while compacting to account for this.
> > 
> > Allocate the DMA buffer via dma_alloc_noncoherent() and synchronise it
> > per period to allow it to be cached by the CPU while compacting.
> > 
> > Disable the completion IRQ for the duration of the DMA transfer, as the
> > ICU does not mask this event from reaching the GIC even if it is being
> > used to drive the DMA capture.
> > 
> > Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@xxxxxxxxxxx>  
> 
> I'm messing around with b4 review tui and sashiko integration. I've
> left the Sashiko comments in here as I'm out of time today to look
> at them in detial.

Apparently I got the options wrong to include the sashiko replies :(

So take a look at :
https://sashiko.dev/#/patchset/20260828145943.2077589-1-cosmin-gabriel.tanislav.xa%40renesas.com

> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 415e519ad4eb..6c7b30d2b6e7 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -1551,6 +1551,8 @@ config RZT2H_ADC
> >  	tristate "Renesas RZ/T2H / RZ/N2H ADC driver"
> >  	depends on ARCH_RENESAS || COMPILE_TEST
> >  	select IIO_ADC_HELPER
> > +	select IIO_BUFFER
> > +	select IIO_KFIFO_BUF
> >  	help
> >  	  Say yes here to build support for the ADC found in Renesas
> >  	  RZ/T2H / RZ/N2H SoCs.
> > diff --git a/drivers/iio/adc/rzt2h_adc.c b/drivers/iio/adc/rzt2h_adc.c
> > index 95bcebdc02cb..d76226375f22 100644
> > --- a/drivers/iio/adc/rzt2h_adc.c
> > +++ b/drivers/iio/adc/rzt2h_adc.c
> > @@ -4,11 +4,16 @@
> >  #include <linux/cleanup.h>
> >  #include <linux/completion.h>
> >  #include <linux/delay.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/dmaengine.h>
> >  #include <linux/iio/adc-helpers.h>
> > +#include <linux/iio/buffer.h>
> >  #include <linux/iio/iio.h>
> > +#include <linux/iio/kfifo_buf.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/io.h>
> >  #include <linux/iopoll.h>
> > +#include <linux/kthread.h>
> >  #include <linux/module.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_runtime.h>
> > @@ -18,6 +23,7 @@
> >  #define RZT2H_ADCSR_ADIE_MASK		BIT(12)
> >  #define RZT2H_ADCSR_ADCS_MASK		GENMASK(14, 13)
> >  #define RZT2H_ADCSR_ADCS_SINGLE		0b00
> > +#define RZT2H_ADCSR_ADCS_CONTINUOUS	0b10
> >  #define RZT2H_ADCSR_ADST_MASK		BIT(15)
> >  
> >  #define RZT2H_ADANSA0_REG		0x04
> > @@ -31,18 +37,47 @@
> >  #define RZT2H_ADCALCTL_CAL_ERR_MASK	BIT(2)
> >  
> >  #define RZT2H_ADC_MAX_CHANNELS		16
> > +#define RZT2H_ADC_CHANNEL_BYTES		sizeof(u16)
> > +#define RZT2H_ADC_DMA_PERIOD_SAMPLES	128
> > +#define RZT2H_ADC_DMA_PERIODS		64
> > +#define RZT2H_ADC_DMA_BUFFER_SAMPLES	(RZT2H_ADC_DMA_PERIODS * \
> > +					 RZT2H_ADC_DMA_PERIOD_SAMPLES)
> > +#define RZT2H_ADC_DMA_BUFFER_SIZE	(RZT2H_ADC_DMA_BUFFER_SAMPLES * \
> > +					 RZT2H_ADC_MAX_CHANNELS * \
> > +					 RZT2H_ADC_CHANNEL_BYTES)
> > +
> > +struct rzt2h_adc_dma {
> > +	struct dma_chan *chan;
> > +	u16 *buf;
> > +	dma_addr_t addr;
> > +
> > +	unsigned int period_index;
> > +	unsigned int period_bytes;
> > +	unsigned int first_chan;
> > +	unsigned int sample_chans;
> > +
> > +	u8 gather[RZT2H_ADC_MAX_CHANNELS];
> > +	unsigned int gather_len;
> > +
> > +	atomic_t pending_periods;
> > +
> > +	wait_queue_head_t wq;
> > +	struct task_struct *thread;
> > +};
> >  
> >  struct rzt2h_adc {
> >  	void __iomem *base;
> >  	struct device *dev;
> >  
> >  	phys_addr_t phys_base;
> > +	struct rzt2h_adc_dma dma;
> >  	struct completion completion;
> >  	/* lock to protect against multiple access to the device */
> >  	struct mutex lock;
> >  
> >  	const struct iio_chan_spec *channels;
> >  	unsigned int num_channels;
> > +	u16 buf[RZT2H_ADC_MAX_CHANNELS];
> >  
> >  	int irq;
> >  };
> > @@ -151,6 +186,253 @@ static int rzt2h_adc_calibrate(struct rzt2h_adc *adc)
> >  	return 0;
> >  }
> >  
> > +static void rzt2h_adc_push_period(struct iio_dev *indio_dev, u16 *period,
> > +				  dma_addr_t addr)
> > +{
> > +	struct rzt2h_adc *adc = iio_priv(indio_dev);
> > +	u16 *dst = adc->buf;
> > +	u16 *src = period;
> > +
> > +	dma_sync_single_for_cpu(adc->dev, addr, adc->dma.period_bytes,
> > +				DMA_FROM_DEVICE);
> > +
> > +	for (unsigned int sample = 0; sample < RZT2H_ADC_DMA_PERIOD_SAMPLES; sample++) {
> > +		for (unsigned int i = 0; i < adc->dma.gather_len; i++)
> > +			dst[i] = src[adc->dma.gather[i]];
> > +
> > +		src += adc->dma.sample_chans;
> > +
> > +		iio_push_to_buffers(indio_dev, adc->buf);
> > +	}  
> 
> 
> 
> > +}
> > +
> > +static void rzt2h_adc_advance_period_index(struct rzt2h_adc *adc, unsigned int i)
> > +{
> > +	adc->dma.period_index += i;
> > +	adc->dma.period_index %= RZT2H_ADC_DMA_PERIODS;
> > +}
> > +
> > +static void rzt2h_adc_dma_thread_loop(struct iio_dev *indio_dev)
> > +{
> > +	struct rzt2h_adc *adc = iio_priv(indio_dev);
> > +	int pending, drop;
> > +	dma_addr_t addr;
> > +	u16 *period;
> > +
> > +	pending = atomic_xchg(&adc->dma.pending_periods, 0);
> > +
> > +	if (pending >= RZT2H_ADC_DMA_PERIODS) {  
> 
> Add a comment here to say what is being dropped and why.
> 
> > +		drop = pending - RZT2H_ADC_DMA_PERIODS + 1;
> > +
> > +		rzt2h_adc_advance_period_index(adc, drop);
> > +		pending -= drop;
> > +	}
> > +
> > +	for (unsigned int i = 0; i < pending; i++) {
> > +		unsigned int backlog = atomic_read(&adc->dma.pending_periods) +
> > +				       pending - i;
> > +
> > +		/*
> > +		 * Bail if enough new periods have completed since reading the
> > +		 * pending_periods that the next period about to be read is at
> > +		 * risk of being overwritten.
> > +		 */
> > +		if (backlog >= RZT2H_ADC_DMA_PERIODS)
> > +			break;  
> 
> 
> > +
> > +		period = adc->dma.buf + adc->dma.period_index *
> > +			 RZT2H_ADC_DMA_PERIOD_SAMPLES * adc->dma.sample_chans;
> > +		addr = adc->dma.addr + adc->dma.period_index *
> > +		       adc->dma.period_bytes;
> > +
> > +		rzt2h_adc_push_period(indio_dev, period, addr);
> > +		rzt2h_adc_advance_period_index(adc, 1);
> > +	}
> > +}
> > +
> > +static int rzt2h_adc_dma_thread(void *data)
> > +{
> > +	struct iio_dev *indio_dev = data;
> > +	struct rzt2h_adc *adc = iio_priv(indio_dev);
> > +
> > +	while (!kthread_should_stop()) {
> > +		wait_event_interruptible(adc->dma.wq,
> > +					 atomic_read(&adc->dma.pending_periods) ||
> > +					 kthread_should_stop());  
> 
> I've not thought that much about the following but it seems plausible so please
> take a look
> 
> 
> > +
> > +		if (kthread_should_stop())
> > +			break;
> > +
> > +		rzt2h_adc_dma_thread_loop(indio_dev);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void rzt2h_adc_dma_callback(void *data)
> > +{
> > +	struct iio_dev *indio_dev = data;
> > +	struct rzt2h_adc *adc = iio_priv(indio_dev);
> > +
> > +	atomic_inc(&adc->dma.pending_periods);
> > +	wake_up(&adc->dma.wq);
> > +}
> > +
> > +static void rzt2h_adc_dma_calc_layout(struct iio_dev *indio_dev)
> > +{
> > +	struct rzt2h_adc *adc = iio_priv(indio_dev);
> > +	unsigned int hi = 0, lo = RZT2H_ADC_MAX_CHANNELS - 1;
> > +	const struct iio_chan_spec *chan;
> > +	unsigned int sample_chans;
> > +	unsigned int first_chan;
> > +	unsigned int scan_index;
> > +	unsigned int swap;
> > +	unsigned int idx;  
> 
> As mentioned at the top, I think you can probably avoid all this complexity.
> Lots of devices have restrictions on combinations of channels that
> are enabled together. For that we have available_scan_masks
> and the demux stuff in the IIO core.
> 
> > +
> > +	/* Find the lowest and highest enabled channel. */
> > +	iio_for_each_active_channel(indio_dev, scan_index) {
> > +		chan = &indio_dev->channels[scan_index];
> > +
> > +		lo = min_t(unsigned int, lo, chan->channel);
> > +		hi = max_t(unsigned int, hi, chan->channel);
> > +	}
> > +
> > +	/*
> > +	 * The DMA has no scatter-gather and transfers must have a power-of-two
> > +	 * width, so pick the smallest power-of-two-aligned block of channels
> > +	 * that covers all enabled channels.
> > +	 */
> > +	for (sample_chans = 1; sample_chans < RZT2H_ADC_MAX_CHANNELS; sample_chans <<= 1) {
> > +		first_chan = round_down(lo, sample_chans);
> > +
> > +		if (first_chan + sample_chans > hi)
> > +			break;
> > +	}
> > +
> > +	/*
> > +	 * Build a table to map each enabled channel to its position in the
> > +	 * transferred block, it will be used later to extract only the enabled
> > +	 * channels out of it.
> > +	 * The DMA moves data in 32-bit words, which swaps each pair of adjacent
> > +	 * 16-bit channels. Undo it.
> > +	 */
> > +	adc->dma.gather_len = 0;
> > +	swap = sample_chans > 1;
> > +	iio_for_each_active_channel(indio_dev, scan_index) {
> > +		chan = &indio_dev->channels[scan_index];
> > +		idx = chan->channel - first_chan;
> > +
> > +		adc->dma.gather[adc->dma.gather_len++] = idx ^ swap;
> > +	}
> > +
> > +	adc->dma.first_chan = first_chan;
> > +	adc->dma.sample_chans = sample_chans;
> > +	adc->dma.period_bytes = RZT2H_ADC_DMA_PERIOD_SAMPLES * sample_chans *
> > +				RZT2H_ADC_CHANNEL_BYTES;
> > +}
> > +
> > +static int rzt2h_adc_start_dma(struct iio_dev *indio_dev)
> > +{
> > +	struct rzt2h_adc *adc = iio_priv(indio_dev);
> > +	struct dma_async_tx_descriptor *desc;
> > +	struct dma_slave_config config = {};
> > +	unsigned int buffer_bytes;
> > +	dma_cookie_t cookie;
> > +	int ret;
> > +
> > +	rzt2h_adc_dma_calc_layout(indio_dev);
> > +
> > +	config.src_addr = adc->phys_base + RZT2H_ADDR_REG(adc->dma.first_chan);
> > +	config.src_addr_width = adc->dma.sample_chans * RZT2H_ADC_CHANNEL_BYTES;  
> 
> 	config = (struct dma_slave_config) {
> 		.src_addr = adc->phys_base + RZT2H_ADDR_REG(adc->dma.first_chan),
> 		.src_addr_width = adc->dma.sample_chans * RZT2H_ADC_CHANNEL_BYTES,
> 	};
> 
> 	keeps all the filling in of info together and avoids need to do = { }
> 	above then overwrite some of the zeroed memory.
> 
> > +
> > +	buffer_bytes = RZT2H_ADC_DMA_PERIODS * adc->dma.period_bytes;
> > +
> > +	ret = dmaengine_slave_config(adc->dma.chan, &config);
> > +	if (ret)
> > +		return ret;
> > +
> > +	desc = dmaengine_prep_dma_cyclic(adc->dma.chan, adc->dma.addr,
> > +					 buffer_bytes, adc->dma.period_bytes,
> > +					 DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT);
> > +	if (!desc)
> > +		return -EBUSY;
> > +
> > +	desc->callback = rzt2h_adc_dma_callback;
> > +	desc->callback_param = indio_dev;
> > +
> > +	cookie = dmaengine_submit(desc);
> > +	ret = dma_submit_error(cookie);
> > +	if (ret) {
> > +		dmaengine_terminate_sync(adc->dma.chan);
> > +		return ret;
> > +	}
> > +
> > +	adc->dma.thread = kthread_run(rzt2h_adc_dma_thread, indio_dev,
> > +				      "rzt2h-adc-dma");
> > +	if (IS_ERR(adc->dma.thread)) {
> > +		dmaengine_terminate_sync(adc->dma.chan);
> > +		return PTR_ERR(adc->dma.thread);
> > +	}
> > +
> > +	disable_irq(adc->irq);
> > +
> > +	dma_async_issue_pending(adc->dma.chan);
> > +
> > +	return 0;
> > +}
> > +  
> 
> > +static int rzt2h_adc_setup_dma(struct iio_dev *indio_dev)
> > +{
> > +	struct rzt2h_adc *adc = iio_priv(indio_dev);
> > +	struct device *dev = adc->dev;
> > +	int ret;
> > +
> > +	adc->dma.chan = devm_dma_request_chan(adc->dev, "rx");
> > +	if (IS_ERR(adc->dma.chan)) {
> > +		ret = PTR_ERR(adc->dma.chan);
> > +		if (ret != -ENODEV)
> > +			return dev_err_probe(adc->dev, ret, "DMA channel request failed\n");  
> 
> You have dev that can be used here.
> 
> > +
> > +		adc->dma.chan = NULL;
> > +		return 0;
> > +	}
> > +
> > +	adc->dma.buf = dma_alloc_noncoherent(adc->dev, RZT2H_ADC_DMA_BUFFER_SIZE,  
> 
> Same here.
>