* [PATCH v8 0/2] dmaengine: fsl-edma: Scatter/gather improvements
@ 2026-08-03 16:01 Benoît Monin
2026-08-03 16:02 ` [PATCH v8 1/2] dmaengine: fsl-edma: Implement device_prep_peripheral_dma_vec Benoît Monin
2026-08-03 16:02 ` [PATCH v8 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining Benoît Monin
0 siblings, 2 replies; 7+ messages in thread
From: Benoît Monin @ 2026-08-03 16:01 UTC (permalink / raw)
To: Frank Li, Vinod Koul
Cc: Thomas Petazzoni, Frank Li, imx, dmaengine, linux-kernel,
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 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 | 241 ++++++++++++++++++++++++++++++++++++++++--
drivers/dma/fsl-edma-common.h | 7 ++
drivers/dma/fsl-edma-main.c | 2 +
drivers/dma/fsl-edma-trace.h | 5 +
4 files changed, 248 insertions(+), 7 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260428-fsl-edma-dyn-sg-960731e37da2
Best regards,
--
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v8 1/2] dmaengine: fsl-edma: Implement device_prep_peripheral_dma_vec
2026-08-03 16:01 [PATCH v8 0/2] dmaengine: fsl-edma: Scatter/gather improvements Benoît Monin
@ 2026-08-03 16:02 ` Benoît Monin
2026-08-03 16:23 ` sashiko-bot
2026-08-03 16:02 ` [PATCH v8 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining Benoît Monin
1 sibling, 1 reply; 7+ messages in thread
From: Benoît Monin @ 2026-08-03 16:02 UTC (permalink / raw)
To: Frank Li, Vinod Koul
Cc: Thomas Petazzoni, Frank Li, imx, dmaengine, linux-kernel,
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 36155ab1602a..6693b4270a1a 100644
--- a/drivers/dma/fsl-edma-main.c
+++ b/drivers/dma/fsl-edma-main.c
@@ -841,6 +841,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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v8 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining
2026-08-03 16:01 [PATCH v8 0/2] dmaengine: fsl-edma: Scatter/gather improvements Benoît Monin
2026-08-03 16:02 ` [PATCH v8 1/2] dmaengine: fsl-edma: Implement device_prep_peripheral_dma_vec Benoît Monin
@ 2026-08-03 16:02 ` Benoît Monin
2026-08-03 16:28 ` sashiko-bot
2026-08-04 21:47 ` Frank Li
1 sibling, 2 replies; 7+ messages in thread
From: Benoît Monin @ 2026-08-03 16:02 UTC (permalink / raw)
To: Frank Li, Vinod Koul
Cc: Thomas Petazzoni, Frank Li, imx, dmaengine, linux-kernel,
Benoît Monin
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(), we 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, we only link
transactions for DMA controllers flagged with FSL_EDMA_DRV_SPLIT_REG.
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 | 125 +++++++++++++++++++++++++++++++++++++++---
drivers/dma/fsl-edma-common.h | 3 +
drivers/dma/fsl-edma-trace.h | 5 ++
3 files changed, 126 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
index c5f5951c988b..189eb9d1269e 100644
--- a/drivers/dma/fsl-edma-common.c
+++ b/drivers/dma/fsl-edma-common.c
@@ -55,10 +55,37 @@ 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);
+ bool e_link = FIELD_GET(EDMA_TCD_CSR_E_LINK, 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 ((e_link || id_match) && fsl_chan->status != DMA_COMPLETE)
+ break;
+
+ list_del(&vdesc->node);
+ vchan_cookie_complete(vdesc);
+
+ if (e_link || id_match)
+ break;
+ }
+
fsl_chan->edesc = NULL;
- fsl_chan->status = DMA_COMPLETE;
+
} else {
vchan_cyclic_callback(&fsl_chan->edesc->vdesc);
}
@@ -931,14 +958,93 @@ 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_SPLIT_REG) || 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 ||
+ list_count_nodes(&fsl_chan->vchan.desc_issued) >= FIELD_MAX(EDMA_TCD_CSR_LINKCH))
+ 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);
+
+ 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;
unsigned long flags;
spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
@@ -949,7 +1055,12 @@ void fsl_edma_issue_pending(struct dma_chan *chan)
return;
}
- if (vchan_issue_pending(&fsl_chan->vchan) && !fsl_chan->edesc)
+ list_for_each_entry_safe(vdesc, tmp, &fsl_chan->vchan.desc_submitted, node) {
+ fsl_edma_link_sg(fsl_chan, to_fsl_edma_desc(vdesc));
+ list_move_tail(&vdesc->node, &fsl_chan->vchan.desc_issued);
+ }
+
+ if (!list_empty(&fsl_chan->vchan.desc_issued) && !fsl_chan->edesc)
fsl_edma_xfer_desc(fsl_chan);
spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags);
diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
index 0d028048701d..ab7ec43f93cf 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 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[];
diff --git a/drivers/dma/fsl-edma-trace.h b/drivers/dma/fsl-edma-trace.h
index d3541301a247..ac319d2dbb90 100644
--- a/drivers/dma/fsl-edma-trace.h
+++ b/drivers/dma/fsl-edma-trace.h
@@ -119,6 +119,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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v8 1/2] dmaengine: fsl-edma: Implement device_prep_peripheral_dma_vec
2026-08-03 16:02 ` [PATCH v8 1/2] dmaengine: fsl-edma: Implement device_prep_peripheral_dma_vec Benoît Monin
@ 2026-08-03 16:23 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-08-03 16:23 UTC (permalink / raw)
To: Benoît Monin; +Cc: dmaengine, Frank.Li, vkoul, imx
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [High] Out-of-bounds memory access when the caller passes a 0-length vector array.
- [High] Division by zero kernel panic when processing unaligned DMA_DEV_TO_DEV transfers.
- [High] BUG_ON kernel panic when mapping a resource for DMA_DEV_TO_DEV transfers on a reused channel.
- [High] Silent truncation of unaligned DMA transfers leads to data corruption.
- [Medium] State leakage of the burst size (`nbytes`) reduces performance and can cause transfer rejection for subsequent vectors.
--
--- Patch [1]: [PATCH v8 1/2] dmaengine: fsl-edma: Implement device_prep_peripheral_dma_vec ---
commit eeea1310bfd4138f851a3a95d72a197b35c5d579
Author: Benoît Monin <benoit.monin@bootlin.com>
dmaengine: fsl-edma: Implement device_prep_peripheral_dma_vec
This patch adds the .device_prep_peripheral_dma_vec() callback to the Freescale
eDMA driver. It sets up a scatter/gather DMA transfer from an array of dma_vec
structures and supports cyclic transfers when DMA_PREP_REPEAT 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: High]
This is a pre-existing issue, but if this channel is reused and reconfigured
from DMA_MEM_TO_DEV to DMA_DEV_TO_DEV, fsl_edma_prep_slave_dma() will evaluate
dma_dir as DMA_NONE. This causes it to bypass the early return, unmap the old
resource, and call dma_map_resource() with DMA_NONE:
fsl_edma_prep_slave_dma()
dma_dir = DMA_NONE;
...
fsl_chan->dma_dev_addr = dma_map_resource(dev, addr, size, dma_dir, 0);
Since DMA_NONE is not a valid mapping direction, does this trigger the
BUG_ON(!valid_dma_direction(dir)) in the DMA mapping core and crash the kernel?
> +
> + fsl_desc = fsl_edma_alloc_desc(fsl_chan, nb);
[Severity: High]
This isn't a bug introduced by this patch, but if nb is 0, does
fsl_edma_alloc_desc() allocate a descriptor with a 0-length tcd flexible
array?
If so, later when the hardware transfer is initiated in fsl_edma_xfer_desc(),
would the unconditional access to fsl_chan->edesc->tcd[0].vtcd cause an
out-of-bounds memory access?
> + if (!fsl_desc)
> + return NULL;
[ ... ]
> + /*
> + * 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: High]
This is a pre-existing issue, but for DMA_DEV_TO_DEV transfers, both the
source offset (soff) and destination offset (doff) are initialized to 0,
resulting in width being assigned 0.
Does this check evaluate to a modulo-by-zero operation and cause a kernel
panic?
> + nbytes = j * width;
[Severity: Medium]
This isn't a bug introduced by this patch, but nbytes is modified here for an
unaligned vector and never restored to its optimal maximum value at the start
of the next iteration.
Could this cause subsequent aligned vectors to incorrectly inherit this
reduced burst size, potentially degrading performance or causing the transfer
to be rejected if the iteration count exceeds the hardware maximum?
> + 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;
[Severity: High]
This is a pre-existing issue, but if a vector's length cannot be perfectly
divided by any suitable burst size, nbytes is set to width.
When iter is calculated using integer division here, are any remaining
unaligned bytes silently discarded, causing the hardware to transfer fewer
bytes than requested and leading to silent data corruption?
> + if (i < nb - 1) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-fsl-edma-dyn-sg-v8-0-dc2b0317206d@bootlin.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v8 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining
2026-08-03 16:02 ` [PATCH v8 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining Benoît Monin
@ 2026-08-03 16:28 ` sashiko-bot
2026-08-04 22:06 ` Frank Li
2026-08-04 21:47 ` Frank Li
1 sibling, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-08-03 16:28 UTC (permalink / raw)
To: Benoît Monin; +Cc: Frank.Li, vkoul, imx, dmaengine
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] O(N^2) algorithmic complexity inside a spinlock during scatter-gather descriptor submission can cause severe IRQ latency.
--
--- Patch [2]: [PATCH v8 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining ---
commit f4efa8f669813b67983a5bf98e40939bbc5e00b4
Author: Benoît Monin <benoit.monin@bootlin.com>
dmaengine: fsl-edma: Support dynamic scatter/gather chaining
This commit implements dynamic linking of scatter/gather transfers to enable
chaining multiple DMA descriptors without stopping the channel. It adds
fsl_edma_link_sg() to dynamically link the last TCD of a previously issued
descriptor to the first TCD of a new descriptor.
> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> index c5f5951c988be..189eb9d1269ea 100644
> --- a/drivers/dma/fsl-edma-common.c
> +++ b/drivers/dma/fsl-edma-common.c
[ ... ]
> @@ -931,14 +958,93 @@ void fsl_edma_xfer_desc(struct fsl_edma_chan *fsl_chan)
[ ... ]
> +static void fsl_edma_link_sg(struct fsl_edma_chan *fsl_chan, struct fsl_edma_desc *fsl_desc)
> +{
[ ... ]
> + if (!(last_csr & EDMA_TCD_CSR_D_REQ) ||
> + last_csr & EDMA_TCD_CSR_E_LINK ||
> + list_count_nodes(&fsl_chan->vchan.desc_issued) >= FIELD_MAX(EDMA_TCD_CSR_LINKCH))
> + return;
[Severity: High]
Could this list_count_nodes() call cause severe IRQ latency spikes?
list_count_nodes() unconditionally iterates over the entire list to compute
its length. If the list contains hundreds or thousands of nodes, it will
traverse all of them, well past the limit of 31 checked here.
[ ... ]
> @@ -949,7 +1055,12 @@ void fsl_edma_issue_pending(struct dma_chan *chan)
[ ... ]
> - if (vchan_issue_pending(&fsl_chan->vchan) && !fsl_chan->edesc)
> + list_for_each_entry_safe(vdesc, tmp, &fsl_chan->vchan.desc_submitted, node) {
> + fsl_edma_link_sg(fsl_chan, to_fsl_edma_desc(vdesc));
> + list_move_tail(&vdesc->node, &fsl_chan->vchan.desc_issued);
> + }
Since fsl_edma_link_sg() is called here in a loop over desc_submitted, while
holding the vchan.lock spinlock with interrupts disabled, submitting a large
batch of N descriptors (e.g., from network or ALSA drivers) could result in
O(N^2) list node traversals.
Could this heavily stall the CPU and potentially cause soft lockups?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-fsl-edma-dyn-sg-v8-0-dc2b0317206d@bootlin.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v8 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining
2026-08-03 16:02 ` [PATCH v8 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining Benoît Monin
2026-08-03 16:28 ` sashiko-bot
@ 2026-08-04 21:47 ` Frank Li
1 sibling, 0 replies; 7+ messages in thread
From: Frank Li @ 2026-08-04 21:47 UTC (permalink / raw)
To: Benoît Monin
Cc: Frank Li, Vinod Koul, Thomas Petazzoni, Frank Li, imx, dmaengine,
linux-kernel
On Mon, Aug 03, 2026 at 06:02:01PM +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(), we assign a non-zero identifier to the new
Nit: remove "we"
> 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, we only link
Nit" remove "We only".
> transactions for DMA controllers flagged with FSL_EDMA_DRV_SPLIT_REG.
suggest use new flags such as FSL_EDMA_DRV_CSR_LINKCH
>
> 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 | 125 +++++++++++++++++++++++++++++++++++++++---
> drivers/dma/fsl-edma-common.h | 3 +
> drivers/dma/fsl-edma-trace.h | 5 ++
> 3 files changed, 126 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> index c5f5951c988b..189eb9d1269e 100644
> --- a/drivers/dma/fsl-edma-common.c
> +++ b/drivers/dma/fsl-edma-common.c
> @@ -55,10 +55,37 @@ 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);
> + bool e_link = FIELD_GET(EDMA_TCD_CSR_E_LINK, csr);
Are you sure check e_link, suppose you reuse MAJORLINKCH, e_link should be
0 always.
> + 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);
good idea for using link_sg_id.
> +
> + /*
> + * If the transfer is still running,
> + * don't mark as complete the current descriptor
> + */
> + if ((e_link || id_match) && fsl_chan->status != DMA_COMPLETE)
> + break;
> +
> + list_del(&vdesc->node);
> + vchan_cookie_complete(vdesc);
> +
> + if (e_link || id_match)
> + break;
> + }
> +
> fsl_chan->edesc = NULL;
> - fsl_chan->status = DMA_COMPLETE;
> +
> } else {
> vchan_cyclic_callback(&fsl_chan->edesc->vdesc);
> }
> @@ -931,14 +958,93 @@ 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_SPLIT_REG) || 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 ||
> + list_count_nodes(&fsl_chan->vchan.desc_issued) >= FIELD_MAX(EDMA_TCD_CSR_LINKCH))
> + 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);
> +
> + if (prev_desc == fsl_chan->edesc &&
> + prev_desc->n_tcds == 1 &&
What means here of check?
> + !(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;
> unsigned long flags;
>
> spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
> @@ -949,7 +1055,12 @@ void fsl_edma_issue_pending(struct dma_chan *chan)
> return;
> }
>
> - if (vchan_issue_pending(&fsl_chan->vchan) && !fsl_chan->edesc)
> + list_for_each_entry_[Isafe(vdesc, tmp, &fsl_chan->vchan.desc_submitted, node) {
> + fsl_edma_link_sg(fsl_chan, to_fsl_edma_desc(vdesc));
if > 32, hardware have not link vdesc, move it desc_issued list. for example
desc_issue have 64, only first 31 put to tcd, where put last 33 to tcd?
Frank
> + list_move_tail(&vdesc->node, &fsl_chan->vchan.desc_issued);
> + }
> +
> + if (!list_empty(&fsl_chan->vchan.desc_issued) && !fsl_chan->edesc)
> fsl_edma_xfer_desc(fsl_chan);
>
> spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags);
> diff --git a/drivers/dma/fsl-edma-common.h b/drivers/dma/fsl-edma-common.h
> index 0d028048701d..ab7ec43f93cf 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 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[];
> diff --git a/drivers/dma/fsl-edma-trace.h b/drivers/dma/fsl-edma-trace.h
> index d3541301a247..ac319d2dbb90 100644
> --- a/drivers/dma/fsl-edma-trace.h
> +++ b/drivers/dma/fsl-edma-trace.h
> @@ -119,6 +119,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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v8 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining
2026-08-03 16:28 ` sashiko-bot
@ 2026-08-04 22:06 ` Frank Li
0 siblings, 0 replies; 7+ messages in thread
From: Frank Li @ 2026-08-04 22:06 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Benoît Monin, Frank.Li, vkoul, imx, dmaengine
On Mon, Aug 03, 2026 at 04:28:20PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] O(N^2) algorithmic complexity inside a spinlock during scatter-gather descriptor submission can cause severe IRQ latency.
> --
>
> --- Patch [2]: [PATCH v8 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining ---
> commit f4efa8f669813b67983a5bf98e40939bbc5e00b4
> Author: Benoît Monin <benoit.monin@bootlin.com>
>
> dmaengine: fsl-edma: Support dynamic scatter/gather chaining
>
> This commit implements dynamic linking of scatter/gather transfers to enable
> chaining multiple DMA descriptors without stopping the channel. It adds
> fsl_edma_link_sg() to dynamically link the last TCD of a previously issued
> descriptor to the first TCD of a new descriptor.
>
> > diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> > index c5f5951c988be..189eb9d1269ea 100644
> > --- a/drivers/dma/fsl-edma-common.c
> > +++ b/drivers/dma/fsl-edma-common.c
> [ ... ]
> > @@ -931,14 +958,93 @@ void fsl_edma_xfer_desc(struct fsl_edma_chan *fsl_chan)
> [ ... ]
> > +static void fsl_edma_link_sg(struct fsl_edma_chan *fsl_chan, struct fsl_edma_desc *fsl_desc)
> > +{
> [ ... ]
> > + if (!(last_csr & EDMA_TCD_CSR_D_REQ) ||
> > + last_csr & EDMA_TCD_CSR_E_LINK ||
> > + list_count_nodes(&fsl_chan->vchan.desc_issued) >= FIELD_MAX(EDMA_TCD_CSR_LINKCH))
> > + return;
>
> [Severity: High]
> Could this list_count_nodes() call cause severe IRQ latency spikes?
>
> list_count_nodes() unconditionally iterates over the entire list to compute
> its length. If the list contains hundreds or thousands of nodes, it will
> traverse all of them, well past the limit of 31 checked here.
Nice work, only left this issue by found AI.
>
> [ ... ]
> > @@ -949,7 +1055,12 @@ void fsl_edma_issue_pending(struct dma_chan *chan)
> [ ... ]
> > - if (vchan_issue_pending(&fsl_chan->vchan) && !fsl_chan->edesc)
> > + list_for_each_entry_safe(vdesc, tmp, &fsl_chan->vchan.desc_submitted, node) {
> > + fsl_edma_link_sg(fsl_chan, to_fsl_edma_desc(vdesc));
You check return value, if > 31, use list_splice to whole submitted queue
to pending queue.
but this is small optimzation.
Frank
> > + list_move_tail(&vdesc->node, &fsl_chan->vchan.desc_issued);
> > + }
>
> Since fsl_edma_link_sg() is called here in a loop over desc_submitted, while
> holding the vchan.lock spinlock with interrupts disabled, submitting a large
> batch of N descriptors (e.g., from network or ALSA drivers) could result in
> O(N^2) list node traversals.
>
> Could this heavily stall the CPU and potentially cause soft lockups?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260803-fsl-edma-dyn-sg-v8-0-dc2b0317206d@bootlin.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-08-04 22:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-03 16:01 [PATCH v8 0/2] dmaengine: fsl-edma: Scatter/gather improvements Benoît Monin
2026-08-03 16:02 ` [PATCH v8 1/2] dmaengine: fsl-edma: Implement device_prep_peripheral_dma_vec Benoît Monin
2026-08-03 16:23 ` sashiko-bot
2026-08-03 16:02 ` [PATCH v8 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining Benoît Monin
2026-08-03 16:28 ` sashiko-bot
2026-08-04 22:06 ` Frank Li
2026-08-04 21:47 ` Frank Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox