Re: [PATCH] iio: buffer-dmaengine: Add support for cyclic DMA transfers

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 11 Jun 2026 16:28:57 +0100
Nuno Sá via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:

> From: Nuno Sá <nuno.sa@analog.com>
> 
> Allow buffer blocks flagged as cyclic to be submitted as repeating DMA
> transfers. For cyclic blocks, use DMA_PREP_REPEAT so the engine keeps
> replaying the descriptor.
> 
> Skip installing the completion callback for cyclic blocks. Since the
> transfer is continuously replayed, the callback would fire on every
> period, throwing off the block refcount.
> 
> Because nothing prevents a new cyclic transfer from replacing an
> already active cyclic one, always set DMA_PREP_LOAD_EOT so the engine
> correctly terminates the active transfer before loading the new
> descriptor.
> 
I'd like a little more use case stuff in here.  Basically I had
same question David did on when this was useful.

Otherwise, seems fine to me. Sashiko raised a concern about a particular
sequence of blocks being added.  
https://sashiko.dev/#/patchset/20260611-iio-dma-cyclic-buffer-support-v1-1-bcf00e8d802c%40analog.com
I don't really understand this code well enough to be sure if the issue
is real or not!

> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
> There's one subtle choice in here. Given that the termination callback
> is not set. We will never give the block refcount. That means cyclic
> blocks are only completely freed when we disable the buffer and
> iio_dmaengine_buffer_abort() get's called. So no leak, we just defer it
> as it makes it more simple to handle. I also think this a fair
> expectation from a cyclic transfer. We set it up and let it run until we
> disable the buffer.

To me that seems fine

> 
> Alternatively, we can give in the refcount as soon as we give the block
> to the DMA layer with dma_async_issue_pending(). But we also need to
> make sure that the block is not added to the dmaengine_buffer->active list.
> As said, I feel that the current approach is just simpler.
> ---
>  drivers/iio/buffer/industrialio-buffer-dmaengine.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/buffer/industrialio-buffer-dmaengine.c b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> index 98acce909854..4a78cd3e7c7d 100644
> --- a/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> +++ b/drivers/iio/buffer/industrialio-buffer-dmaengine.c
> @@ -80,6 +80,8 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
>  		dma_dir = DMA_MEM_TO_DEV;
>  
>  	if (block->sg_table) {
> +		unsigned long flags;
> +
>  		sgl = block->sg_table->sgl;
>  		nents = sg_nents_for_len(sgl, block->bytes_used);
>  		if (nents < 0)
> @@ -99,9 +101,18 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
>  			sgl = sg_next(sgl);
>  		}
>  
> +		if (block->cyclic)
> +			flags = DMA_PREP_REPEAT;
> +		else
> +			flags = DMA_PREP_INTERRUPT;
> +
> +		/*
> +		 * There's nothing preventing a cyclic transfer to replace an active
> +		 * cyclic one. So always set the EOT flag.
> +		 */
>  		desc = dmaengine_prep_peripheral_dma_vec(dmaengine_buffer->chan,
>  							 vecs, nents, dma_dir,
> -							 DMA_PREP_INTERRUPT);
> +							 flags | DMA_PREP_LOAD_EOT);
>  		kfree(vecs);
>  	} else {
>  		max_size = min(block->size, dmaengine_buffer->max_size);
> @@ -122,8 +133,10 @@ static int iio_dmaengine_buffer_submit_block(struct iio_dma_buffer_queue *queue,
>  	if (!desc)
>  		return -ENOMEM;
>  
> -	desc->callback_result = iio_dmaengine_buffer_block_done;
> -	desc->callback_param = block;
> +	if (!block->cyclic) {
> +		desc->callback_result = iio_dmaengine_buffer_block_done;
> +		desc->callback_param = block;
> +	}
>  
>  	cookie = dmaengine_submit(desc);
>  	if (dma_submit_error(cookie))
> 
> ---
> base-commit: ae696dfa47c30016cd429b9db5e70b259b8f509e
> change-id: 20260609-iio-dma-cyclic-buffer-support-f18034f8f34c
> --
> 
> Thanks!
> - Nuno Sá
> 
>