[PATCH v3 0/9] dmaengine: dw-edma: Fixes and interrupt-path groundwork

[PATCH v3 0/9] dmaengine: dw-edma: Fixes and interrupt-path groundwork

[Koichiro Den]

Hi,

This series collects dw-edma fixes and interrupt-path groundwork, mostly
split from v1 of the "Support dynamic LL appends" series.

The previous posting should have been marked v2, as Frank pointed out,
so this one is v3. It also addresses Sashiko's review and includes fixes
for several pre-existing issues found during that review.

  v2: https://lore.kernel.org/r/20260710080903.2392888-1-den@valinux.co.jp/
 (v1: https://lore.kernel.org/r/20260615154111.2174161-1-den@valinux.co.jp/)

Based on v7.2-rc1 (dmaengine/master).

Best regards,
Koichiro


Koichiro Den (9):
  dmaengine: dw-edma: Fix HDMA channel status register access
  dmaengine: dw-edma: Terminate all descriptors without callbacks
  dmaengine: dw-edma: Serialize abort state updates
  dmaengine: dw-edma: Complete descriptors before pausing
  dmaengine: dw-edma: Serialize channel state checks
  dmaengine: dw-edma: Clear stale requests on termination
  dmaengine: dw-edma-pcie: Drop redundant pci_free_irq_vectors()
  dmaengine: dw-edma: Snapshot the v0 interrupt status once per handler
    pass
  dmaengine: dw-edma: Defer channel IRQ handling to workqueue

 drivers/dma/dw-edma/dw-edma-core.c    | 192 ++++++++++++++++++++++----
 drivers/dma/dw-edma/dw-edma-core.h    |  11 ++
 drivers/dma/dw-edma/dw-edma-pcie.c    |   3 -
 drivers/dma/dw-edma/dw-edma-v0-core.c |  28 ++--
 drivers/dma/dw-edma/dw-hdma-v0-core.c |   2 +-
 5 files changed, 190 insertions(+), 46 deletions(-)

-- 
2.51.0

[PATCH v3 1/9] dmaengine: dw-edma: Fix HDMA channel status register access

[Koichiro Den]

GET_CH_32() takes the direction before the channel ID, but
dw_hdma_v0_core_ch_status() passed them in the opposite order. This can
make the status callback read another HDMA channel status register.

Use the same argument order as the other HDMA register accesses.

Fixes: e74c39573d35 ("dmaengine: dw-edma: Add support for native HDMA")
Cc: stable@vger.kernel.org
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v3:
  - No changes.

 drivers/dma/dw-edma/dw-hdma-v0-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index 632abb8b481c..2beec876b184 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -79,7 +79,7 @@ static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)
 	u32 tmp;
 
 	tmp = FIELD_GET(HDMA_V0_CH_STATUS_MASK,
-			GET_CH_32(dw, chan->id, chan->dir, ch_stat));
+			GET_CH_32(dw, chan->dir, chan->id, ch_stat));
 
 	if (tmp == 1)
 		return DMA_IN_PROGRESS;
-- 
2.51.0

[PATCH v3 2/9] dmaengine: dw-edma: Terminate all descriptors without callbacks

[Koichiro Den]

The DMA Engine client documentation says in the "Terminate APIs" section
of Documentation/driver-api/dmaengine/client.rst:

"No callback functions will be called for any incomplete transfers."

dw-edma instead calls vchan_cookie_complete() when a deferred STOP reaches
the interrupt handler. This schedules a callback for the active descriptor
and leaves other issued or submitted descriptors queued. A late callback
after dmaengine_terminate_sync() can dereference client state that has
already been freed, while leftover descriptors may later restart into
reused buffers or leak.

Move all issued and submitted descriptors to the terminated list whenever
termination completes. For a pending STOP, do this from both the DONE and
ABORT paths. Complete their cookies in order without scheduling callbacks.

A STOP can remain pending until the running transfer raises an interrupt.
Make device_synchronize() wait until the channel has stopped and
terminate_all() deconfigures it before releasing terminated descriptors.
Reuse it from free_chan_resources(), then release the remaining virt-dma
resources. Sleep instead of busy-polling while waiting, and warn if the
existing timeout expires.

Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v3:
  - Merge the descriptor cleanup patch so device_synchronize() is safe
    when introduced. (Sashiko)
  - Handle pending STOP requests from the ABORT path as well.
  - Drop a redundant cond_resched() after usleep_range().
  - Drop Frank's Reviewed-by tag due to these changes.
  - Polish a source comment.

 drivers/dma/dw-edma/dw-edma-core.c | 87 ++++++++++++++++++++++++++----
 1 file changed, 76 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 89a4c498a17b..44ef5fbe3fd4 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -7,6 +7,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/dmaengine.h>
@@ -201,6 +202,35 @@ static int dw_edma_start_transfer(struct dw_edma_chan *chan)
 	return 1;
 }
 
+static void dw_edma_terminate_vdesc(struct virt_dma_desc *vd)
+{
+	list_del(&vd->node);
+	dma_cookie_complete(&vd->tx);
+	vchan_terminate_vdesc(vd);
+}
+
+static void dw_edma_terminate_vdesc_list(struct list_head *head)
+{
+	struct virt_dma_desc *vd, *_vd;
+
+	list_for_each_entry_safe(vd, _vd, head, node)
+		dw_edma_terminate_vdesc(vd);
+}
+
+/* Must be called with vc.lock held. */
+static void dw_edma_terminate_all_descs(struct dw_edma_chan *chan)
+{
+	/*
+	 * This order must not be reversed. Cookies are assigned when
+	 * descriptors are submitted, so desc_issued contains older cookies
+	 * than desc_submitted. Completing desc_submitted first could move
+	 * chan->vc.chan.completed_cookie backwards when desc_issued is
+	 * terminated afterwards.
+	 */
+	dw_edma_terminate_vdesc_list(&chan->vc.desc_issued);
+	dw_edma_terminate_vdesc_list(&chan->vc.desc_submitted);
+}
+
 static void dw_edma_device_caps(struct dma_chan *dchan,
 				struct dma_slave_caps *caps)
 {
@@ -306,20 +336,25 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
 static int dw_edma_device_terminate_all(struct dma_chan *dchan)
 {
 	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+	unsigned long flags;
 	int err = 0;
 
+	spin_lock_irqsave(&chan->vc.lock, flags);
 	if (!chan->configured) {
-		/* Do nothing */
+		dw_edma_terminate_all_descs(chan);
 	} else if (chan->status == EDMA_ST_PAUSE) {
+		dw_edma_terminate_all_descs(chan);
 		chan->status = EDMA_ST_IDLE;
 		chan->configured = false;
 	} else if (chan->status == EDMA_ST_IDLE) {
+		dw_edma_terminate_all_descs(chan);
 		chan->configured = false;
 	} else if (dw_edma_core_ch_status(chan) == DMA_COMPLETE) {
 		/*
 		 * The channel is in a false BUSY state, probably didn't
 		 * receive or lost an interrupt
 		 */
+		dw_edma_terminate_all_descs(chan);
 		chan->status = EDMA_ST_IDLE;
 		chan->configured = false;
 	} else if (chan->request > EDMA_REQ_PAUSE) {
@@ -327,6 +362,7 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
 	} else {
 		chan->request = EDMA_REQ_STOP;
 	}
+	spin_unlock_irqrestore(&chan->vc.lock, flags);
 
 	return err;
 }
@@ -673,8 +709,7 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
 			break;
 
 		case EDMA_REQ_STOP:
-			list_del(&vd->node);
-			vchan_cookie_complete(vd);
+			dw_edma_terminate_all_descs(chan);
 			chan->request = EDMA_REQ_NONE;
 			chan->status = EDMA_ST_IDLE;
 			break;
@@ -698,7 +733,9 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
 
 	spin_lock_irqsave(&chan->vc.lock, flags);
 	vd = vchan_next_desc(&chan->vc);
-	if (vd) {
+	if (vd && chan->request == EDMA_REQ_STOP) {
+		dw_edma_terminate_all_descs(chan);
+	} else if (vd) {
 		dw_hdma_set_callback_result(vd, DMA_TRANS_ABORTED);
 		list_del(&vd->node);
 		vchan_cookie_complete(vd);
@@ -856,21 +893,48 @@ static int dw_edma_alloc_chan_resources(struct dma_chan *dchan)
 	return 0;
 }
 
-static void dw_edma_free_chan_resources(struct dma_chan *dchan)
+static void dw_edma_wait_termination(struct dma_chan *dchan)
 {
+	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
 	unsigned long timeout = jiffies + msecs_to_jiffies(5000);
-	int ret;
+	unsigned long flags;
+	bool configured = true;
 
+	/*
+	 * dw_edma_device_terminate_all() may defer cleanup to a later interrupt
+	 * while the channel is still running. Retry until the channel is
+	 * deconfigured, which means termination is complete.
+	 */
 	while (time_before(jiffies, timeout)) {
-		ret = dw_edma_device_terminate_all(dchan);
-		if (!ret)
-			break;
+		dw_edma_device_terminate_all(dchan);
 
-		if (time_after_eq(jiffies, timeout))
+		spin_lock_irqsave(&chan->vc.lock, flags);
+		configured = chan->configured;
+		spin_unlock_irqrestore(&chan->vc.lock, flags);
+		if (!configured)
 			return;
 
-		cpu_relax();
+		usleep_range(1000, 2000);
 	}
+
+	dev_warn(chan->dw->chip->dev,
+		 "timeout waiting for channel termination\n");
+}
+
+static void dw_edma_device_synchronize(struct dma_chan *dchan)
+{
+	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+
+	dw_edma_wait_termination(dchan);
+	vchan_synchronize(&chan->vc);
+}
+
+static void dw_edma_free_chan_resources(struct dma_chan *dchan)
+{
+	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+
+	dw_edma_device_synchronize(dchan);
+	vchan_free_chan_resources(&chan->vc);
 }
 
 static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
@@ -968,6 +1032,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
 	dma->device_pause = dw_edma_device_pause;
 	dma->device_resume = dw_edma_device_resume;
 	dma->device_terminate_all = dw_edma_device_terminate_all;
+	dma->device_synchronize = dw_edma_device_synchronize;
 	dma->device_issue_pending = dw_edma_device_issue_pending;
 	dma->device_tx_status = dw_edma_device_tx_status;
 	dma->device_prep_slave_sg = dw_edma_device_prep_slave_sg;
-- 
2.51.0

[PATCH v3 3/9] dmaengine: dw-edma: Serialize abort state updates

[Koichiro Den]

dw_edma_abort_interrupt() drops vc.lock before changing request and
status. issue_pending() can acquire the lock in that small window,
observe the old busy state, and skip starting queued descriptors. Then
the abort handler overwrites the channel status as idle, leaving the new
descriptors stranded for good.

Keep descriptor completion and the state transition in the same critical
section.

Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v3:
  - New patch for a pre-existing abort state race. (Sashiko)

 drivers/dma/dw-edma/dw-edma-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 44ef5fbe3fd4..272b03405746 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -740,9 +740,9 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
 		list_del(&vd->node);
 		vchan_cookie_complete(vd);
 	}
-	spin_unlock_irqrestore(&chan->vc.lock, flags);
 	chan->request = EDMA_REQ_NONE;
 	chan->status = EDMA_ST_IDLE;
+	spin_unlock_irqrestore(&chan->vc.lock, flags);
 }
 
 static void dw_edma_emul_irq_ack(struct irq_data *d)
