[PATCH v9 0/2] dmaengine: fsl-edma: Scatter/gather improvements

[PATCH v9 0/2] dmaengine: fsl-edma: Scatter/gather improvements

[Benoît Monin]

This series adds support for scatter/gather DMA transfers via dma_vec
and dynamic descriptor chaining to the Freescale eDMA controller driver.

The first patch implements the .device_prep_peripheral_dma_vec() callback,
enabling the DMA engine to accept an array of dma_vec structures. This
callback supports both regular and cyclic transfer modes.

The second patch introduces dynamic scatter/gather chaining, which allows
multiple DMA descriptors to be linked together without stopping the channel.
This optimization eliminates idle periods when back-to-back transfers are
submitted, improving throughput and reducing latency. The implementation
carefully preserves cyclic transfer semantics and respects hardware
constraints on platforms with split register layouts.

I tested it on the i.MX93. The dynamic scatter/gather chaining should
work with other eDMA controller with split register layout.

Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
Changes in v9:
- Rebased and retested on v7.3-rc1.
- Drop the check on CSR E_LINK flag as it is never set in the driver.
- Add FSL_EDMA_DRV_CSR_LINKCH flag for eDMA3 and eDMA4 to indicate which
  controllers can link SG descriptors.
- Count and limit the number of linked SG descriptors once per
  issue_pending() call, instead of O(n^2).
- Link to v8:
https://patch.msgid.link/20260803-fsl-edma-dyn-sg-v8-0-dc2b0317206d@bootlin.com

Changes in v8:
- Enforce that major channel linking in not used when linking a descriptor
  in fsl_edma_link_sg() by checking all TCD, not just the first one.
- Don't compare the descriptor link_sg_id with the CSR LINKCH field
  if major channel linking is enabled, to avoid completing unfinished
  descriptors in fsl_edma_tx_chan_handler().
- Handle the completion of the last transaction of a linked SG chain if
  the previous interrupt was missed by checking for the channel completion.
- Add comments to clarify the assumptions I made.
- Link to v7: https://patch.msgid.link/20260728-fsl-edma-dyn-sg-v7-0-10dffb4167c2@bootlin.com

Changes in v7:
- In fsl_edma_prep_peripheral_dma_vec(), make sure that CITER/BITER
  values fit in their registers.
- Add the identifier to all TCD of a linked transaction so that we can
  always identify it when handling the end-of-transfer interrupt.
- Handle missed/coalesced end-of-transfer interrupt even when the transfer
  is completed.
- Link to v6: https://patch.msgid.link/20260710-fsl-edma-dyn-sg-v6-0-831b96be3f31@bootlin.com

Changes in v6:
- Link DMA transactions in fsl_edma_issue_pending() when they are issued,
  not when submitted.
- Add an identifier to linked transactions to handle missed/coalesced
  end-of-transfer interrupt.
- Link to v5: https://patch.msgid.link/20260702-fsl-edma-dyn-sg-v5-0-16787185be49@bootlin.com

Changes in v5:
- Rebased on v7.2-rc1.
- Add a call to dma_wmb() to ensure that dlast_sga is updated
  before csr when linking scatter/gather transactions.
- Don't update TCD registers if updating csr requires clearing the
  channel DONE bit to avoid a status mismatch in fsl_edma_tx_chan_handler().
- Link to v4: https://patch.msgid.link/20260518-fsl-edma-dyn-sg-v4-0-8ce7d95b1ce9@bootlin.com

Changes in v4:
- To keep transactions in order, link DMA transaction to the end of
  submitted list first, only lookup the issued list is the submitted
  list is empty.
- Link to v3: https://patch.msgid.link/20260511-fsl-edma-dyn-sg-v3-0-98a181775dae@bootlin.com

Changes in v3:
- Fix formatting errors reported by Frank Li.
- Add fsl_edma_tx_submit() to link the DMA transactions
  when they are submitted, not when they are prepared.
- Link to v2: https://patch.msgid.link/20260506-fsl-edma-dyn-sg-v2-0-66439cdd414e@bootlin.com

Changes in v2:
- Drop the RFC prefix, as asked by Frank Li
- No code change
- Link to v1: https://patch.msgid.link/20260430-fsl-edma-dyn-sg-v1-0-4e0ecbe2df66@bootlin.com

To: Frank Li <Frank.Li@nxp.com>
To: Vinod Koul <vkoul@kernel.org>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Frank Li <Frank.Li@kernel.org>
Cc: imx@lists.linux.dev
Cc: dmaengine@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Benoît Monin (2):
      dmaengine: fsl-edma: Implement device_prep_peripheral_dma_vec
      dmaengine: fsl-edma: Support dynamic scatter/gather chaining

 drivers/dma/fsl-edma-common.c | 254 +++++++++++++++++++++++++++++++++++++++++-
 drivers/dma/fsl-edma-common.h |  34 ++++--
 drivers/dma/fsl-edma-main.c   |   2 +
 drivers/dma/fsl-edma-trace.h  |   5 +
 4 files changed, 277 insertions(+), 18 deletions(-)
