Re: [PATCH v2 1/8] dmaengine: Add API to combine configuration and preparation (sg and single)

Vinod Koul
2025-12-23T10:41:55Z

On 18-12-25, 10:56, Frank Li wrote:
> Previously, configuration and preparation required two separate calls. This
> works well when configuration is done only once during initialization.
>
> However, in cases where the burst length or source/destination address must
> be adjusted for each transfer, calling two functions is verbose and
> requires additional locking to ensure both steps complete atomically.
>
> Add a new API dmaengine_prep_config_single() and dmaengine_prep_config_sg()
> and callback device_prep_config_sg() that combines configuration and
> preparation into a single operation. If the configuration argument is
> passed as NULL, fall back to the existing implementation.
>
> Add a new API dmaengine_prep_config_single_safe() and
> dmaengine_prep_config_sg_safe() for re-entrancy, which require driver
> implement callback device_prep_config_sg().
>
> Tested-by: Niklas Cassel <cassel@kernel.org>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change in v2
> - add () for function
> - use short name device_prep_sg(), remove "slave" and "config". the 'slave'
> is reduntant. after remove slave, the function name is difference existed
> one, so remove _config suffix.
> ---
> Documentation/driver-api/dmaengine/client.rst | 9 +++
> include/linux/dmaengine.h | 103 ++++++++++++++++++++++++--
> 2 files changed, 105 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
> index d491e385d61a98b8a804cd823caf254a2dc62cf4..02c45b7d7a779421411eb9c68325cdedafcfe3b1 100644
> --- a/Documentation/driver-api/dmaengine/client.rst
> +++ b/Documentation/driver-api/dmaengine/client.rst
> @@ -80,6 +80,10 @@ The details of these operations are:
>
> - slave_sg: DMA a list of scatter gather buffers from/to a peripheral
>
> + - config_sg: Similar with slave_sg, just pass down dma_slave_config
> + struct to avoid call dmaengine_slave_config() every time if need
> + adjust burst length or FIFO address.
> +
> - peripheral_dma_vec: DMA an array of scatter gather buffers from/to a
> peripheral. Similar to slave_sg, but uses an array of dma_vec
> structures instead of a scatterlist.
> @@ -106,6 +110,11 @@ The details of these operations are:
> unsigned int sg_len, enum dma_data_direction direction,
> unsigned long flags);
>
> + struct dma_async_tx_descriptor *dmaengine_prep_config_sg(
> + struct dma_chan *chan, struct scatterlist *sgl,
> + unsigned int sg_len, enum dma_transfer_direction dir,
> + unsigned long flags, struct dma_slave_config *config);
> +
> struct dma_async_tx_descriptor *dmaengine_prep_peripheral_dma_vec(
> struct dma_chan *chan, const struct dma_vec *vecs,
> size_t nents, enum dma_data_direction direction,
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 99efe2b9b4ea9844ca6161208362ef18ef111d96..276dca760f95e1131f5ff5bf69752c4c9cb1bcad 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -835,6 +835,8 @@ struct dma_filter {
> * where the address and size of each segment is located in one entry of
> * the dma_vec array.
> * @device_prep_slave_sg: prepares a slave dma operation
> + * (Deprecated, use @device_prep_config_sg)

Sorry that is _not_ deprecated, we are adding another way to do this in
a single shot

> + * @device_prep_config_sg: prepares a slave DMA operation with dma_slave_config
> * @device_prep_dma_cyclic: prepare a cyclic dma operation suitable for audio.
> * The function takes a buffer of size buf_len. The callback function will
> * be called after period_len bytes have been transferred.
> @@ -934,6 +936,11 @@ struct dma_device {
> struct dma_chan *chan, struct scatterlist *sgl,
> unsigned int sg_len, enum dma_transfer_direction direction,
> unsigned long flags, void *context);
> + struct dma_async_tx_descriptor *(*device_prep_config_sg)(
> + struct dma_chan *chan, struct scatterlist *sgl,
> + unsigned int sg_len, enum dma_transfer_direction direction,
> + unsigned long flags, struct dma_slave_config *config,
> + void *context);
> struct dma_async_tx_descriptor *(*device_prep_dma_cyclic)(
> struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
> size_t period_len, enum dma_transfer_direction direction,
> @@ -974,22 +981,85 @@ static inline bool is_slave_direction(enum dma_transfer_direction direction)
> (direction == DMA_DEV_TO_DEV);
> }
>
> -static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_single(
> - struct dma_chan *chan, dma_addr_t buf, size_t len,
> - enum dma_transfer_direction dir, unsigned long flags)
> +/*
> + * Re-entrancy and locking considerations for callers:
> + *
> + * dmaengine_prep_config_single(sg)_safe() is re-entrant and requires the
> + * DMA engine driver to implement device_prep_config_sg(). It returns NULL
> + * if device_prep_config_sg() is not implemented.
> + *
> + * The unsafe variant (without the _safe suffix) falls back to calling
> + * dmaengine_slave_config() and dmaengine_prep_slave_sg() separately.
> + * In this case, additional locking may be required, depending on the
> + * DMA consumer's usage.
> + */
> +static inline struct dma_async_tx_descriptor *
> +dmaengine_prep_config_sg_safe(struct dma_chan *chan, struct scatterlist *sgl,
> + unsigned int sg_len, enum dma_transfer_direction dir,
> + unsigned long flags, struct dma_slave_config *config)
> +{
> + if (!chan || !chan->device || !chan->device->device_prep_config_sg)
> + return NULL;
> +
> + return chan->device->device_prep_config_sg(chan, sgl, sg_len,
> + dir, flags, config, NULL);
> +}
> +
> +static inline struct dma_async_tx_descriptor *
> +dmaengine_prep_config_single_safe(struct dma_chan *chan, dma_addr_t buf,
> + size_t len, enum dma_transfer_direction dir, unsigned long flags,
> + struct dma_slave_config *config)

Agree with Damien, this could look better!

--
~Vinod