-- 
2.51.0

[PATCH v3 4/9] dmaengine: dw-edma: Complete descriptors before pausing

[Koichiro Den]

If PAUSE is requested while the final chunk of a descriptor is in
flight, the DONE interrupt takes the PAUSE path without checking whether
the descriptor has been depleted. The depleted descriptor remains on the
issued list and the channel enters EDMA_ST_PAUSE.

On resume, dw_edma_start_transfer() finds the descriptor but no chunk to
start and returns 0. The caller ignores that result and leaves the
channel stuck in EDMA_ST_BUSY with no transfer running.

Check for descriptor completion before acknowledging PAUSE. If there is
no work to start on resume, leave the channel idle. Also ignore DONE
interrupts while the channel is paused so a stale or repeated interrupt
cannot change its state or start queued work.

Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v3:
  - New patch for a pre-existing PAUSE completion issue. (Sashiko)

 drivers/dma/dw-edma/dw-edma-core.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 272b03405746..b06b299661c0 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -327,7 +327,8 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
 		err = -EPERM;
 	} else {
 		chan->status = EDMA_ST_BUSY;
-		dw_edma_start_transfer(chan);
+		if (!dw_edma_start_transfer(chan))
+			chan->status = EDMA_ST_IDLE;
 	}
 
 	return err;
@@ -691,10 +692,16 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
 	unsigned long flags;
 
 	spin_lock_irqsave(&chan->vc.lock, flags);
+	if (chan->status == EDMA_ST_PAUSE) {
+		spin_unlock_irqrestore(&chan->vc.lock, flags);
+		return;
+	}
+
 	vd = vchan_next_desc(&chan->vc);
 	if (vd) {
 		switch (chan->request) {
 		case EDMA_REQ_NONE:
+		case EDMA_REQ_PAUSE:
 			desc = vd2dw_edma_desc(vd);
 			if (!desc->chunks_alloc) {
 				dw_hdma_set_callback_result(vd,
@@ -703,6 +710,12 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
 				vchan_cookie_complete(vd);
 			}
 
+			if (chan->request == EDMA_REQ_PAUSE) {
+				chan->request = EDMA_REQ_NONE;
+				chan->status = EDMA_ST_PAUSE;
+				break;
+			}
+
 			/* Continue transferring if there are remaining chunks or issued requests.
 			 */
 			chan->status = dw_edma_start_transfer(chan) ? EDMA_ST_BUSY : EDMA_ST_IDLE;
@@ -714,11 +727,6 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
 			chan->status = EDMA_ST_IDLE;
 			break;
 
-		case EDMA_REQ_PAUSE:
-			chan->request = EDMA_REQ_NONE;
-			chan->status = EDMA_ST_PAUSE;
-			break;
-
 		default:
 			break;
 		}
-- 
2.51.0

[PATCH v3 5/9] dmaengine: dw-edma: Serialize channel state checks

[Koichiro Den]

pause() and resume() read and update channel state without holding vc.lock,
while the interrupt handlers update the same state under it. Take the same
lock around those state checks so that request, status, and configured stay
consistent.

For example, pause() can observe EDMA_ST_BUSY right before the interrupt
handler completes the final descriptor and moves the channel to
EDMA_ST_IDLE, and then record EDMA_REQ_PAUSE on an already idle channel. No
further interrupt will acknowledge the request, and since issue_pending()
requires EDMA_REQ_NONE, the channel is wedged for good: terminate_all()
leaves the stale request behind, so even reconfiguring the channel does not
recover it.

issue_pending() already runs under vc.lock, but it tests configured before
taking it. Move that test under the lock as well, so that the decision to
start work is made against the current value rather than one observed
before a concurrent terminate_all() deconfigured the channel.

Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v3:
  - No changes.

 drivers/dma/dw-edma/dw-edma-core.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index b06b299661c0..8b0e2af734da 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -300,8 +300,10 @@ static int dw_edma_device_config(struct dma_chan *dchan,
 static int dw_edma_device_pause(struct dma_chan *dchan)
 {
 	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+	unsigned long flags;
 	int err = 0;
 
+	spin_lock_irqsave(&chan->vc.lock, flags);
 	if (!chan->configured)
 		err = -EPERM;
 	else if (chan->status != EDMA_ST_BUSY)
@@ -310,6 +312,7 @@ static int dw_edma_device_pause(struct dma_chan *dchan)
 		err = -EPERM;
 	else
 		chan->request = EDMA_REQ_PAUSE;
+	spin_unlock_irqrestore(&chan->vc.lock, flags);
 
 	return err;
 }
@@ -317,8 +320,10 @@ static int dw_edma_device_pause(struct dma_chan *dchan)
 static int dw_edma_device_resume(struct dma_chan *dchan)
 {
 	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+	unsigned long flags;
 	int err = 0;
 
+	spin_lock_irqsave(&chan->vc.lock, flags);
 	if (!chan->configured) {
 		err = -EPERM;
 	} else if (chan->status != EDMA_ST_PAUSE) {
@@ -330,6 +335,7 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
 		if (!dw_edma_start_transfer(chan))
 			chan->status = EDMA_ST_IDLE;
 	}
+	spin_unlock_irqrestore(&chan->vc.lock, flags);
 
 	return err;
 }
@@ -373,11 +379,9 @@ static void dw_edma_device_issue_pending(struct dma_chan *dchan)
 	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
 	unsigned long flags;
 
-	if (!chan->configured)
-		return;
-
 	spin_lock_irqsave(&chan->vc.lock, flags);
-	if (vchan_issue_pending(&chan->vc) && chan->request == EDMA_REQ_NONE &&
+	if (chan->configured && vchan_issue_pending(&chan->vc) &&
+	    chan->request == EDMA_REQ_NONE &&
 	    chan->status == EDMA_ST_IDLE) {
 		chan->status = EDMA_ST_BUSY;
 		dw_edma_start_transfer(chan);
-- 
2.51.0

[PATCH v3 6/9] dmaengine: dw-edma: Clear stale requests on termination

[Koichiro Den]

terminate_all() can deconfigure a channel immediately when it is already
unconfigured, paused, idle, or stopped in hardware. A pending PAUSE
request can survive these paths and block issue_pending() after the
channel is configured again.

Clear request whenever termination leaves the channel deconfigured. A
running channel remains configured while its STOP request is pending, so
the interrupt handler can still consume that request.

Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v3:
  - New patch for a pre-existing stale request issue. (Sashiko)

 drivers/dma/dw-edma/dw-edma-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 8b0e2af734da..2369b4c4630b 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -369,6 +369,8 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
 	} else {
 		chan->request = EDMA_REQ_STOP;
 	}
+	if (!chan->configured)
+		chan->request = EDMA_REQ_NONE;
 	spin_unlock_irqrestore(&chan->vc.lock, flags);
 
 	return err;
-- 
2.51.0

[PATCH v3 7/9] dmaengine: dw-edma-pcie: Drop redundant pci_free_irq_vectors()

[Koichiro Den]

dw_edma_pcie enables the PCI device with pcim_enable_device(), so IRQ
vectors allocated by pci_alloc_irq_vectors() are released by
pcim_msi_release() on device release. The driver should not call
pci_free_irq_vectors() manually.

Drop the redundant remove-time cleanup and rely on the managed PCI
device lifetime instead, as documented by commit 03e4905402ae ("PCI/MSI:
Clarify pci_free_irq_vectors() usage for managed devices").

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v3:
  - (Re-)collect Frank's Reviewed-by tag.
  - No code changes.

 drivers/dma/dw-edma/dw-edma-pcie.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 791c46e8ae4c..5e81a433a957 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -555,9 +555,6 @@ static void dw_edma_pcie_remove(struct pci_dev *pdev)
 	err = dw_edma_remove(chip);
 	if (err)
 		pci_warn(pdev, "can't remove device properly: %d\n", err);
-
-	/* Freeing IRQs */
-	pci_free_irq_vectors(pdev);
 }
 
 static const struct pci_device_id dw_edma_pcie_id_table[] = {
-- 
2.51.0

[PATCH v3 8/9] dmaengine: dw-edma: Snapshot the v0 interrupt status once per handler pass

[Koichiro Den]

The v0 interrupt handler reads the interrupt status register twice per
invocation, once through the DONE accessor and once through the ABORT
accessor, although both fields live in the same 32-bit register. On
remote setups (dw-edma-pcie) each read is a non-posted round trip across
the PCIe link costing on the order of a microsecond, and with one
completion interrupt per element the duplicate adds up. As an example,
profiling the R-Car S4 remote path put the handler at ~7us per
invocation, dominated by such reads.

Read the register once and derive the DONE and ABORT views from the
snapshot. No abort is lost to this because the pass only clears status
bits it observed, so an abort raised after the snapshot keeps its status
and its own interrupt delivery brings it to the next pass. An abort on
an observed channel cannot race the clear either. Software can restart
the halted channel only after abort() runs, and abort() is called after
dw_edma_v0_core_clear_abort_int().

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v3:
  - (Re-)collect Frank's Reviewed-by tag.
  - Revise the commit message and a source comment.

 drivers/dma/dw-edma/dw-edma-v0-core.c | 28 +++++++++++++--------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index cfdd6463252e..6befae2c95ac 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -218,18 +218,6 @@ static void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
 		  FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id)));
 }
 
-static u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
-{
-	return FIELD_GET(EDMA_V0_DONE_INT_MASK,
-			 GET_RW_32(dw, dir, int_status));
-}
-
-static u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
-{
-	return FIELD_GET(EDMA_V0_ABORT_INT_MASK,
-			 GET_RW_32(dw, dir, int_status));
-}
-
 static irqreturn_t
 dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
 			   dw_edma_handler_t done, dw_edma_handler_t abort)