---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260428-fsl-edma-dyn-sg-960731e37da2

Best regards,
--  
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v9 1/2] dmaengine: fsl-edma: Implement device_prep_peripheral_dma_vec

[Benoît Monin]

Add implementation of .device_prep_peripheral_dma_vec() callback to setup
a scatter/gather DMA transfer from an array of dma_vec structures. Setup
a cyclic transfer if the DMA_PREP_REPEAT flag is set.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
---
 drivers/dma/fsl-edma-common.c | 116 ++++++++++++++++++++++++++++++++++++++++++
 drivers/dma/fsl-edma-common.h |   4 ++
 drivers/dma/fsl-edma-main.c   |   2 +
 3 files changed, 122 insertions(+)

diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
index bb7531c456df..c5f5951c988b 100644
--- a/drivers/dma/fsl-edma-common.c
+++ b/drivers/dma/fsl-edma-common.c
@@ -673,6 +673,122 @@ struct dma_async_tx_descriptor *fsl_edma_prep_dma_cyclic(
 	return vchan_tx_prep(&fsl_chan->vchan, &fsl_desc->vdesc, flags);
 }
 
+struct dma_async_tx_descriptor *
+fsl_edma_prep_peripheral_dma_vec(struct dma_chan *chan, const struct dma_vec *vecs,
+				 size_t nb, enum dma_transfer_direction direction,
+				 unsigned long flags)
+{
+	struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
+	dma_addr_t src_addr, dst_addr, last_sg;
+	struct fsl_edma_desc *fsl_desc;
+	u16 soff, doff, iter;
+	u32 nbytes;
+	int i;
+
+	if (!is_slave_direction(direction))
+		return NULL;
+
+	if (!fsl_edma_prep_slave_dma(fsl_chan, direction))
+		return NULL;
+
+	fsl_desc = fsl_edma_alloc_desc(fsl_chan, nb);
+	if (!fsl_desc)
+		return NULL;
+	fsl_desc->iscyclic = flags & DMA_PREP_REPEAT;
+	fsl_desc->dirn = direction;
+
+	if (direction == DMA_MEM_TO_DEV) {
+		if (!fsl_chan->cfg.src_addr_width)
+			fsl_chan->cfg.src_addr_width = fsl_chan->cfg.dst_addr_width;
+		fsl_chan->attr =
+			fsl_edma_get_tcd_attr(fsl_chan->cfg.src_addr_width,
+					      fsl_chan->cfg.dst_addr_width);
+		nbytes = fsl_chan->cfg.dst_addr_width * fsl_chan->cfg.dst_maxburst;
+	} else {
+		if (!fsl_chan->cfg.dst_addr_width)
+			fsl_chan->cfg.dst_addr_width = fsl_chan->cfg.src_addr_width;
+		fsl_chan->attr =
+			fsl_edma_get_tcd_attr(fsl_chan->cfg.src_addr_width,
+					      fsl_chan->cfg.dst_addr_width);
+		nbytes = fsl_chan->cfg.src_addr_width * fsl_chan->cfg.src_maxburst;
+	}
+
+	for (i = 0; i < nb; i++) {
+		if (direction == DMA_MEM_TO_DEV) {
+			src_addr = vecs[i].addr;
+			dst_addr = fsl_chan->dma_dev_addr;
+			soff = fsl_chan->cfg.dst_addr_width;
+			doff = 0;
+		} else if (direction == DMA_DEV_TO_MEM) {
+			src_addr = fsl_chan->dma_dev_addr;
+			dst_addr = vecs[i].addr;
+			soff = 0;
+			doff = fsl_chan->cfg.src_addr_width;
+		} else {
+			/* DMA_DEV_TO_DEV */
+			src_addr = fsl_chan->cfg.src_addr;
+			dst_addr = fsl_chan->cfg.dst_addr;
+			soff = 0;
+			doff = 0;
+		}
+
+		/*
+		 * Choose the suitable burst length if dma_vec length is not
+		 * multiple of burst length so that the whole transfer length is
+		 * multiple of minor loop(burst length).
+		 */
+		if (nbytes && vecs[i].len % nbytes) {
+			u32 width = (direction == DMA_DEV_TO_MEM) ? doff : soff;
+			u32 burst = (direction == DMA_DEV_TO_MEM) ?
+						fsl_chan->cfg.src_maxburst :
+						fsl_chan->cfg.dst_maxburst;
+			int j;
+
+			for (j = burst; j > 1; j--) {
+				if (!(vecs[i].len % (j * width))) {
+					nbytes = j * width;
+					break;
+				}
+			}
+			/* Set burst size as 1 if there's no suitable one */
+			if (j == 1)
+				nbytes = width;
+		}
+
+		if (!nbytes || vecs[i].len / nbytes > FIELD_MAX(EDMA_TCD_ITER_MASK))
+			goto err_free_desc;
+
+		iter = vecs[i].len / nbytes;
+		if (i < nb - 1) {
+			last_sg = fsl_desc->tcd[(i + 1)].ptcd;
+			fsl_edma_fill_tcd(fsl_chan, fsl_desc->tcd[i].vtcd, src_addr,
+					  dst_addr, fsl_chan->attr, soff,
+					  nbytes, 0, iter, iter, doff, last_sg,
+					  false, false, true);
+		} else {
+			if (fsl_desc->iscyclic) {
+				last_sg = fsl_desc->tcd[0].ptcd;
+				fsl_edma_fill_tcd(fsl_chan, fsl_desc->tcd[i].vtcd, src_addr,
+						  dst_addr, fsl_chan->attr, soff,
+						  nbytes, 0, iter, iter, doff, last_sg,
+						  true, false, true);
+			} else {
+				last_sg = 0;
+				fsl_edma_fill_tcd(fsl_chan, fsl_desc->tcd[i].vtcd, src_addr,
+						  dst_addr, fsl_chan->attr, soff,
+						  nbytes, 0, iter, iter, doff, last_sg,
+						  true, true, false);
+			}
+		}
+	}
+
+	return vchan_tx_prep(&fsl_chan->vchan, &fsl_desc->vdesc, flags);
+
+err_free_desc:
+	fsl_edma_free_desc(&fsl_desc->vdesc);
+	return NULL;
+}
+
 struct dma_async_tx_descriptor *fsl_edma_prep_slave_sg(
 		struct dma_chan *chan, struct scatterlist *sgl,
 		unsigned int sg_len, enum dma_transfer_direction direction,
diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
index 205a96489094..0d028048701d 100644
--- a/drivers/dma/fsl-edma-common.h
+++ b/drivers/dma/fsl-edma-common.h
@@ -496,6 +496,10 @@ struct dma_async_tx_descriptor *fsl_edma_prep_dma_cyclic(
 		struct dma_chan *chan, dma_addr_t dma_addr, size_t buf_len,
 		size_t period_len, enum dma_transfer_direction direction,
 		unsigned long flags);
+struct dma_async_tx_descriptor *fsl_edma_prep_peripheral_dma_vec(
+		struct dma_chan *chan, const struct dma_vec *vecs,
+		size_t nb, enum dma_transfer_direction direction,
+		unsigned long flags);
 struct dma_async_tx_descriptor *fsl_edma_prep_slave_sg(
 		struct dma_chan *chan, struct scatterlist *sgl,
 		unsigned int sg_len, enum dma_transfer_direction direction,
diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
index d9fb717b5b53..662fe941d084 100644
--- a/drivers/dma/fsl-edma-main.c
+++ b/drivers/dma/fsl-edma-main.c
@@ -843,6 +843,8 @@ static int fsl_edma_probe(struct platform_device *pdev)
 	fsl_edma->dma_dev.device_free_chan_resources
 		= fsl_edma_free_chan_resources;
 	fsl_edma->dma_dev.device_tx_status = fsl_edma_tx_status;
+	fsl_edma->dma_dev.device_prep_peripheral_dma_vec
+		= fsl_edma_prep_peripheral_dma_vec;
 	fsl_edma->dma_dev.device_prep_slave_sg = fsl_edma_prep_slave_sg;
 	fsl_edma->dma_dev.device_prep_dma_cyclic = fsl_edma_prep_dma_cyclic;
 	fsl_edma->dma_dev.device_prep_dma_memcpy = fsl_edma_prep_memcpy;

-- 
2.55.0

Re: [PATCH v9 1/2] dmaengine: fsl-edma: Implement device_prep_peripheral_dma_vec

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Kernel panic due to division by zero when adjusting the burst length for unaligned DMA_DEV_TO_DEV transfers.
- [Critical] Missing parameter validation for `nb == 0` (or `sg_len == 0`) allows the allocation of an empty descriptor, leading to critical out-of-bounds access.
- [Critical] Kernel panic (BUG_ON) triggered in `dma_map_phys` when mapping a DMA_DEV_TO_DEV resource because the driver passes `DMA_NONE` to `dma_map_resource()`.
- [High] The local `nbytes` variable is permanently reduced during vector length alignment, affecting all subsequent vectors.
- [High] Lockless modification of shared channel state during descriptor preparation causes race conditions and resource leaks.
--

commit bc7a9931f6cf0321b75f8853ea3938f42947649d
Author: Benoît Monin <benoit.monin@bootlin.com>

    dmaengine: fsl-edma: Implement device_prep_peripheral_dma_vec
    
    Add implementation of .device_prep_peripheral_dma_vec() callback to setup
    a scatter/gather DMA transfer from an array of dma_vec structures. Setup
    a cyclic transfer if the DMA_PREP_REPEAT flag is set.

> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> index bb7531c456dfa..c5f5951c988be 100644
> --- a/drivers/dma/fsl-edma-common.c
> +++ b/drivers/dma/fsl-edma-common.c
> @@ -673,6 +673,122 @@ struct dma_async_tx_descriptor *fsl_edma_prep_dma_cyclic(
>  	return vchan_tx_prep(&fsl_chan->vchan, &fsl_desc->vdesc, flags);
>  }
>  
> +struct dma_async_tx_descriptor *
> +fsl_edma_prep_peripheral_dma_vec(struct dma_chan *chan, const struct dma_vec *vecs,
> +				 size_t nb, enum dma_transfer_direction direction,
> +				 unsigned long flags)
> +{
> +	struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> +	dma_addr_t src_addr, dst_addr, last_sg;
> +	struct fsl_edma_desc *fsl_desc;
> +	u16 soff, doff, iter;
> +	u32 nbytes;
> +	int i;
> +
> +	if (!is_slave_direction(direction))
> +		return NULL;
> +
> +	if (!fsl_edma_prep_slave_dma(fsl_chan, direction))
> +		return NULL;

[Severity: Critical]
This is a pre-existing issue, but if the direction is DMA_DEV_TO_DEV, does
fsl_edma_prep_slave_dma() eventually pass DMA_NONE to dma_map_resource()?

drivers/dma/fsl-edma-common.c:fsl_edma_prep_slave_dma() {
    ...
    switch (dir) {
    ...
    default:
        dma_dir = DMA_NONE;
        break;
    }
    ...
    fsl_chan->dma_dev_addr = dma_map_resource(dev, addr, size, dma_dir, 0);
    ...
}

Will passing DMA_NONE to dma_map_resource() trigger a BUG_ON() panic in the
core DMA mapping subsystem?

> +
> +	fsl_desc = fsl_edma_alloc_desc(fsl_chan, nb);
> +	if (!fsl_desc)
> +		return NULL;

[Severity: Critical]
This is a pre-existing issue, but does this allow the creation of an empty
descriptor if nb == 0?

If nb == 0, a descriptor with a 0-length flexible array is allocated.
When the DMA engine later issues the transfer, for example in
fsl_edma_xfer_desc():

    fsl_edma_set_tcd_regs(fsl_chan, fsl_chan->edesc->tcd[0].vtcd);

could this unconditionally access the nonexistent zeroth element tcd[0],
leading to out-of-bounds access?

> +	fsl_desc->iscyclic = flags & DMA_PREP_REPEAT;
> +	fsl_desc->dirn = direction;
> +
> +	if (direction == DMA_MEM_TO_DEV) {
> +		if (!fsl_chan->cfg.src_addr_width)
> +			fsl_chan->cfg.src_addr_width = fsl_chan->cfg.dst_addr_width;
> +		fsl_chan->attr =
> +			fsl_edma_get_tcd_attr(fsl_chan->cfg.src_addr_width,
> +					      fsl_chan->cfg.dst_addr_width);

[Severity: High]
This isn't a bug introduced by this patch, but does this code modify the
shared channel state fsl_chan->cfg.src_addr_width without holding the channel
lock?

If multiple threads concurrently call fsl_edma_prep_peripheral_dma_vec() on
the same channel, could they race when mutating fsl_chan->cfg.src_addr_width
and when calling dma_map_resource() in fsl_edma_prep_slave_dma() earlier,
causing a resource leak or use-after-free of active hardware mappings?

[ ... ]

> +		} else {
> +			/* DMA_DEV_TO_DEV */
> +			src_addr = fsl_chan->cfg.src_addr;
> +			dst_addr = fsl_chan->cfg.dst_addr;
> +			soff = 0;
> +			doff = 0;
> +		}
> +
> +		/*
> +		 * Choose the suitable burst length if dma_vec length is not
> +		 * multiple of burst length so that the whole transfer length is
> +		 * multiple of minor loop(burst length).
> +		 */
> +		if (nbytes && vecs[i].len % nbytes) {
> +			u32 width = (direction == DMA_DEV_TO_MEM) ? doff : soff;
> +			u32 burst = (direction == DMA_DEV_TO_MEM) ?
> +						fsl_chan->cfg.src_maxburst :
> +						fsl_chan->cfg.dst_maxburst;
> +			int j;
> +
> +			for (j = burst; j > 1; j--) {
> +				if (!(vecs[i].len % (j * width))) {

[Severity: Critical]
This is a pre-existing issue, but if the transfer direction is DMA_DEV_TO_DEV,
both soff and doff are 0. Since width evaluates to 0, does the modulo
operation vecs[i].len % (j * width) cause a division by zero and kernel panic?

> +					nbytes = j * width;

[Severity: High]
This is a pre-existing issue, but does setting nbytes = j * width
permanently reduce nbytes for all subsequent vectors in the loop?

If the transfer contains an unaligned vector followed by a very large vector,
the first unaligned vector permanently downgrades nbytes for the rest of the
loop since it is never reset to its original value. Could this cause the
iteration count iter to exceed hardware limits FIELD_MAX(EDMA_TCD_ITER_MASK),
rejecting valid transfers?

> +					break;
> +				}
> +			}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-fsl-edma-dyn-sg-v9-0-60bb2e678e8f@bootlin.com?part=1

Re: [PATCH v9 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining

[Frank Li]

On Wed, Sep 09, 2026 at 05:19:46PM +0200, Benoît Monin wrote:
> Implement dynamic linking of scatter/gather transfers to enable
> chaining multiple DMA descriptors without stopping the channel.
> This avoids waiting for the channel to go idle if there is another
> transaction already issued.
>
> Add fsl_edma_link_sg() to dynamically link the last TCD of a previously
> issued descriptor to the first TCD of a new descriptor by setting the
> scatter/gather address and the E_SG flag, and keeping the channel active
> by clearing the DREQ bit.
>
> Also in fsl_edma_link_sg(), assign a non-zero identifier to the new
> descriptor that is stored in the EDMA_TCD_CSR_LINKCH field of the CSR
> of each TCD, after checking that the descriptors are not using channel
> linking. The use of this field (MAJORLINKCH in the datasheet) as an
> identifier for dynamic scatter/gather is suggested in the i.MX93 datasheet.
>
> When the last issued descriptor is the one currently active on the
> channel and has a single TCD, the scatter/gather address and CSR are
> also written directly to the hardware registers to ensure the link
> takes effect immediately, unless the DMA controller requires the DONE
> bit to be cleared for the CSR change to take effect. Clearing the DONE
> bit would disrupt the interrupt handler.
>
> Linking is done in fsl_edma_issue_pending(), which iterates over the
> submitted descriptors, links each one to the previously issued
> descriptor via fsl_edma_link_sg(), and then moves it to the issued
> list. This ensures that transactions are linked in the order they were
> issued. Linking of a descriptor is limited to 31 outstanding descriptors
> on the issued list so the identifier fits in the LINKCH field of the CSR
> register. The value of zero is left to identify non-linked descriptors.
>
> Update fsl_edma_xfer_desc() to avoid re-initializing the hardware when a
> transfer is already in progress, allowing seamless chaining of descriptors.
>
> Modify the transfer completion handler to check the DONE flag in the
> channel CSR before marking the transfer complete. Since this flag is only
> available on SoC with the split registers layout, only link transactions
> for DMA controllers flagged with newly added FSL_EDMA_DRV_CSR_LINKCH. This
> flag is set for eDMA3 and EDMA4 controllers which are able to perform
> dynamic scatter/gather linking.
>
> The completion handler also reaps issued descriptors whose link channel ID
> (EDMA_TCD_CSR_LINKCH) has already been passed by the hardware, marking
> them as completed even if their corresponding interrupt has been missed.
>
> Add trace event for scatter/gather linking operations.
>
> Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
> ---
>  drivers/dma/fsl-edma-common.c | 138 ++++++++++++++++++++++++++++++++++++++++--
>  drivers/dma/fsl-edma-common.h |  30 +++++----
>  drivers/dma/fsl-edma-trace.h  |   5 ++
>  3 files changed, 155 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> index c5f5951c988b..76fbd2bc5154 100644
> --- a/drivers/dma/fsl-edma-common.c
> +++ b/drivers/dma/fsl-edma-common.c
> @@ -55,10 +55,36 @@ void fsl_edma_tx_chan_handler(struct fsl_edma_chan *fsl_chan)
>  	}
>
>  	if (!fsl_chan->edesc->iscyclic) {
> -		list_del(&fsl_chan->edesc->vdesc.node);
> -		vchan_cookie_complete(&fsl_chan->edesc->vdesc);
> +		u16 csr = edma_read_tcdreg(fsl_chan, csr);
> +		u8 link_sg_id = FIELD_GET(EDMA_TCD_CSR_LINKCH, csr);
> +		struct virt_dma_desc *vdesc, *tmp;
> +
> +		/* Channel is DONE when a TCD with D_REQ set completes */
> +		if (!(fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_SPLIT_REG) ||
> +		    (edma_readl_chreg(fsl_chan, ch_csr) & EDMA_V3_CH_CSR_DONE)) {
> +			fsl_chan->status = DMA_COMPLETE;
> +		}
> +
> +		list_for_each_entry_safe(vdesc, tmp, &fsl_chan->vchan.desc_issued, node) {
> +			struct fsl_edma_desc *fsl_desc = to_fsl_edma_desc(vdesc);
> +			bool id_match = (link_sg_id == fsl_desc->link_sg_id);
> +
> +			/*
> +			 * If the transfer is still running,
> +			 * don't mark as complete the current descriptor
> +			 */
> +			if (id_match && fsl_chan->status != DMA_COMPLETE)
> +				break;
> +
> +			list_del(&vdesc->node);
> +			vchan_cookie_complete(vdesc);
> +
> +			if (id_match)
> +				break;
> +		}
> +
>  		fsl_chan->edesc = NULL;
> -		fsl_chan->status = DMA_COMPLETE;
> +
>  	} else {
>  		vchan_cyclic_callback(&fsl_chan->edesc->vdesc);
>  	}
> @@ -931,14 +957,99 @@ void fsl_edma_xfer_desc(struct fsl_edma_chan *fsl_chan)
>  	if (!vdesc)
>  		return;
>  	fsl_chan->edesc = to_fsl_edma_desc(vdesc);
> -	fsl_edma_set_tcd_regs(fsl_chan, fsl_chan->edesc->tcd[0].vtcd);
> -	fsl_edma_enable_request(fsl_chan);
> -	fsl_chan->status = DMA_IN_PROGRESS;
> +
> +	if (fsl_chan->status != DMA_IN_PROGRESS) {
> +		fsl_edma_set_tcd_regs(fsl_chan, fsl_chan->edesc->tcd[0].vtcd);
> +		fsl_edma_enable_request(fsl_chan);
> +		fsl_chan->status = DMA_IN_PROGRESS;
> +	}
> +}
> +
> +static void fsl_edma_link_sg(struct fsl_edma_chan *fsl_chan, struct fsl_edma_desc *fsl_desc)
> +{
> +	u32 flags = fsl_edma_drvflags(fsl_chan);
> +	struct fsl_edma_hw_tcd *last_tcd;
> +	struct fsl_edma_desc *prev_desc;
> +	struct virt_dma_desc *vdesc;
> +	u16 last_csr;
> +
> +	lockdep_assert_held(&fsl_chan->vchan.lock);
> +
> +	if (!(flags & FSL_EDMA_DRV_CSR_LINKCH) || fsl_desc->iscyclic)
> +		return;
> +
> +	vdesc = list_last_entry_or_null(&fsl_chan->vchan.desc_issued,
> +					struct virt_dma_desc, node);
> +	if (!vdesc)
> +		return;
> +
> +	prev_desc = to_fsl_edma_desc(vdesc);
> +	if (prev_desc->iscyclic)
> +		return;
> +
> +	last_tcd = prev_desc->tcd[prev_desc->n_tcds - 1].vtcd;
> +	last_csr = fsl_edma_get_tcd_to_cpu(fsl_chan, last_tcd, csr);
> +
> +	for (unsigned int i = 0; i < fsl_desc->n_tcds; i++) {
> +		struct fsl_edma_hw_tcd *tcd = fsl_desc->tcd[i].vtcd;
> +
> +		if (fsl_edma_get_tcd_to_cpu(fsl_chan, tcd, csr) & EDMA_TCD_CSR_E_LINK)
> +			return;
> +	}
> +
> +	if (!(last_csr & EDMA_TCD_CSR_D_REQ) ||
> +	    last_csr & EDMA_TCD_CSR_E_LINK)
> +		return;
> +
> +	/* Set a non-zero linked SG identifier to all TCD of the new descriptor */
> +	fsl_chan->link_sg_id++;
> +	if (fsl_chan->link_sg_id > FIELD_MAX(EDMA_TCD_CSR_LINKCH))
> +		fsl_chan->link_sg_id = 1;
> +
> +	fsl_desc->link_sg_id = fsl_chan->link_sg_id;
> +
> +	for (unsigned int i = 0; i < fsl_desc->n_tcds; i++) {
> +		struct fsl_edma_hw_tcd *tcd = fsl_desc->tcd[i].vtcd;
> +		u16 csr = fsl_edma_get_tcd_to_cpu(fsl_chan, tcd, csr);
> +
> +		csr |= FIELD_PREP(EDMA_TCD_CSR_LINKCH, fsl_chan->link_sg_id);
> +		fsl_edma_set_tcd_to_le(fsl_chan, tcd, csr, csr);
> +	}
> +
> +	/*
> +	 * Set DLAST_SGA before enabling E_SG in CSR: if the DMA engine
> +	 * only picks up the former, it updates DADDR at end of transfer,
> +	 * which is not reused.
> +	 */
> +	fsl_edma_set_tcd_to_le(fsl_chan, last_tcd, fsl_desc->tcd[0].ptcd, dlast_sga);
> +
> +	dma_wmb();
> +
> +	last_csr &= ~EDMA_TCD_CSR_D_REQ;
> +	last_csr |= EDMA_TCD_CSR_E_SG;
> +	fsl_edma_set_tcd_to_le(fsl_chan, last_tcd, last_csr, csr);
> +
> +	/*
> +	 * Update the registers if the engine already loaded the TCD,
> +	 * unless doing so requires clearing the DONE bit, as that could
> +	 * lead to lost completion.
> +	 */
> +	if (prev_desc == fsl_chan->edesc &&
> +	    prev_desc->n_tcds == 1 &&
> +	    !(flags & FSL_EDMA_DRV_CLEAR_DONE_E_SG)) {
> +		edma_cp_tcd_to_reg(fsl_chan, last_tcd, dlast_sga);
> +		edma_cp_tcd_to_reg(fsl_chan, last_tcd, csr);
> +	}
> +
> +	trace_edma_link_sg(fsl_chan, last_tcd);
>  }
>
>  void fsl_edma_issue_pending(struct dma_chan *chan)
>  {
>  	struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> +	struct virt_dma_desc *vdesc, *tmp;
> +	size_t issued_count = 0;
> +	struct list_head *pos;
>  	unsigned long flags;
>
>  	spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
> @@ -949,6 +1060,21 @@ void fsl_edma_issue_pending(struct dma_chan *chan)
>  		return;
>  	}
>
> +	/* Count the issued desc up to the maximum number of linked SG id */
> +	list_for_each(pos, &fsl_chan->vchan.desc_issued) {
> +		if (++issued_count > FIELD_MAX(EDMA_TCD_CSR_LINKCH))
> +			break;
> +	}
> +
> +	/* Link the SG descriptors with the available identifiers */
> +	list_for_each_entry_safe(vdesc, tmp, &fsl_chan->vchan.desc_submitted, node) {
> +		if (++issued_count > FIELD_MAX(EDMA_TCD_CSR_LINKCH))
> +			break;
> +		fsl_edma_link_sg(fsl_chan, to_fsl_edma_desc(vdesc));
> +		list_move_tail(&vdesc->node, &fsl_chan->vchan.desc_issued);

If this happen, it will depend on DONE irq to queue left desc_issued to
hardware tcd? So there are idle gap?

I think it is fine, less possible longer than 31. I just want to confirm
if my understand is correct.

> +	}
> +
> +	/* Issue the rest of the descriptors unlinked */
>  	if (vchan_issue_pending(&fsl_chan->vchan) && !fsl_chan->edesc)
>  		fsl_edma_xfer_desc(fsl_chan);
>
> diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
> index 0d028048701d..deb172f5a4db 100644
> --- a/drivers/dma/fsl-edma-common.h
> +++ b/drivers/dma/fsl-edma-common.h
> @@ -42,6 +42,7 @@
>  #define EDMA_TCD_CSR_E_LINK		BIT(5)
>  #define EDMA_TCD_CSR_ACTIVE		BIT(6)
>  #define EDMA_TCD_CSR_DONE		BIT(7)
> +#define EDMA_TCD_CSR_LINKCH		GENMASK(12, 8)
>
>  #define EDMA_V3_TCD_NBYTES_MLOFF_NBYTES(x) ((x) & GENMASK(9, 0))
>  #define EDMA_V3_TCD_NBYTES_MLOFF(x)        (x << 10)
> @@ -169,6 +170,7 @@ struct fsl_edma_chan {
>  	struct dma_slave_config		cfg;
>  	u32				attr;
>  	bool                            is_sw;
> +	u8				link_sg_id;
>  	struct dma_pool			*tcd_pool;
>  	dma_addr_t	/fsl_edma_xfer_desc		dma_dev_addr;
>  	u32				dma_dev_size;
> @@ -199,6 +201,7 @@ struct fsl_edma_desc {
>  	struct virt_dma_desc		vdesc;
>  	struct fsl_edma_chan		*echan;
>  	bool				iscyclic;
> +	u8				link_sg_id;
>  	enum dma_transfer_direction	dirn;
>  	unsigned int			n_tcds;
>  	struct fsl_edma_sw_tcd		tcd[];
> @@ -225,20 +228,23 @@ struct fsl_edma_desc {
>  #define FSL_EDMA_DRV_TCD64		BIT(15)
>  /* All channel ERR IRQ share one IRQ line */
>  #define FSL_EDMA_DRV_ERRIRQ_SHARE       BIT(16)
> +/* Major link channel in CSR used for linking SG descriptors */
> +#define FSL_EDMA_DRV_CSR_LINKCH		BIT(17)
>
> +#define FSL_EDMA_DRV_EDMA3	(FSL_EDMA_DRV_SPLIT_REG |		\
> +				 FSL_EDMA_DRV_BUS_8BYTE |		\
> +				 FSL_EDMA_DRV_DEV_TO_DEV |		\
> +				 FSL_EDMA_DRV_ALIGN_64BYTE |		\
> +				 FSL_EDMA_DRV_CLEAR_DONE_E_SG |		\
> +				 FSL_EDMA_DRV_CLEAR_DONE_E_LINK |	\
> +				 FSL_EDMA_DRV_CSR_LINKCH)
>
> -#define FSL_EDMA_DRV_EDMA3	(FSL_EDMA_DRV_SPLIT_REG |	\
> -				 FSL_EDMA_DRV_BUS_8BYTE |	\
> -				 FSL_EDMA_DRV_DEV_TO_DEV |	\
> -				 FSL_EDMA_DRV_ALIGN_64BYTE |	\
> -				 FSL_EDMA_DRV_CLEAR_DONE_E_SG |	\
> -				 FSL_EDMA_DRV_CLEAR_DONE_E_LINK)
> -
> -#define FSL_EDMA_DRV_EDMA4	(FSL_EDMA_DRV_SPLIT_REG |	\
> -				 FSL_EDMA_DRV_BUS_8BYTE |	\
> -				 FSL_EDMA_DRV_DEV_TO_DEV |	\
> -				 FSL_EDMA_DRV_ALIGN_64BYTE |	\
> -				 FSL_EDMA_DRV_CLEAR_DONE_E_LINK)
> +#define FSL_EDMA_DRV_EDMA4	(FSL_EDMA_DRV_SPLIT_REG |		\
> +				 FSL_EDMA_DRV_BUS_8BYTE |		\
> +				 FSL_EDMA_DRV_DEV_TO_DEV |		\
> +				 FSL_EDMA_DRV_ALIGN_64BYTE |		\
> +				 FSL_EDMA_DRV_CLEAR_DONE_E_LINK |	\
> +				 FSL_EDMA_DRV_CSR_LINKCH)

Can you avoid move \ in this patch to reduce difference.

Frank
>
>  struct fsl_edma_drvdata {
>  	u32			dmamuxs; /* only used before v3 */
> diff --git a/drivers/dma/fsl-edma-trace.h b/drivers/dma/fsl-edma-trace.h
> index 45d964a3726d..f4b0021d2bd0 100644
> --- a/drivers/dma/fsl-edma-trace.h
> +++ b/drivers/dma/fsl-edma-trace.h
> @@ -121,6 +121,11 @@ DEFINE_EVENT(edma_log_tcd, edma_fill_tcd,
>  	TP_ARGS(chan, tcd)
>  );
>
> +DEFINE_EVENT(edma_log_tcd, edma_link_sg,
> +	     TP_PROTO(struct fsl_edma_chan *chan, void *tcd),
> +	     TP_ARGS(chan, tcd)
> +);
> +
>  #endif
>
>  /* this part must be outside header guard */
>
> --
> 2.55.0
>

Re: [PATCH v9 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining

[Benoît Monin]

On Friday, 11 September 2026 at 00:08:41 CEST, Frank Li wrote:
> > +      /* Link the SG descriptors with the available identifiers */
> > +      list_for_each_entry_safe(vdesc, tmp, &fsl_chan->vchan.desc_submitted, node) {
> > +              if (++issued_count > FIELD_MAX(EDMA_TCD_CSR_LINKCH))
> > +                      break;
> > +              fsl_edma_link_sg(fsl_chan, to_fsl_edma_desc(vdesc));
> > +              list_move_tail(&vdesc->node, &fsl_chan->vchan.desc_issued);
> 
> If this happen, it will depend on DONE irq to queue left desc_issued to
> hardware tcd? So there are idle gap?
> 
> I think it is fine, less possible longer than 31. I just want to confirm
> if my understand is correct.
> 
Yes, only 31 descriptors get linked at most, to make sure that we never get
the same link_sg_id twice in the issued queue. So we always know which
descriptor to handle in the end of transfer interrupt.

If we have more descriptors, their last TCD keep the D_REQ flag, and they
are handled one by one as we are doing today with a "stop-and-start" of the
channel.

[...]
> > +#define FSL_EDMA_DRV_EDMA3    (FSL_EDMA_DRV_SPLIT_REG |               \
> > +                               FSL_EDMA_DRV_BUS_8BYTE |               \
> > +                               FSL_EDMA_DRV_DEV_TO_DEV |              \
> > +                               FSL_EDMA_DRV_ALIGN_64BYTE |            \
> > +                               FSL_EDMA_DRV_CLEAR_DONE_E_SG |         \
> > +                               FSL_EDMA_DRV_CLEAR_DONE_E_LINK |       \
> > +                               FSL_EDMA_DRV_CSR_LINKCH)
> >
> > -#define FSL_EDMA_DRV_EDMA3    (FSL_EDMA_DRV_SPLIT_REG |       \
> > -                               FSL_EDMA_DRV_BUS_8BYTE |       \
> > -                               FSL_EDMA_DRV_DEV_TO_DEV |      \
> > -                               FSL_EDMA_DRV_ALIGN_64BYTE |    \
> > -                               FSL_EDMA_DRV_CLEAR_DONE_E_SG | \
> > -                               FSL_EDMA_DRV_CLEAR_DONE_E_LINK)
> > -
> > -#define FSL_EDMA_DRV_EDMA4    (FSL_EDMA_DRV_SPLIT_REG |       \
> > -                               FSL_EDMA_DRV_BUS_8BYTE |       \
> > -                               FSL_EDMA_DRV_DEV_TO_DEV |      \
> > -                               FSL_EDMA_DRV_ALIGN_64BYTE |    \
> > -                               FSL_EDMA_DRV_CLEAR_DONE_E_LINK)
> > +#define FSL_EDMA_DRV_EDMA4    (FSL_EDMA_DRV_SPLIT_REG |               \
> > +                               FSL_EDMA_DRV_BUS_8BYTE |               \
> > +                               FSL_EDMA_DRV_DEV_TO_DEV |              \
> > +                               FSL_EDMA_DRV_ALIGN_64BYTE |            \
> > +                               FSL_EDMA_DRV_CLEAR_DONE_E_LINK |       \
> > +                               FSL_EDMA_DRV_CSR_LINKCH)
> 
> Can you avoid move \ in this patch to reduce difference.
> 
Ok, I will revert that change and resend.

Best regards,
-- 
Benoît