@@ -239,7 +227,7 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
 	irqreturn_t ret = IRQ_NONE;
 	struct dw_edma_chan *chan;
 	unsigned long off;
-	u32 mask;
+	u32 mask, sts;
 
 	if (dir == EDMA_DIR_WRITE) {
 		total = dw->wr_ch_cnt;
@@ -251,7 +239,17 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
 		mask = dw_irq->rd_mask;
 	}
 
-	val = dw_edma_v0_core_status_done_int(dw, dir);
+	/*
+	 * DONE and ABORT status share one register, and on remote setups
+	 * every read is a non-posted round trip across the PCIe link. Take
+	 * one snapshot and derive both views from it. An abort raised
+	 * after the snapshot is deferred, not lost: only bits observed in
+	 * the snapshot are ever cleared below, so its status remains set and
+	 * triggers another handler pass.
+	 */
+	sts = GET_RW_32(dw, dir, int_status);
+
+	val = FIELD_GET(EDMA_V0_DONE_INT_MASK, sts);
 	val &= mask;
 	for_each_set_bit(pos, &val, total) {
 		chan = &dw->chan[pos + off];
@@ -262,7 +260,7 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
 		ret = IRQ_HANDLED;
 	}
 
-	val = dw_edma_v0_core_status_abort_int(dw, dir);
+	val = FIELD_GET(EDMA_V0_ABORT_INT_MASK, sts);
 	val &= mask;
 	for_each_set_bit(pos, &val, total) {
 		chan = &dw->chan[pos + off];
-- 
2.51.0

[PATCH v3 9/9] dmaengine: dw-edma: Defer channel IRQ handling to workqueue

[Koichiro Den]

On some SoCs (e.g. R-Car S4) the endpoint-side eDMA raises a single
fixed SPI that is hardwired to CPU0 and covers every read and write
channel. Handling channel events directly in that hard IRQ context
serializes the completion processing of all channels on one CPU:
descriptor recycling and refill, client callbacks (the vchan tasklet
runs on the scheduling CPU) and the doorbell writes all funnel through
CPU0, while the handler additionally spins on each channel's vc.lock.
Especially under heavy multichannel load, this contention becomes a
performance bottleneck.

Keep the hard IRQ handler minimal: clear the status, dispatch channel
events, and defer per-channel processing to work items. A work item per
channel preserves ordering while allowing different channels to run in
parallel on any CPU.

Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v3:
  - Clear irq_pending in dw_edma_device_synchronize(). (Sashiko)
  - Drop Frank's Reviewed-by tag due to this change.
  - Refine the commit message and source comments.

 drivers/dma/dw-edma/dw-edma-core.c | 69 +++++++++++++++++++++++++++---
 drivers/dma/dw-edma/dw-edma-core.h | 11 +++++
 2 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 2369b4c4630b..4ee7f14c3a82 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -30,6 +30,11 @@ struct dw_edma_desc *vd2dw_edma_desc(struct virt_dma_desc *vd)
 	return container_of(vd, struct dw_edma_desc, vd);
 }
 
+enum dw_edma_irq_event {
+	DW_EDMA_IRQ_DONE	= BIT(0),
+	DW_EDMA_IRQ_ABORT	= BIT(1),
+};
+
 static inline
 u64 dw_edma_get_pci_address(struct dw_edma_chan *chan, phys_addr_t cpu_addr)
 {
@@ -759,6 +764,39 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
 	spin_unlock_irqrestore(&chan->vc.lock, flags);
 }
 
+static void dw_edma_irq_work(struct work_struct *work)
+{
+	struct dw_edma_chan *chan = container_of(work, struct dw_edma_chan,
+						 irq_work);
+	unsigned int events;
+
+	do {
+		events = atomic_xchg(&chan->irq_pending, 0);
+
+		if (events & DW_EDMA_IRQ_DONE)
+			dw_edma_done_interrupt(chan);
+		if (events & DW_EDMA_IRQ_ABORT)
+			dw_edma_abort_interrupt(chan);
+	} while (atomic_read(&chan->irq_pending));
+}
+
+static void dw_edma_queue_irq_work(struct dw_edma_chan *chan,
+				   enum dw_edma_irq_event event)
+{
+	atomic_or(event, &chan->irq_pending);
+	queue_work(chan->dw->wq, &chan->irq_work);
+}
+
+static void dw_edma_done_interrupt_deferred(struct dw_edma_chan *chan)
+{
+	dw_edma_queue_irq_work(chan, DW_EDMA_IRQ_DONE);
+}
+
+static void dw_edma_abort_interrupt_deferred(struct dw_edma_chan *chan)
+{
+	dw_edma_queue_irq_work(chan, DW_EDMA_IRQ_ABORT);
+}
+
 static void dw_edma_emul_irq_ack(struct irq_data *d)
 {
 	struct dw_edma *dw = irq_data_get_irq_chip_data(d);
@@ -853,8 +891,8 @@ static inline irqreturn_t dw_edma_interrupt_write_inner(int irq, void *data)
 	struct dw_edma_irq *dw_irq = data;
 
 	return dw_edma_core_handle_int(dw_irq, EDMA_DIR_WRITE,
-				       dw_edma_done_interrupt,
-				       dw_edma_abort_interrupt);
+				       dw_edma_done_interrupt_deferred,
+				       dw_edma_abort_interrupt_deferred);
 }
 
 static inline irqreturn_t dw_edma_interrupt_read_inner(int irq, void *data)
@@ -862,8 +900,8 @@ static inline irqreturn_t dw_edma_interrupt_read_inner(int irq, void *data)
 	struct dw_edma_irq *dw_irq = data;
 
 	return dw_edma_core_handle_int(dw_irq, EDMA_DIR_READ,
-				       dw_edma_done_interrupt,
-				       dw_edma_abort_interrupt);
+				       dw_edma_done_interrupt_deferred,
+				       dw_edma_abort_interrupt_deferred);
 }
 
 static inline irqreturn_t dw_edma_interrupt_write(int irq, void *data)
@@ -940,6 +978,8 @@ static void dw_edma_device_synchronize(struct dma_chan *dchan)
 	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
 
 	dw_edma_wait_termination(dchan);
+	cancel_work_sync(&chan->irq_work);
+	atomic_set(&chan->irq_pending, 0);
 	vchan_synchronize(&chan->vc);
 }
 
@@ -982,6 +1022,8 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
 		chan->configured = false;
 		chan->request = EDMA_REQ_NONE;
 		chan->status = EDMA_ST_IDLE;
+		INIT_WORK(&chan->irq_work, dw_edma_irq_work);
+		atomic_set(&chan->irq_pending, 0);
 
 		if (chan->dir == EDMA_DIR_WRITE)
 			chan->ll_max = (chip->ll_region_wr[chan->id].sz / EDMA_LL_SZ);
@@ -1195,10 +1237,21 @@ int dw_edma_probe(struct dw_edma_chip *chip)
 	/* Disable eDMA, only to establish the ideal initial conditions */
 	dw_edma_core_off(dw);
 
+	/*
+	 * Deferred IRQ works are queued from the hard IRQ handlers, so the
+	 * workqueue must exist before any IRQ is requested.
+	 */
+	dw->wq = alloc_workqueue("dw-edma:%s", WQ_UNBOUND | WQ_HIGHPRI, 0,
+				 dev_name(chip->dev));
+	if (!dw->wq)
+		return -ENOMEM;
+
 	/* Request IRQs */
 	err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
-	if (err)
+	if (err) {
+		destroy_workqueue(dw->wq);
 		return err;
+	}
 
 	/* Allocate a dedicated virtual IRQ for interrupt-emulation doorbells */
 	err = dw_edma_emul_irq_alloc(dw);
@@ -1221,6 +1274,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
 	for (i = (dw->nr_irqs - 1); i >= 0; i--)
 		free_irq(chip->ops->irq_vector(dev, i), &dw->irq[i]);
 	dw_edma_emul_irq_free(dw);
+	destroy_workqueue(dw->wq);
 
 	return err;
 }
@@ -1245,6 +1299,11 @@ int dw_edma_remove(struct dw_edma_chip *chip)
 		free_irq(chip->ops->irq_vector(dev, i), &dw->irq[i]);
 	dw_edma_emul_irq_free(dw);
 
+	for (i = 0; i < dw->wr_ch_cnt + dw->rd_ch_cnt; i++)
+		cancel_work_sync(&dw->chan[i].irq_work);
+
+	destroy_workqueue(dw->wq);
+
 	/* Deregister eDMA device */
 	dma_async_device_unregister(&dw->dma);
 	list_for_each_entry_safe(chan, _chan, &dw->dma.channels,
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 6474cacf7195..7f6301572fa4 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -9,8 +9,10 @@
 #ifndef _DW_EDMA_CORE_H
 #define _DW_EDMA_CORE_H
 
+#include <linux/atomic.h>
 #include <linux/msi.h>
 #include <linux/dma/edma.h>
+#include <linux/workqueue.h>
 
 #include "../virt-dma.h"
 
@@ -87,6 +89,9 @@ struct dw_edma_chan {
 
 	struct dma_slave_config		config;
 	bool				non_ll;
+
+	struct work_struct		irq_work;
+	atomic_t			irq_pending;
 };
 
 struct dw_edma_irq {
@@ -109,6 +114,12 @@ struct dw_edma {
 
 	struct dw_edma_chan		*chan;
 
+	/*
+	 * WQ_HIGHPRI keeps completion processing responsive under heavy load;
+	 * WQ_UNBOUND lets different channels run on different CPUs.
+	 */
+	struct workqueue_struct		*wq;
+
 	raw_spinlock_t			lock;		/* Protect v0 shared registers */
 
 	struct dw_edma_chip             *chip;
-- 
2.51.0

Re: [PATCH v3 4/9] dmaengine: dw-edma: Complete descriptors before pausing

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing locking in `dw_edma_device_pause()` and `dw_edma_device_resume()` leads to data races, list corruption, double free, and channel deadlock.
--

--- Patch [4]: [PATCH v3 4/9] dmaengine: dw-edma: Complete descriptors before pausing ---
commit 59c4b510d7df19eb3243d231516e5c29dab50d72
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Complete descriptors before pausing

This patch modifies dw_edma_start_transfer() usage on resume to set the
channel state to idle if no transfer can be started. It also adds logic
to ignore done interrupts when the channel is paused and ensures
descriptors are completed during a pause request.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 272b034057469..b06b299661c02 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -327,7 +327,8 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
>  		err = -EPERM;
>  	} else {
>  		chan->status = EDMA_ST_BUSY;
> -		dw_edma_start_transfer(chan);
> +		if (!dw_edma_start_transfer(chan))
> +			chan->status = EDMA_ST_IDLE;

[Severity: High]
This is a pre-existing issue, but does dw_edma_device_resume() execute this
path without holding chan->vc.lock?

The dw_edma_start_transfer() function manipulates virt-dma lists, frees
chunk metadata, and invokes vchan_next_desc() which expects the lock to
be held:

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_start_transfer()
    vd = vchan_next_desc(&chan->vc);

A concurrent interrupt in dw_edma_done_interrupt() also takes the lock
and can call dw_edma_start_transfer(). Could this race lead to linked
list corruption, double frees, or lockdep panics?

Additionally, does dw_edma_device_pause() similarly modify chan->request
locklessly?

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_device_pause()
    else
        chan->request = EDMA_REQ_PAUSE;

Could this cause data races with dw_edma_done_interrupt() when
dmaengine_pause() and dmaengine_resume() are called while transfers are
ongoing?

>  	}
>  
>  	return err;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715175740.1110404-1-den@valinux.co.jp?part=4

Re: [PATCH v3 3/9] dmaengine: dw-edma: Serialize abort state updates

[sashiko-bot]

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

Pre-existing issues:
- [High] dw_edma_device_pause and dw_edma_device_resume modify channel state and start transfers without holding the virtual channel lock.
--

--- Patch [3]: [PATCH v3 3/9] dmaengine: dw-edma: Serialize abort state updates ---
commit 9fc733b7a8d11a99c619ff0dfa84d208446f0ab9
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Serialize abort state updates
    
This commit fixes a race condition in dw_edma_abort_interrupt() where dropping
the virtual channel lock before updating the channel request and status could
cause queued descriptors to be stranded. It keeps the descriptor completion
and state transition within the same critical section.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 44ef5fbe3fd4a..272b034057469 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -740,9 +740,9 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
>  		list_del(&vd->node);
>  		vchan_cookie_complete(vd);
>  	}
> -	spin_unlock_irqrestore(&chan->vc.lock, flags);
>  	chan->request = EDMA_REQ_NONE;
>  	chan->status = EDMA_ST_IDLE;
> +	spin_unlock_irqrestore(&chan->vc.lock, flags);

[Severity: High]
This is a pre-existing issue, but since this patch fixes locking around
state updates, are dw_edma_device_pause() and dw_edma_device_resume() also
missing necessary locks?

Looking at dw_edma_device_pause(), it modifies chan->request without holding
chan->vc.lock. Similarly, dw_edma_device_resume() modifies chan->status and
invokes dw_edma_start_transfer() locklessly:

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_device_resume() {
        ...
        } else {
                chan->status = EDMA_ST_BUSY;
                dw_edma_start_transfer(chan);
        }

        return err;
}

Since dw_edma_start_transfer() accesses protected lists and calls
vchan_next_desc() which expects the lock to be held
(lockdep_assert_held(&vc->lock)), could concurrent access from interrupt
handlers or issue_pending() lead to list corruption or invalid channel
states?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715175740.1110404-1-den@valinux.co.jp?part=3

Re: [PATCH v3 6/9] dmaengine: dw-edma: Clear stale requests on termination

[sashiko-bot]

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

Pre-existing issues:
- [High] The PCIe driver fails to validate that the PCI BAR size is large enough to contain the hardcoded offsets and sizes for the DMA linked-list and data regions. A malicious PCIe device can present an artificially small BAR, causing the computed iomem pointers to fall outside the mapped region.
--

--- Patch [6]: [PATCH v3 6/9] dmaengine: dw-edma: Clear stale requests on termination ---
commit c7a703de13f7d742a92f8673efc6a54e5f2e9c63
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Clear stale requests on termination

This commit fixes a stale request issue where a pending PAUSE request
could survive paths that deconfigure a channel. It ensures that the
request state is cleared whenever termination leaves the channel
deconfigured.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 8b0e2af734da9..2369b4c4630b9 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c

[ ... ]

[Severity: High]
This is a pre-existing issue and was not introduced by this patch, but does
dw_edma_pcie_probe() lack validation against the actual PCI BAR length
before computing I/O memory pointers?

Looking at dw_edma_pcie_probe() in drivers/dma/dw-edma/dw-edma-pcie.c:

drivers/dma/dw-edma/dw-edma-pcie.c:dw_edma_pcie_probe() {
[ ... ]
	for (i = 0; i < chip->ll_wr_cnt && !non_ll; i++) {
		struct dw_edma_region *ll_region = &chip->ll_region_wr[i];
		struct dw_edma_region *dt_region = &chip->dt_region_wr[i];
		struct dw_edma_block *ll_block = &vsec_data->ll_wr[i];
		struct dw_edma_block *dt_block = &vsec_data->dt_wr[i];

		ll_region->vaddr.io = pcim_iomap_table(pdev)[ll_block->bar];
		if (!ll_region->vaddr.io)
			return -ENOMEM;

		ll_region->vaddr.io += ll_block->off;
		ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
							 ll_block->bar);
		ll_region->paddr += ll_block->off;
		ll_region->sz = ll_block->sz;
[ ... ]
}

If a malicious PCIe or Thunderbolt device presents a spoofed vendor/device
ID with an artificially small BAR, couldn't the computed iomem pointers
fall outside the mapped region?

Should the driver verify that the hardcoded offsets and sizes fit within
the BAR by checking if ll_block->off + ll_block->sz <=
pci_resource_len(pdev, ll_block->bar) prior to pointer arithmetic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715175740.1110404-1-den@valinux.co.jp?part=6

Re: [PATCH v3 5/9] dmaengine: dw-edma: Serialize channel state checks

[sashiko-bot]

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

Pre-existing issues:
- [High] `dw_edma_device_terminate_all()` fails to actually stop the DMA hardware engine, leaving it vulnerable to permanent hangs if the hardware stalls.
- [High] The driver advertises `DMA_CYCLIC` support and accepts cyclic transfers, but fails to implement true infinite looping or fire the required period completion callbacks.
--

--- Patch [5]: [PATCH v3 5/9] dmaengine: dw-edma: Serialize channel state checks ---
commit 1467b5248fd39f737e5901d6f209dccdfffd622e
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Serialize channel state checks

The commit fixes a race condition between pause()/resume() and interrupt 
handlers by extending vc.lock coverage to include channel state checks. 
This prevents the channel from becoming permanently wedged due to missed state 
updates, and ensures issue_pending() uses the correct configuration state.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index b06b299661c02..8b0e2af734da9 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but does dw_edma_device_terminate_all() fail to
actually stop the DMA hardware engine?

If the hardware stalls (e.g., due to an unresponsive peripheral or AXI bus 
hang), it looks like the DMA channel could become permanently wedged. When a
client calls dmaengine_terminate_all() to abort a transfer, the code sets a
software flag:

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_device_terminate_all() {
    ...
    } else if (chan->request > EDMA_REQ_PAUSE) {
        err = -EPERM;
    } else {
        chan->request = EDMA_REQ_STOP;
    }
    ...
}

Will this cause the polling loop in dw_edma_wait_termination() to time out 
if the hardware is stalled and the interrupt never fires? Should the hardware 
be explicitly stopped rather than relying exclusively on natural completion?

[Severity: High]
This is a pre-existing issue, but does the driver fail to implement true 
infinite looping for DMA_CYCLIC transfers?

When a client submits a cyclic transfer via device_prep_dma_cyclic(), it is 
unrolled into a finite list of bursts based on xfer.cyclic.cnt. However, in 
dw_edma_done_interrupt(), the descriptor appears to be completely finished 
without calling vchan_cyclic_callback(vd):

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_done_interrupt() {
    ...
    case EDMA_REQ_PAUSE:
        desc = vd2dw_edma_desc(vd);
        if (!desc->chunks_alloc) {
            dw_hdma_set_callback_result(vd,
                                        DMA_TRANS_NOERROR);
            list_del(&vd->node);
            vchan_cookie_complete(vd);
        }
    ...
}

Will this cause subsystems relying on cyclic DMA (like ALSA for audio) to hang 
indefinitely while waiting for period completion events?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715175740.1110404-1-den@valinux.co.jp?part=5

Re: [PATCH v3 2/9] dmaengine: dw-edma: Terminate all descriptors without callbacks

[sashiko-bot]

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

New issues:
- [High] `dmaengine_synchronize()` explicitly waits for and causes the channel to lose its configuration, breaking generic clients that expect to reuse the channel without calling `dmaengine_slave_config()` again.

Pre-existing issues:
- [High] State updates in `dw_edma_abort_interrupt` are performed outside the channel lock, creating a data race that can permanently stall the channel.
- [High] `dw_edma_device_pause` and `dw_edma_device_resume` read and modify channel state variables without acquiring `chan->vc.lock`.
--

--- Patch [2]: [PATCH v3 2/9] dmaengine: dw-edma: Terminate all descriptors without callbacks ---
commit ca03488a0b49733d1020ab17fdc7c52f079bfb7c
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Terminate all descriptors without callbacks

This commit changes the termination behavior of the dw-edma driver. It
moves all issued and submitted descriptors to the terminated list without
scheduling callbacks when termination completes. It also introduces
dw_edma_device_synchronize() to wait for channel termination.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 89a4c498a17b8..44ef5fbe3fd4a 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -306,20 +336,25 @@ static int dw_edma_device_resume(struct dma_chan *dchan)

[Severity: High]
This is a pre-existing issue, but does dw_edma_device_pause() and
dw_edma_device_resume() have a data race?

In drivers/dma/dw-edma/dw-edma-core.c:dw_edma_device_pause():

	if (!chan->configured)
		err = -EPERM;
	else if (chan->status != EDMA_ST_BUSY)
		err = -EPERM;
	else if (chan->request != EDMA_REQ_NONE)
		err = -EPERM;
	else
		chan->request = EDMA_REQ_PAUSE;

These functions read and modify chan->status and chan->request without
acquiring chan->vc.lock. If device_pause() runs concurrently with an
interrupt completing a transfer, it might overwrite chan->request to
EDMA_REQ_PAUSE on an already idle channel, preventing new transfers from
starting.

Similarly, device_resume() calls dw_edma_start_transfer() which manipulates
virt-dma lists without synchronization.

>  static int dw_edma_device_terminate_all(struct dma_chan *dchan)
>  {
[ ... ]
> @@ -698,7 +733,9 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
>  
>  	spin_lock_irqsave(&chan->vc.lock, flags);
>  	vd = vchan_next_desc(&chan->vc);
> -	if (vd) {
> +	if (vd && chan->request == EDMA_REQ_STOP) {
> +		dw_edma_terminate_all_descs(chan);
> +	} else if (vd) {
>  		dw_hdma_set_callback_result(vd, DMA_TRANS_ABORTED);
>  		list_del(&vd->node);
>  		vchan_cookie_complete(vd);

[Severity: High]
This is a pre-existing issue, but can this sequence permanently stall the
channel?

The end of dw_edma_abort_interrupt() looks like this:

	spin_unlock_irqrestore(&chan->vc.lock, flags);
	chan->request = EDMA_REQ_NONE;
	chan->status = EDMA_ST_IDLE;
}

Because chan->request and chan->status are updated after releasing
chan->vc.lock, a concurrent client completion callback could queue a new
descriptor and call dma_async_issue_pending().

The issue_pending() call would acquire the lock, observe the stale
chan->status (e.g. EDMA_ST_BUSY), and fail to start the hardware. The
interrupt handler then unconditionally writes EDMA_ST_IDLE, leaving the
hardware idle while the descriptor is stuck in the desc_issued list.

[ ... ]
> @@ -856,21 +893,48 @@ static int dw_edma_alloc_chan_resources(struct dma_chan *dchan)
>  	return 0;
>  }
>  
> -static void dw_edma_free_chan_resources(struct dma_chan *dchan)
> +static void dw_edma_wait_termination(struct dma_chan *dchan)
>  {
> +	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
>  	unsigned long timeout = jiffies + msecs_to_jiffies(5000);
> -	int ret;
> +	unsigned long flags;
> +	bool configured = true;
>  
> +	/*
> +	 * dw_edma_device_terminate_all() may defer cleanup to a later interrupt
> +	 * while the channel is still running. Retry until the channel is
> +	 * deconfigured, which means termination is complete.
> +	 */
>  	while (time_before(jiffies, timeout)) {
> -		ret = dw_edma_device_terminate_all(dchan);
> -		if (!ret)
> -			break;
> +		dw_edma_device_terminate_all(dchan);
>  
> -		if (time_after_eq(jiffies, timeout))
> +		spin_lock_irqsave(&chan->vc.lock, flags);
> +		configured = chan->configured;
> +		spin_unlock_irqrestore(&chan->vc.lock, flags);
> +		if (!configured)
>  			return;
>  

[Severity: High]
Will this break generic dmaengine clients?

The loop in dw_edma_wait_termination() forces the channel to become
unconfigured by polling dw_edma_device_terminate_all() until
chan->configured is set to false.

However, dw_edma_wait_termination() is called by dw_edma_device_synchronize().
Generic dmaengine clients expect the channel configuration to persist across
dmaengine_terminate_sync(). Because this explicitly clears the configuration,
subsequent dmaengine_prep_* calls from generic clients will return NULL
and fail.

> -		cpu_relax();
> +		usleep_range(1000, 2000);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715175740.1110404-1-den@valinux.co.jp?part=2

Re: [PATCH v3 9/9] dmaengine: dw-edma: Defer channel IRQ handling to workqueue

[sashiko-bot]

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

Pre-existing issues:
- [High] Shared IRQs are requested before `irq->dw` is initialized, potentially causing a NULL pointer dereference on early interrupts.
--

--- Patch [9]: [PATCH v3 9/9] dmaengine: dw-edma: Defer channel IRQ handling to workqueue ---
commit 61576b5286f799082619f98fb9d5cfc6fa61833f
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Defer channel IRQ handling to workqueue

This patch modifies the eDMA driver to keep the hard IRQ handler minimal to
prevent performance bottlenecks under multichannel load. It defers per-channel
completion and abort processing to a workqueue, queuing a work item per
channel to allow parallel execution across multiple CPUs.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 2369b4c4630b9..4ee7f14c3a824 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c

[ ... ]

> @@ -1195,10 +1237,21 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>  	/* Disable eDMA, only to establish the ideal initial conditions */
>  	dw_edma_core_off(dw);
>  
> +	/*
> +	 * Deferred IRQ works are queued from the hard IRQ handlers, so the
> +	 * workqueue must exist before any IRQ is requested.
> +	 */
> +	dw->wq = alloc_workqueue("dw-edma:%s", WQ_UNBOUND | WQ_HIGHPRI, 0,
> +				 dev_name(chip->dev));
> +	if (!dw->wq)
> +		return -ENOMEM;
> +
>  	/* Request IRQs */
>  	err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);

[Severity: High]
This is a pre-existing issue, but could this lead to a NULL pointer
dereference on early interrupts?

In dw_edma_probe(), we call dw_edma_irq_request() which requests the
shared IRQ:

dw_edma_irq_request() {
    ...
    err = request_irq(irq, dw_edma_interrupt_common, IRQF_SHARED, dw->name, &dw->irq[0]);
    ...
}

If an interrupt from another device sharing this IRQ line fires immediately,
the handler will be invoked before we've fully initialized the context.

Specifically, dw_edma_interrupt_common() calls down into
dw_edma_core_handle_int(), which dereferences dw_irq->dw:

dw_edma_core_handle_int() {
    ...
    return dw_irq->dw->core->handle_int(dw_irq, dir, done, abort);
}

However, irq->dw is only initialized later in dw_edma_channel_setup():

dw_edma_channel_setup() {
    ...
    for (i = 0; i < ch_cnt; i++) {
        ...
        irq->dw = dw;
    }
}

If the shared handler fires before dw_edma_channel_setup() runs, will
dw_irq->dw be NULL here and crash the kernel?

> -	if (err)
> +	if (err) {
> +		destroy_workqueue(dw->wq);
>  		return err;
> +	}
>  
>  	/* Allocate a dedicated virtual IRQ for interrupt-emulation doorbells */
>  	err = dw_edma_emul_irq_alloc(dw);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715175740.1110404-1-den@valinux.co.jp?part=9

Re: [PATCH v3 2/9] dmaengine: dw-edma: Terminate all descriptors without callbacks

[Frank Li]

On Thu, Jul 16, 2026 at 02:57:33AM +0900, Koichiro Den wrote:
> The DMA Engine client documentation says in the "Terminate APIs" section
> of Documentation/driver-api/dmaengine/client.rst:
>
> "No callback functions will be called for any incomplete transfers."
>
> dw-edma instead calls vchan_cookie_complete() when a deferred STOP reaches
> the interrupt handler. This schedules a callback for the active descriptor
> and leaves other issued or submitted descriptors queued. A late callback
> after dmaengine_terminate_sync() can dereference client state that has
> already been freed, while leftover descriptors may later restart into
> reused buffers or leak.
>
> Move all issued and submitted descriptors to the terminated list whenever
> termination completes. For a pending STOP, do this from both the DONE and
> ABORT paths. Complete their cookies in order without scheduling callbacks.
>
> A STOP can remain pending until the running transfer raises an interrupt.
> Make device_synchronize() wait until the channel has stopped and
> terminate_all() deconfigures it before releasing terminated descriptors.
> Reuse it from free_chan_resources(), then release the remaining virt-dma
> resources. Sleep instead of busy-polling while waiting, and warn if the
> existing timeout expires.
>
> Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
> Changes in v3:
>   - Merge the descriptor cleanup patch so device_synchronize() is safe
>     when introduced. (Sashiko)
>   - Handle pending STOP requests from the ABORT path as well.
>   - Drop a redundant cond_resched() after usleep_range().
>   - Drop Frank's Reviewed-by tag due to these changes.
>   - Polish a source comment.
>
>  drivers/dma/dw-edma/dw-edma-core.c | 87 ++++++++++++++++++++++++++----
>  1 file changed, 76 insertions(+), 11 deletions(-)
>
...
> +static void dw_edma_wait_termination(struct dma_chan *dchan)
>  {
> +	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
>  	unsigned long timeout = jiffies + msecs_to_jiffies(5000);
> -	int ret;
> +	unsigned long flags;
> +	bool configured = true;
>
> +	/*
> +	 * dw_edma_device_terminate_all() may defer cleanup to a later interrupt
> +	 * while the channel is still running. Retry until the channel is
> +	 * deconfigured, which means termination is complete.
> +	 */
>  	while (time_before(jiffies, timeout)) {
> -		ret = dw_edma_device_terminate_all(dchan);
> -		if (!ret)
> -			break;
> +		dw_edma_device_terminate_all(dchan);
>
> -		if (time_after_eq(jiffies, timeout))
> +		spin_lock_irqsave(&chan->vc.lock, flags);
> +		configured = chan->configured;
> +		spin_unlock_irqrestore(&chan->vc.lock, flags);
> +		if (!configured)
>  			return;
>
> -		cpu_relax();
> +		usleep_range(1000, 2000);

now use fsleep()

Frank

Re: [PATCH v3 3/9] dmaengine: dw-edma: Serialize abort state updates

[Frank Li]

On Thu, Jul 16, 2026 at 02:57:34AM +0900, Koichiro Den wrote:
> dw_edma_abort_interrupt() drops vc.lock before changing request and
> status. issue_pending() can acquire the lock in that small window,
> observe the old busy state, and skip starting queued descriptors. Then
> the abort handler overwrites the channel status as idle, leaving the new
> descriptors stranded for good.
>
> Keep descriptor completion and the state transition in the same critical
> section.
>
> Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v3:
>   - New patch for a pre-existing abort state race. (Sashiko)
>
>  drivers/dma/dw-edma/dw-edma-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 44ef5fbe3fd4..272b03405746 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -740,9 +740,9 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
>  		list_del(&vd->node);
>  		vchan_cookie_complete(vd);
>  	}
> -	spin_unlock_irqrestore(&chan->vc.lock, flags);
>  	chan->request = EDMA_REQ_NONE;
>  	chan->status = EDMA_ST_IDLE;
> +	spin_unlock_irqrestore(&chan->vc.lock, flags);
>  }
>
>  static void dw_edma_emul_irq_ack(struct irq_data *d)
> --
> 2.51.0
>

Re: [PATCH v3 4/9] dmaengine: dw-edma: Complete descriptors before pausing

[Frank Li]

On Thu, Jul 16, 2026 at 02:57:35AM +0900, Koichiro Den wrote:
> If PAUSE is requested while the final chunk of a descriptor is in
> flight, the DONE interrupt takes the PAUSE path without checking whether
> the descriptor has been depleted. The depleted descriptor remains on the
> issued list and the channel enters EDMA_ST_PAUSE.
>
> On resume, dw_edma_start_transfer() finds the descriptor but no chunk to
> start and returns 0. The caller ignores that result and leaves the
> channel stuck in EDMA_ST_BUSY with no transfer running.
>
> Check for descriptor completion before acknowledging PAUSE. If there is
> no work to start on resume, leave the channel idle. Also ignore DONE
> interrupts while the channel is paused so a stale or repeated interrupt
> cannot change its state or start queued work.
>
> Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v3:
>   - New patch for a pre-existing PAUSE completion issue. (Sashiko)
>
>  drivers/dma/dw-edma/dw-edma-core.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 272b03405746..b06b299661c0 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -327,7 +327,8 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
>  		err = -EPERM;
>  	} else {
>  		chan->status = EDMA_ST_BUSY;
> -		dw_edma_start_transfer(chan);
> +		if (!dw_edma_start_transfer(chan))
> +			chan->status = EDMA_ST_IDLE;
>  	}
>
>  	return err;
> @@ -691,10 +692,16 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
>  	unsigned long flags;
>
>  	spin_lock_irqsave(&chan->vc.lock, flags);
> +	if (chan->status == EDMA_ST_PAUSE) {
> +		spin_unlock_irqrestore(&chan->vc.lock, flags);
> +		return;
> +	}
> +
>  	vd = vchan_next_desc(&chan->vc);
>  	if (vd) {
>  		switch (chan->request) {
>  		case EDMA_REQ_NONE:
> +		case EDMA_REQ_PAUSE:
>  			desc = vd2dw_edma_desc(vd);
>  			if (!desc->chunks_alloc) {
>  				dw_hdma_set_callback_result(vd,
> @@ -703,6 +710,12 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
>  				vchan_cookie_complete(vd);
>  			}
>
> +			if (chan->request == EDMA_REQ_PAUSE) {
> +				chan->request = EDMA_REQ_NONE;
> +				chan->status = EDMA_ST_PAUSE;
> +				break;
> +			}
> +
>  			/* Continue transferring if there are remaining chunks or issued requests.
>  			 */
>  			chan->status = dw_edma_start_transfer(chan) ? EDMA_ST_BUSY : EDMA_ST_IDLE;
> @@ -714,11 +727,6 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
>  			chan->status = EDMA_ST_IDLE;
>  			break;
>
> -		case EDMA_REQ_PAUSE:
> -			chan->request = EDMA_REQ_NONE;
> -			chan->status = EDMA_ST_PAUSE;
> -			break;
> -
>  		default:
>  			break;
>  		}
> --
> 2.51.0
>

Re: [PATCH v3 5/9] dmaengine: dw-edma: Serialize channel state checks

[Frank Li]

On Thu, Jul 16, 2026 at 02:57:36AM +0900, Koichiro Den wrote:
> pause() and resume() read and update channel state without holding vc.lock,
> while the interrupt handlers update the same state under it. Take the same
> lock around those state checks so that request, status, and configured stay
> consistent.
>
> For example, pause() can observe EDMA_ST_BUSY right before the interrupt
> handler completes the final descriptor and moves the channel to
> EDMA_ST_IDLE, and then record EDMA_REQ_PAUSE on an already idle channel. No
> further interrupt will acknowledge the request, and since issue_pending()
> requires EDMA_REQ_NONE, the channel is wedged for good: terminate_all()
> leaves the stale request behind, so even reconfiguring the channel does not
> recover it.
>
> issue_pending() already runs under vc.lock, but it tests configured before
> taking it. Move that test under the lock as well, so that the decision to
> start work is made against the current value rather than one observed
> before a concurrent terminate_all() deconfigured the channel.
>
> Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
> Changes in v3:
>   - No changes.
>
>  drivers/dma/dw-edma/dw-edma-core.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index b06b299661c0..8b0e2af734da 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -300,8 +300,10 @@ static int dw_edma_device_config(struct dma_chan *dchan,
>  static int dw_edma_device_pause(struct dma_chan *dchan)
>  {
>  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> +	unsigned long flags;
>  	int err = 0;
>
> +	spin_lock_irqsave(&chan->vc.lock, flags);

new code, can you use clean up guard()

Frank

>  	if (!chan->configured)
>  		err = -EPERM;
>  	else if (chan->status != EDMA_ST_BUSY)
> @@ -310,6 +312,7 @@ static int dw_edma_device_pause(struct dma_chan *dchan)
>  		err = -EPERM;
>  	else
>  		chan->request = EDMA_REQ_PAUSE;
> +	spin_unlock_irqrestore(&chan->vc.lock, flags);
>
>  	return err;
>  }
> @@ -317,8 +320,10 @@ static int dw_edma_device_pause(struct dma_chan *dchan)
>  static int dw_edma_device_resume(struct dma_chan *dchan)
>  {
>  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> +	unsigned long flags;
>  	int err = 0;
>
> +	spin_lock_irqsave(&chan->vc.lock, flags);
>  	if (!chan->configured) {
>  		err = -EPERM;
>  	} else if (chan->status != EDMA_ST_PAUSE) {
> @@ -330,6 +335,7 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
>  		if (!dw_edma_start_transfer(chan))
>  			chan->status = EDMA_ST_IDLE;
>  	}
> +	spin_unlock_irqrestore(&chan->vc.lock, flags);
>
>  	return err;
>  }
> @@ -373,11 +379,9 @@ static void dw_edma_device_issue_pending(struct dma_chan *dchan)
>  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
>  	unsigned long flags;
>
> -	if (!chan->configured)
> -		return;
> -
>  	spin_lock_irqsave(&chan->vc.lock, flags);
> -	if (vchan_issue_pending(&chan->vc) && chan->request == EDMA_REQ_NONE &&
> +	if (chan->configured && vchan_issue_pending(&chan->vc) &&
> +	    chan->request == EDMA_REQ_NONE &&
>  	    chan->status == EDMA_ST_IDLE) {
>  		chan->status = EDMA_ST_BUSY;
>  		dw_edma_start_transfer(chan);
> --
> 2.51.0
>

Re: [PATCH v3 6/9] dmaengine: dw-edma: Clear stale requests on termination

[Frank Li]

On Thu, Jul 16, 2026 at 02:57:37AM +0900, Koichiro Den wrote:
> terminate_all() can deconfigure a channel immediately when it is already
> unconfigured, paused, idle, or stopped in hardware. A pending PAUSE
> request can survive these paths and block issue_pending() after the
> channel is configured again.
>
> Clear request whenever termination leaves the channel deconfigured. A
> running channel remains configured while its STOP request is pending, so
> the interrupt handler can still consume that request.
>
> Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
> Signed-off-by: Koichiro Den <den@valinux.co.jp>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
> Changes in v3:
>   - New patch for a pre-existing stale request issue. (Sashiko)
>
>  drivers/dma/dw-edma/dw-edma-core.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 8b0e2af734da..2369b4c4630b 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -369,6 +369,8 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
>  	} else {
>  		chan->request = EDMA_REQ_STOP;
>  	}
> +	if (!chan->configured)
> +		chan->request = EDMA_REQ_NONE;
>  	spin_unlock_irqrestore(&chan->vc.lock, flags);
>
>  	return err;
> --
> 2.51.0
>

Re: [PATCH v3 9/9] dmaengine: dw-edma: Defer channel IRQ handling to workqueue

[Frank Li]

On Thu, Jul 16, 2026 at 02:57:40AM +0900, Koichiro Den wrote:
> On some SoCs (e.g. R-Car S4) the endpoint-side eDMA raises a single
> fixed SPI that is hardwired to CPU0 and covers every read and write
> channel. Handling channel events directly in that hard IRQ context
> serializes the completion processing of all channels on one CPU:
> descriptor recycling and refill, client callbacks (the vchan tasklet
> runs on the scheduling CPU) and the doorbell writes all funnel through
> CPU0, while the handler additionally spins on each channel's vc.lock.
> Especially under heavy multichannel load, this contention becomes a
> performance bottleneck.
>
> Keep the hard IRQ handler minimal: clear the status, dispatch channel
> events, and defer per-channel processing to work items. A work item per
> channel preserves ordering while allowing different channels to run in
> parallel on any CPU.
>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v3:
>   - Clear irq_pending in dw_edma_device_synchronize(). (Sashiko)
>   - Drop Frank's Reviewed-by tag due to this change.
>   - Refine the commit message and source comments.
>
>  drivers/dma/dw-edma/dw-edma-core.c | 69 +++++++++++++++++++++++++++---
>  drivers/dma/dw-edma/dw-edma-core.h | 11 +++++
>  2 files changed, 75 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 2369b4c4630b..4ee7f14c3a82 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -30,6 +30,11 @@ struct dw_edma_desc *vd2dw_edma_desc(struct virt_dma_desc *vd)
>  	return container_of(vd, struct dw_edma_desc, vd);
>  }
>
> +enum dw_edma_irq_event {
> +	DW_EDMA_IRQ_DONE	= BIT(0),
> +	DW_EDMA_IRQ_ABORT	= BIT(1),
> +};
> +
>  static inline
>  u64 dw_edma_get_pci_address(struct dw_edma_chan *chan, phys_addr_t cpu_addr)
>  {
> @@ -759,6 +764,39 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
>  	spin_unlock_irqrestore(&chan->vc.lock, flags);
>  }
>
> +static void dw_edma_irq_work(struct work_struct *work)
> +{
> +	struct dw_edma_chan *chan = container_of(work, struct dw_edma_chan,
> +						 irq_work);
> +	unsigned int events;
> +
> +	do {
> +		events = atomic_xchg(&chan->irq_pending, 0);
> +
> +		if (events & DW_EDMA_IRQ_DONE)
> +			dw_edma_done_interrupt(chan);
> +		if (events & DW_EDMA_IRQ_ABORT)
> +			dw_edma_abort_interrupt(chan);
> +	} while (atomic_read(&chan->irq_pending));
> +}
> +
> +static void dw_edma_queue_irq_work(struct dw_edma_chan *chan,
> +				   enum dw_edma_irq_event event)
> +{
> +	atomic_or(event, &chan->irq_pending);
> +	queue_work(chan->dw->wq, &chan->irq_work);
> +}
> +
> +static void dw_edma_done_interrupt_deferred(struct dw_edma_chan *chan)
> +{
> +	dw_edma_queue_irq_work(chan, DW_EDMA_IRQ_DONE);
> +}
> +
> +static void dw_edma_abort_interrupt_deferred(struct dw_edma_chan *chan)
> +{
> +	dw_edma_queue_irq_work(chan, DW_EDMA_IRQ_ABORT);
> +}
> +
>  static void dw_edma_emul_irq_ack(struct irq_data *d)
>  {
>  	struct dw_edma *dw = irq_data_get_irq_chip_data(d);
> @@ -853,8 +891,8 @@ static inline irqreturn_t dw_edma_interrupt_write_inner(int irq, void *data)
>  	struct dw_edma_irq *dw_irq = data;
>
>  	return dw_edma_core_handle_int(dw_irq, EDMA_DIR_WRITE,
> -				       dw_edma_done_interrupt,
> -				       dw_edma_abort_interrupt);
> +				       dw_edma_done_interrupt_deferred,
> +				       dw_edma_abort_interrupt_deferred);
>  }
>
>  static inline irqreturn_t dw_edma_interrupt_read_inner(int irq, void *data)
> @@ -862,8 +900,8 @@ static inline irqreturn_t dw_edma_interrupt_read_inner(int irq, void *data)
>  	struct dw_edma_irq *dw_irq = data;
>
>  	return dw_edma_core_handle_int(dw_irq, EDMA_DIR_READ,
> -				       dw_edma_done_interrupt,
> -				       dw_edma_abort_interrupt);
> +				       dw_edma_done_interrupt_deferred,
> +				       dw_edma_abort_interrupt_deferred);
>  }
>
>  static inline irqreturn_t dw_edma_interrupt_write(int irq, void *data)
> @@ -940,6 +978,8 @@ static void dw_edma_device_synchronize(struct dma_chan *dchan)
>  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
>
>  	dw_edma_wait_termination(dchan);
> +	cancel_work_sync(&chan->irq_work);
> +	atomic_set(&chan->irq_pending, 0);
>  	vchan_synchronize(&chan->vc);
>  }
>
> @@ -982,6 +1022,8 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
>  		chan->configured = false;
>  		chan->request = EDMA_REQ_NONE;
>  		chan->status = EDMA_ST_IDLE;
> +		INIT_WORK(&chan->irq_work, dw_edma_irq_work);
> +		atomic_set(&chan->irq_pending, 0);
>
>  		if (chan->dir == EDMA_DIR_WRITE)
>  			chan->ll_max = (chip->ll_region_wr[chan->id].sz / EDMA_LL_SZ);
> @@ -1195,10 +1237,21 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>  	/* Disable eDMA, only to establish the ideal initial conditions */
>  	dw_edma_core_off(dw);
>
> +	/*
> +	 * Deferred IRQ works are queued from the hard IRQ handlers, so the
> +	 * workqueue must exist before any IRQ is requested.
> +	 */
> +	dw->wq = alloc_workqueue("dw-edma:%s", WQ_UNBOUND | WQ_HIGHPRI, 0,
> +				 dev_name(chip->dev));
> +	if (!dw->wq)
> +		return -ENOMEM;
> +
>  	/* Request IRQs */
>  	err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
> -	if (err)
> +	if (err) {
> +		destroy_workqueue(dw->wq);
>  		return err;
> +	}
>
>  	/* Allocate a dedicated virtual IRQ for interrupt-emulation doorbells */
>  	err = dw_edma_emul_irq_alloc(dw);
> @@ -1221,6 +1274,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>  	for (i = (dw->nr_irqs - 1); i >= 0; i--)
>  		free_irq(chip->ops->irq_vector(dev, i), &dw->irq[i]);
>  	dw_edma_emul_irq_free(dw);
> +	destroy_workqueue(dw->wq);
>
>  	return err;
>  }
> @@ -1245,6 +1299,11 @@ int dw_edma_remove(struct dw_edma_chip *chip)
>  		free_irq(chip->ops->irq_vector(dev, i), &dw->irq[i]);
>  	dw_edma_emul_irq_free(dw);
>
> +	for (i = 0; i < dw->wr_ch_cnt + dw->rd_ch_cnt; i++)
> +		cancel_work_sync(&dw->chan[i].irq_work);
> +
> +	destroy_workqueue(dw->wq);
> +
>  	/* Deregister eDMA device */
>  	dma_async_device_unregister(&dw->dma);
>  	list_for_each_entry_safe(chan, _chan, &dw->dma.channels,
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index 6474cacf7195..7f6301572fa4 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -9,8 +9,10 @@
>  #ifndef _DW_EDMA_CORE_H
>  #define _DW_EDMA_CORE_H
>
> +#include <linux/atomic.h>
>  #include <linux/msi.h>
>  #include <linux/dma/edma.h>
> +#include <linux/workqueue.h>
>
>  #include "../virt-dma.h"
>
> @@ -87,6 +89,9 @@ struct dw_edma_chan {
>
>  	struct dma_slave_config		config;
>  	bool				non_ll;
> +
> +	struct work_struct		irq_work;
> +	atomic_t			irq_pending;
>  };
>
>  struct dw_edma_irq {
> @@ -109,6 +114,12 @@ struct dw_edma {
>
>  	struct dw_edma_chan		*chan;
>
> +	/*
> +	 * WQ_HIGHPRI keeps completion processing responsive under heavy load;
> +	 * WQ_UNBOUND lets different channels run on different CPUs.
> +	 */
> +	struct workqueue_struct		*wq;
> +
>  	raw_spinlock_t			lock;		/* Protect v0 shared registers */
>
>  	struct dw_edma_chip             *chip;
> --
> 2.51.0
>