[PATCH v4 0/7] dmaengine: dw: serialize completion and termination

[PATCH v4 0/7] dmaengine: dw: serialize completion and termination

[James Hilliard]

Keep client callbacks in the controller tasklet while preserving polled
completion. Serialize descriptor ownership across completion, error
recovery and termination, and wait for callbacks before a client reuses
its resources.

The first two patches serialize channel-completion and error-recovery
handling with termination. The next two complete cookies under the channel
lock and separate residue calculation from completion processing. Channel
release and device_synchronize then gain tasklet draining before the final
patch adds the deferred completion list and tasklet-only notification.

The series adapts Serge Semin's channel-restart and callback fixes [0-2].
Polling continues to advance cookies and software-LLP blocks, while client
notifications run from the controller tasklet. Synchronization preserves
pending tasklet runs from other channels.

[0] https://lore.kernel.org/r/20240911184710.4207-1-fancer.lancer@gmail.com
[1] https://lore.kernel.org/r/20240911184710.4207-2-fancer.lancer@gmail.com
[2] https://lore.kernel.org/r/20240911184710.4207-3-fancer.lancer@gmail.com

Assisted-by: Codex:gpt-6-astra
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

---
Changes in v4:
- Split v3's callback-deferral patch into cookie accounting, residue
  calculation, channel-release draining and callback-deferral patches,
  following Frank's review. The resulting seven-patch series preserves
  v3's final implementation.
- Place device_synchronize before callback deferral so clients can drain
  deferred notifications when the completion list is introduced.
- Carry Frank's Reviewed-by tags on the unchanged channel-completion,
  error-recovery and device_synchronize patches.
- Link to v3: https://patch.msgid.link/20261023-submit-dw-dma-synchronize-v1-v3-0-c62bf048f5ce@gmail.com

Changes in v3:
- Order channel-completion and error-recovery locking before callback
  deferral, retaining four patches.
- Require a locked RAW.XFER event before retiring a transfer; zero source
  residue alone cannot establish destination completion.
- Preserve polled completion and software-LLP progress in tx_status(),
  queuing client notifications for the controller tasklet.
- Serialize cookie updates with termination and report queued residue.
- Drain deferred notifications on channel release and let a pending
  tasklet return without MMIO after the last channel is released.
- Adapt Serge's two patches and retain his authorship and revision notes.
  Drop the Tested-by tags that covered their earlier implementations.
- Identify the introduction of the synchronization API in the final
  patch's Fixes tag.
- Link to v2: https://patch.msgid.link/20261022-submit-dw-dma-synchronize-v1-v2-0-5244e94dca8c@gmail.com

Changes in v2:
- Carry Serge Semin's two posted fixes before the synchronization change,
  preserving his authorship and revision notes. Add the requested stable
  Cc trailers and Andy Shevchenko's Tested-by tags to those unchanged fixes.
- Add a separate error-recovery fix for stale error status, empty channel
  lists and software-LLP state when terminating or restarting a channel.
- Use tasklet_disable()/tasklet_enable() instead of tasklet_kill() so
  synchronization preserves scheduling requests from other channels.
- Explain descriptor ownership during termination and acknowledge the
  shared-tasklet scheduling race reported by Sashiko.
- Link to v1: https://patch.msgid.link/20261021-submit-dw-dma-synchronize-v1-v1-1-6af85c2d6f69@gmail.com

Revision history from Serge's series:

Link: https://lore.kernel.org/dmaengine/20240802080756.7415-1-fancer.lancer@gmail.com/
Changelog RFC:
- Add a new patch:
  [PATCH 2/2] dmaengine: dw: Fix XFER bit set, but channel not idle error
  fixing the "XFER bit set, but channel not idle" error.
- Instead of just dropping the dwc_scan_descriptors() method invocation
  calculate the residue in the Tx-status getter.

---
James Hilliard (4):
      dmaengine: dw: serialize error recovery with channel termination
      dmaengine: dw: complete cookies before releasing the channel lock
      dmaengine: dw: drain the tasklet before releasing channel resources
      dmaengine: dw: synchronize completion callbacks

Serge Semin (3):
      dmaengine: dw: Fix XFER bit set, but channel not idle error
      dmaengine: dw: calculate residue separately from completion
      dmaengine: dw: Prevent tx-status calling DMA-desc callback

 drivers/dma/dw/core.c | 307 ++++++++++++++++++++++++++------------------------
 drivers/dma/dw/regs.h |   1 +
 2 files changed, 160 insertions(+), 148 deletions(-)
---
base-commit: 93f51579e7df248780214094418f205253383cc5
change-id: 20261021-submit-dw-dma-synchronize-v1-e550f47fd1da

Best regards,
--  
James Hilliard <james.hilliard1@gmail.com>

[PATCH v4 1/7] dmaengine: dw: Fix XFER bit set, but channel not idle error

[James Hilliard]

From: Serge Semin <fancer.lancer@gmail.com>

If a client driver gets to use the DW DMAC engine device tougher
than usual, with occasional DMA-transfers termination and restart, then
the next error can be randomly spotted in the system log:

> dma dma0chan0: BUG: XFER bit set, but channel not idle!

For instance that happens in case of the 8250 UART port driver handling
the looped back high-speed traffic (in my case > 1.5Mbaud) by means of the
DMA-engine interface.

The error happens due to the two-staged nature of the DW DMAC IRQs
handling procedure and due to the critical section break in the meantime.
In particular in case if the DMA-transfer is terminated and restarted:
1. after the IRQ-handler submitted the tasklet but before the tasklet
   started handling the DMA-descriptors in dwc_scan_descriptors();
2. after the XFER completion flag was detected in the
   dwc_scan_descriptors() method, but before the dwc_complete_all() method
   is called
the error denoted above is printed due to the overlap of the last transfer
completion and the new transfer execution stages.

Clear the channel interrupt status when stopping it, and keep the XFER
check, descriptor detachment and queued-transfer restart in one locked
section. An empty active list needs no completion processing.

When the locked XFER recheck finds no event, only update residue. A stale
tasklet snapshot may belong to a terminated transfer, and a zero residue
only means the source has been read, not that destination writes have
finished. Requiring RAW.XFER also prevents the fallback scan from freeing
a descriptor still linked on active_list.

Fixes: 69cea5a00d31 ("dmaengine/dw_dmac: Replace spin_lock* with irqsave variants and enable submission from callback")
Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
Cc: stable@vger.kernel.org
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
[james: require a locked XFER event and handle an empty active list]
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/dma/dw/core.c | 70 ++++++++++++++++++++-------------------------------
 1 file changed, 27 insertions(+), 43 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index dd75f97a33b3..07a5f5105787 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -141,6 +141,12 @@ static inline void dwc_chan_disable(struct dw_dma *dw, struct dw_dma_chan *dwc)
 	channel_clear_bit(dw, CH_EN, dwc->mask);
 	while (dma_readl(dw, CH_EN) & dwc->mask)
 		cpu_relax();
+
+	dma_writel(dw, CLEAR.XFER, dwc->mask);
+	dma_writel(dw, CLEAR.BLOCK, dwc->mask);
+	dma_writel(dw, CLEAR.SRC_TRAN, dwc->mask);
+	dma_writel(dw, CLEAR.DST_TRAN, dwc->mask);
+	dma_writel(dw, CLEAR.ERROR, dwc->mask);
 }
 
 /*----------------------------------------------------------------------*/
@@ -257,34 +263,6 @@ dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc,
 	dmaengine_desc_callback_invoke(&cb, NULL);
 }
 
-static void dwc_complete_all(struct dw_dma *dw, struct dw_dma_chan *dwc)
-{
-	struct dw_desc *desc, *_desc;
-	LIST_HEAD(list);
-	unsigned long flags;
-
-	spin_lock_irqsave(&dwc->lock, flags);
-	if (dma_readl(dw, CH_EN) & dwc->mask) {
-		dev_err(chan2dev(&dwc->chan),
-			"BUG: XFER bit set, but channel not idle!\n");
-
-		/* Try to continue after resetting the channel... */
-		dwc_chan_disable(dw, dwc);
-	}
-
-	/*
-	 * Submit queued descriptors ASAP, i.e. before we go through
-	 * the completed ones.
-	 */
-	list_splice_init(&dwc->active_list, &list);
-	dwc_dostart_first_queued(dwc);
-
-	spin_unlock_irqrestore(&dwc->lock, flags);
-
-	list_for_each_entry_safe(desc, _desc, &list, desc_node)
-		dwc_descriptor_complete(dwc, desc, true);
-}
-
 /* Returns how many bytes were already received from source */
 static inline u32 dwc_get_sent(struct dw_dma_chan *dwc)
 {
@@ -302,6 +280,7 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 	struct dw_desc *child;
 	u32 status_xfer;
 	unsigned long flags;
+	LIST_HEAD(list);
 
 	spin_lock_irqsave(&dwc->lock, flags);
 	llp = channel_readl(dwc, LLP);
@@ -310,6 +289,10 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 	if (status_xfer & dwc->mask) {
 		/* Everything we've submitted is done */
 		dma_writel(dw, CLEAR.XFER, dwc->mask);
+		if (list_empty(&dwc->active_list)) {
+			spin_unlock_irqrestore(&dwc->lock, flags);
+			return;
+		}
 
 		if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags)) {
 			struct list_head *head, *active = dwc->tx_node_active;
@@ -341,9 +324,23 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 			clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
 		}
 
+		if (dma_readl(dw, CH_EN) & dwc->mask) {
+			dev_err(chan2dev(&dwc->chan),
+				"BUG: XFER bit set, but channel not idle!\n");
+
+			/* Try to continue after resetting the channel... */
+			dwc_chan_disable(dw, dwc);
+		}
+
+		/* Detach the completed transfer before starting queued work. */
+		list_splice_init(&dwc->active_list, &list);
+		dwc_dostart_first_queued(dwc);
 		spin_unlock_irqrestore(&dwc->lock, flags);
 
-		dwc_complete_all(dw, dwc);
+		list_for_each_entry_safe(desc, _desc, &list, desc_node) {
+			list_del_init(&desc->desc_node);
+			dwc_descriptor_complete(dwc, desc, true);
+		}
 		return;
 	}
 
@@ -389,22 +386,9 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 			desc->residue -= child->len;
 		}
 
-		/*
-		 * No descriptors so far seem to be in progress, i.e.
-		 * this one must be done.
-		 */
-		spin_unlock_irqrestore(&dwc->lock, flags);
-		dwc_descriptor_complete(dwc, desc, true);
-		spin_lock_irqsave(&dwc->lock, flags);
 	}
 
-	dev_err(chan2dev(&dwc->chan),
-		"BUG: All descriptors done, but channel not idle!\n");
-
-	/* Try to continue after resetting the channel... */
-	dwc_chan_disable(dw, dwc);
-
-	dwc_dostart_first_queued(dwc);
+	/* Residue describes source reads; only RAW.XFER completes a transfer. */
 	spin_unlock_irqrestore(&dwc->lock, flags);
 }
 

-- 
2.53.0

[PATCH v4 2/7] dmaengine: dw: serialize error recovery with channel termination

[James Hilliard]

The tasklet samples RAW.ERROR before taking the channel lock. A
concurrent termination can empty the active list before dwc_handle_error()
uses that cached error to select a descriptor. The list head is then
interpreted as a descriptor. Even with an active descriptor, moving
queue.next unconditionally can put an empty queue's head on the active
list and pass it to dwc_dostart().

Recheck the error status under the channel lock and stop the channel
before selecting its failed descriptor. Only one transaction is issued
at a time, so take that descriptor directly instead of scanning for
completions first. The scan can otherwise complete it and start queued
work before the error handler has identified the failed transaction.

Detach the failed descriptor under the same lock and use the queued-start
helper, which leaves an empty queue alone. Clear the software-LLP flag
before restarting so a failed multiblock transfer does not prevent the
next transaction from starting. Keep the completion callback outside the
lock, with the detached descriptor owned by the tasklet.

Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
Fixes: f336e42f73d9 ("dw_dmac: Move single descriptor from dwc->queue to dwc->active_list in dwc_complete_all")
Cc: stable@vger.kernel.org
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/dma/dw/core.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 07a5f5105787..51c5460b4670 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -408,10 +408,17 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
 	struct dw_desc *child;
 	unsigned long flags;
 
-	dwc_scan_descriptors(dw, dwc);
-
 	spin_lock_irqsave(&dwc->lock, flags);
 
+	/* Termination may have cleared the error since the tasklet sampled it. */
+	if (!(dma_readl(dw, RAW.ERROR) & dwc->mask))
+		goto unlock;
+
+	dwc_chan_disable(dw, dwc);
+	clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
+	if (list_empty(&dwc->active_list))
+		goto unlock;
+
 	/*
 	 * The descriptor currently at the head of the active list is
 	 * borked. Since we don't have any way to report errors, we'll
@@ -419,12 +426,7 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
 	 */
 	bad_desc = dwc_first_active(dwc);
 	list_del_init(&bad_desc->desc_node);
-	list_move(dwc->queue.next, dwc->active_list.prev);
-
-	/* Clear the error flag and try to restart the controller */
-	dma_writel(dw, CLEAR.ERROR, dwc->mask);
-	if (!list_empty(&dwc->active_list))
-		dwc_dostart(dwc, dwc_first_active(dwc));
+	dwc_dostart_first_queued(dwc);
 
 	/*
 	 * WARN may seem harsh, but since this only happens
@@ -443,6 +445,10 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
 
 	/* Pretend the descriptor completed successfully */
 	dwc_descriptor_complete(dwc, bad_desc, true);
+	return;
+
+unlock:
+	spin_unlock_irqrestore(&dwc->lock, flags);
 }
 
 static void dw_dma_tasklet(struct tasklet_struct *t)

-- 
2.53.0

[PATCH v4 3/7] dmaengine: dw: complete cookies before releasing the channel lock

[James Hilliard]

Completion detaches a descriptor from the active list under the channel
lock, but updates its cookie later when freeing it. Termination can take
the lock in between and complete newer descriptors. Completing the older
cookie afterwards moves completed_cookie backwards and makes an already
terminated transfer appear outstanding again.

Complete cookies while detaching descriptors in the completion, error
recovery and termination paths. Keep the active-before-queued ordering
when terminating so completed_cookie advances in submission order.
Descriptor freeing and client notification no longer update cookies.

Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
Cc: stable@vger.kernel.org
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/dma/dw/core.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 51c5460b4670..1f7afcd01565 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -236,18 +236,17 @@ static void dwc_dostart_first_queued(struct dw_dma_chan *dwc)
 /*----------------------------------------------------------------------*/
 
 static void
-dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc,
-		bool callback_required)
+dwc_descriptor_free(struct dw_dma_chan *dwc, struct dw_desc *desc,
+		    bool callback_required)
 {
 	struct dma_async_tx_descriptor	*txd = &desc->txd;
 	struct dw_desc			*child;
 	unsigned long			flags;
 	struct dmaengine_desc_callback	cb;
 
-	dev_vdbg(chan2dev(&dwc->chan), "descriptor %u complete\n", txd->cookie);
+	dev_vdbg(chan2dev(&dwc->chan), "freeing descriptor %p\n", desc);
 
 	spin_lock_irqsave(&dwc->lock, flags);
-	dma_cookie_complete(txd);
 	if (callback_required)
 		dmaengine_desc_get_callback(txd, &cb);
 	else
@@ -334,12 +333,14 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 
 		/* Detach the completed transfer before starting queued work. */
 		list_splice_init(&dwc->active_list, &list);
+		list_for_each_entry(desc, &list, desc_node)
+			dma_cookie_complete(&desc->txd);
 		dwc_dostart_first_queued(dwc);
 		spin_unlock_irqrestore(&dwc->lock, flags);
 
 		list_for_each_entry_safe(desc, _desc, &list, desc_node) {
 			list_del_init(&desc->desc_node);
-			dwc_descriptor_complete(dwc, desc, true);
+			dwc_descriptor_free(dwc, desc, true);
 		}
 		return;
 	}
@@ -441,10 +442,11 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
 	list_for_each_entry(child, &bad_desc->tx_list, desc_node)
 		dwc_dump_lli(dwc, child);
 
+	/* Pretend the descriptor completed successfully */
+	dma_cookie_complete(&bad_desc->txd);
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
-	/* Pretend the descriptor completed successfully */
-	dwc_descriptor_complete(dwc, bad_desc, true);
+	dwc_descriptor_free(dwc, bad_desc, true);
 	return;
 
 unlock:
@@ -945,12 +947,16 @@ static int dwc_terminate_all(struct dma_chan *chan)
 	/* active_list entries will end up before queued entries */
 	list_splice_init(&dwc->queue, &list);
 	list_splice_init(&dwc->active_list, &list);
+	list_for_each_entry(desc, &list, desc_node)
+		dma_cookie_complete(&desc->txd);
 
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
 	/* Flush all pending and queued descriptors */
-	list_for_each_entry_safe(desc, _desc, &list, desc_node)
-		dwc_descriptor_complete(dwc, desc, false);
+	list_for_each_entry_safe(desc, _desc, &list, desc_node) {
+		list_del_init(&desc->desc_node);
+		dwc_descriptor_free(dwc, desc, false);
+	}
 
 	return 0;
 }

-- 
2.53.0

[PATCH v4 4/7] dmaengine: dw: calculate residue separately from completion

[James Hilliard]

From: Serge Semin <fancer.lancer@gmail.com>

The descriptor scanner mixes transfer-complete handling with hardware-LLP
residue calculation. A status query needs the latter for the requested
cookie even when no transfer-complete event is pending.

Move hardware-LLP residue calculation into its own helper and call it from
the status getter under the channel lock. Keep software-LLP progress in
the scanner and subtract the current block's progress when reporting its
residue. The scanner now handles transfer-complete events only, retaining
the distinction between zero source residue and destination completion.

Also search the queued descriptors so a transfer waiting to start reports
its full length instead of zero residue.

Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
Cc: stable@vger.kernel.org
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
[james: retain polled completion and report queued residue]
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

---

Changelog RFC:
- Instead of just dropping the dwc_scan_descriptors() method invocation
  calculate the residue in the Tx-status getter.
---
 drivers/dma/dw/core.c | 166 +++++++++++++++++++++-----------------------------
 1 file changed, 68 insertions(+), 98 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 1f7afcd01565..1d6419e1cd3c 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -274,123 +274,60 @@ static inline u32 dwc_get_sent(struct dw_dma_chan *dwc)
 
 static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 {
-	dma_addr_t llp;
 	struct dw_desc *desc, *_desc;
 	struct dw_desc *child;
-	u32 status_xfer;
 	unsigned long flags;
 	LIST_HEAD(list);
 
 	spin_lock_irqsave(&dwc->lock, flags);
-	llp = channel_readl(dwc, LLP);
-	status_xfer = dma_readl(dw, RAW.XFER);
 
-	if (status_xfer & dwc->mask) {
-		/* Everything we've submitted is done */
-		dma_writel(dw, CLEAR.XFER, dwc->mask);
-		if (list_empty(&dwc->active_list)) {
-			spin_unlock_irqrestore(&dwc->lock, flags);
-			return;
-		}
-
-		if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags)) {
-			struct list_head *head, *active = dwc->tx_node_active;
-
-			/*
-			 * We are inside first active descriptor.
-			 * Otherwise something is really wrong.
-			 */
-			desc = dwc_first_active(dwc);
-
-			head = &desc->tx_list;
-			if (active != head) {
-				/* Update residue to reflect last sent descriptor */
-				if (active == head->next)
-					desc->residue -= desc->len;
-				else
-					desc->residue -= to_dw_desc(active->prev)->len;
-
-				child = to_dw_desc(active);
+	if (!(dma_readl(dw, RAW.XFER) & dwc->mask))
+		goto unlock;
 
-				/* Submit next block */
-				dwc_do_single_block(dwc, child);
+	dma_writel(dw, CLEAR.XFER, dwc->mask);
+	if (list_empty(&dwc->active_list))
+		goto unlock;
 
-				spin_unlock_irqrestore(&dwc->lock, flags);
-				return;
-			}
+	desc = dwc_first_active(dwc);
+	if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags)) {
+		struct list_head *head = &desc->tx_list;
+		struct list_head *active = dwc->tx_node_active;
 
-			/* We are done here */
-			clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
-		}
+		if (active != head) {
+			/* Update residue to reflect last sent descriptor. */
+			if (active == head->next)
+				desc->residue -= desc->len;
+			else
+				desc->residue -= to_dw_desc(active->prev)->len;
 
-		if (dma_readl(dw, CH_EN) & dwc->mask) {
-			dev_err(chan2dev(&dwc->chan),
-				"BUG: XFER bit set, but channel not idle!\n");
+			child = to_dw_desc(active);
 
-			/* Try to continue after resetting the channel... */
-			dwc_chan_disable(dw, dwc);
+			/* Submit next block, including for polled transfers. */
+			dwc_do_single_block(dwc, child);
+			goto unlock;
 		}
 
-		/* Detach the completed transfer before starting queued work. */
-		list_splice_init(&dwc->active_list, &list);
-		list_for_each_entry(desc, &list, desc_node)
-			dma_cookie_complete(&desc->txd);
-		dwc_dostart_first_queued(dwc);
-		spin_unlock_irqrestore(&dwc->lock, flags);
-
-		list_for_each_entry_safe(desc, _desc, &list, desc_node) {
-			list_del_init(&desc->desc_node);
-			dwc_descriptor_free(dwc, desc, true);
-		}
-		return;
+		clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
 	}
 
-	if (list_empty(&dwc->active_list)) {
-		spin_unlock_irqrestore(&dwc->lock, flags);
-		return;
-	}
+	if (dma_readl(dw, CH_EN) & dwc->mask) {
+		dev_err(chan2dev(&dwc->chan),
+			"BUG: XFER bit set, but channel not idle!\n");
 
-	if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags)) {
-		dev_vdbg(chan2dev(&dwc->chan), "%s: soft LLP mode\n", __func__);
-		spin_unlock_irqrestore(&dwc->lock, flags);
-		return;
+		/* Try to continue after resetting the channel... */
+		dwc_chan_disable(dw, dwc);
 	}
 
-	dev_vdbg(chan2dev(&dwc->chan), "%s: llp=%pad\n", __func__, &llp);
-
-	list_for_each_entry_safe(desc, _desc, &dwc->active_list, desc_node) {
-		/* Initial residue value */
-		desc->residue = desc->total_len;
-
-		/* Check first descriptors addr */
-		if (desc->txd.phys == DWC_LLP_LOC(llp)) {
-			spin_unlock_irqrestore(&dwc->lock, flags);
-			return;
-		}
-
-		/* Check first descriptors llp */
-		if (lli_read(desc, llp) == llp) {
-			/* This one is currently in progress */
-			desc->residue -= dwc_get_sent(dwc);
-			spin_unlock_irqrestore(&dwc->lock, flags);
-			return;
-		}
-
-		desc->residue -= desc->len;
-		list_for_each_entry(child, &desc->tx_list, desc_node) {
-			if (lli_read(child, llp) == llp) {
-				/* Currently in progress */
-				desc->residue -= dwc_get_sent(dwc);
-				spin_unlock_irqrestore(&dwc->lock, flags);
-				return;
-			}
-			desc->residue -= child->len;
-		}
+	dma_cookie_complete(&desc->txd);
+	list_move_tail(&desc->desc_node, &list);
+	dwc_dostart_first_queued(dwc);
+unlock:
+	spin_unlock_irqrestore(&dwc->lock, flags);
 
+	list_for_each_entry_safe(desc, _desc, &list, desc_node) {
+		list_del_init(&desc->desc_node);
+		dwc_descriptor_free(dwc, desc, true);
 	}
-
-	/* Residue describes source reads; only RAW.XFER completes a transfer. */
-	spin_unlock_irqrestore(&dwc->lock, flags);
 }
 
 static inline void dwc_dump_lli(struct dw_dma_chan *dwc, struct dw_desc *desc)
@@ -969,9 +906,39 @@ static struct dw_desc *dwc_find_desc(struct dw_dma_chan *dwc, dma_cookie_t c)
 		if (desc->txd.cookie == c)
 			return desc;
 
+	list_for_each_entry(desc, &dwc->queue, desc_node)
+		if (desc->txd.cookie == c)
+			return desc;
+
 	return NULL;
 }
 
+static u32 dwc_get_hard_llp_desc_residue(struct dw_dma_chan *dwc, struct dw_desc *desc)
+{
+	u32 residue = desc->total_len;
+	struct dw_desc *child;
+	dma_addr_t llp;
+
+	llp = channel_readl(dwc, LLP);
+
+	/* The first descriptor may still be waiting to be fetched. */
+	if (desc->txd.phys == DWC_LLP_LOC(llp))
+		return residue;
+
+	if (lli_read(desc, llp) == llp)
+		return residue - dwc_get_sent(dwc);
+
+	residue -= desc->len;
+	list_for_each_entry(child, &desc->tx_list, desc_node) {
+		if (lli_read(child, llp) == llp)
+			return residue - dwc_get_sent(dwc);
+
+		residue -= child->len;
+	}
+
+	return residue;
+}
+
 static u32 dwc_get_residue_and_status(struct dw_dma_chan *dwc, dma_cookie_t cookie,
 				      enum dma_status *status)
 {
@@ -984,9 +951,12 @@ static u32 dwc_get_residue_and_status(struct dw_dma_chan *dwc, dma_cookie_t cook
 	desc = dwc_find_desc(dwc, cookie);
 	if (desc) {
 		if (desc == dwc_first_active(dwc)) {
-			residue = desc->residue;
-			if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags) && residue)
+			if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags)) {
+				residue = desc->residue;
 				residue -= dwc_get_sent(dwc);
+			} else {
+				residue = dwc_get_hard_llp_desc_residue(dwc, desc);
+			}
 			if (test_bit(DW_DMA_IS_PAUSED, &dwc->flags))
 				*status = DMA_PAUSED;
 		} else {

-- 
2.53.0

[PATCH v4 5/7] dmaengine: dw: drain the tasklet before releasing channel resources

[James Hilliard]

The controller tasklet invokes callbacks after dropping the channel lock.
Releasing a channel can therefore clear its configuration and disable the
controller while that tasklet run is still using them.

Disable the tasklet before clearing channel resources and re-enable it
after updating the controller's in-use mask. This waits for an executing
run while preserving pending schedules for the other channels. Have a
pending run return before accessing registers when the last channel has
already been released.

Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
Cc: stable@vger.kernel.org
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/dma/dw/core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 1d6419e1cd3c..3bda85adbeee 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -398,6 +398,10 @@ static void dw_dma_tasklet(struct tasklet_struct *t)
 	u32 status_err;
 	unsigned int i;
 
+	/* Channel release may have drained a polled completion before this run. */
+	if (!dw->in_use)
+		return;
+
 	status_xfer = dma_readl(dw, RAW.XFER);
 	status_err = dma_readl(dw, RAW.ERROR);
 
@@ -1067,6 +1071,8 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
 	struct dw_dma		*dw = to_dw_dma(chan->device);
 	unsigned long		flags;
 
+	tasklet_disable(&dw->tasklet);
+
 	dev_dbg(chan2dev(chan), "%s: descs allocated=%u\n", __func__,
 			dwc->descs_allocated);
 
@@ -1092,6 +1098,8 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
 	if (!dw->in_use)
 		do_dw_dma_off(dw);
 
+	tasklet_enable(&dw->tasklet);
+
 	dev_vdbg(chan2dev(chan), "%s: done\n", __func__);
 }
 

-- 
2.53.0

[PATCH v4 6/7] dmaengine: dw: synchronize completion callbacks

[James Hilliard]

The controller tasklet can detach completed descriptors and invoke their
callbacks outside the channel lock. Terminating the channel does not wait
for those callbacks, and there is no device_synchronize hook. A client can
therefore reuse callback state or release its buffers after
dmaengine_synchronize() while an old callback still uses them.

Disable and re-enable the controller tasklet from device_synchronize,
outside the channel lock. Disabling waits for an executing tasklet,
including descriptors it already removed from the channel lists. After
termination, a later tasklet run cannot acquire another descriptor from
the terminated channel. Clients must continue to exclude new submissions
on that channel until synchronization finishes.

The tasklet is shared, so other channels may schedule it while this wait
is in progress. Preserve those pending runs: tasklet_kill() temporarily
owns the scheduling bit and can consume a concurrent scheduling request,
leaving controller interrupts masked. The disable/enable pair leaves the
scheduling bit and pending tasklet queue intact. Other channels' hardware
transfers continue, though their callbacks may be briefly deferred.

Fixes: b36f09c3c441 ("dmaengine: Add transfer termination synchronization support")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/r/sashiko-outbox-180707@kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/dma/dw/core.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 3bda85adbeee..66065d7b26ae 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -902,6 +902,19 @@ static int dwc_terminate_all(struct dma_chan *chan)
 	return 0;
 }
 
+static void dwc_synchronize(struct dma_chan *chan)
+{
+	struct dw_dma *dw = to_dw_dma(chan->device);
+
+	/*
+	 * Termination removes the channel's descriptors from the lists, but a
+	 * running tasklet may still own a completed descriptor and its callback.
+	 * Wait for that run without losing schedules from the other channels.
+	 */
+	tasklet_disable(&dw->tasklet);
+	tasklet_enable(&dw->tasklet);
+}
+
 static struct dw_desc *dwc_find_desc(struct dw_dma_chan *dwc, dma_cookie_t c)
 {
 	struct dw_desc *desc;
@@ -1295,6 +1308,7 @@ int do_dma_probe(struct dw_dma_chip *chip)
 	dw->dma.device_pause = dwc_pause;
 	dw->dma.device_resume = dwc_resume;
 	dw->dma.device_terminate_all = dwc_terminate_all;
+	dw->dma.device_synchronize = dwc_synchronize;
 
 	dw->dma.device_tx_status = dwc_tx_status;
 	dw->dma.device_issue_pending = dwc_issue_pending;

-- 
2.53.0

[PATCH v4 7/7] dmaengine: dw: Prevent tx-status calling DMA-desc callback

[James Hilliard]

From: Serge Semin <fancer.lancer@gmail.com>

The DMA transfer status getter can invoke a completion callback from
dwc_scan_descriptors(). An 8250 RX timeout queries that status while
holding the UART port lock, and its DMA callback takes the same lock:

  serial8250_handle_irq()
    uart_port_lock_irqsave(port)
    serial8250_rx_dma_flush()
      dmaengine_tx_status()
        dwc_scan_descriptors()
          dwc_descriptor_free()
            dma_rx_complete()
              uart_port_lock_irqsave(port)

Queue completed descriptors on a channel completion list under the channel
lock. Let the controller tasklet detach that list and invoke callbacks
outside the lock. Termination discards notifications still on the shared
list; the tasklet owns descriptors it has already detached, and the
synchronization hook waits for that run.

Keep handling raw completion events from tx_status(). Clients such as
catpt poll transfers without DMA_PREP_INTERRUPT and can mask the controller
interrupt externally. Polling must still advance software-emulated LLP
blocks and complete cookies without waiting for tasklet execution. Hold
the channel lock across event handling and status calculation, then
schedule the tasklet for deferred notifications.

On channel release, discard queued notifications after draining the
tasklet, before clearing the controller's in-use state.

Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
Cc: stable@vger.kernel.org
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
[james: defer notifications while preserving polled completion and cleanup]
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

---

Changelog RFC:
- Instead of just dropping the dwc_scan_descriptors() method invocation
  calculate the residue in the Tx-status getter.
---
 drivers/dma/dw/core.c | 111 ++++++++++++++++++++++++++++++--------------------
 drivers/dma/dw/regs.h |   1 +
 2 files changed, 68 insertions(+), 44 deletions(-)

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 66065d7b26ae..d80be93f0c49 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -262,6 +262,22 @@ dwc_descriptor_free(struct dw_dma_chan *dwc, struct dw_desc *desc,
 	dmaengine_desc_callback_invoke(&cb, NULL);
 }
 
+static void dwc_complete(struct dw_dma_chan *dwc)
+{
+	struct dw_desc *desc, *_desc;
+	unsigned long flags;
+	LIST_HEAD(list);
+
+	spin_lock_irqsave(&dwc->lock, flags);
+	list_splice_tail_init(&dwc->completed, &list);
+	spin_unlock_irqrestore(&dwc->lock, flags);
+
+	list_for_each_entry_safe(desc, _desc, &list, desc_node) {
+		list_del_init(&desc->desc_node);
+		dwc_descriptor_free(dwc, desc, true);
+	}
+}
+
 /* Returns how many bytes were already received from source */
 static inline u32 dwc_get_sent(struct dw_dma_chan *dwc)
 {
@@ -274,19 +290,17 @@ static inline u32 dwc_get_sent(struct dw_dma_chan *dwc)
 
 static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 {
-	struct dw_desc *desc, *_desc;
+	struct dw_desc *desc;
 	struct dw_desc *child;
-	unsigned long flags;
-	LIST_HEAD(list);
 
-	spin_lock_irqsave(&dwc->lock, flags);
+	lockdep_assert_held(&dwc->lock);
 
 	if (!(dma_readl(dw, RAW.XFER) & dwc->mask))
-		goto unlock;
+		return;
 
 	dma_writel(dw, CLEAR.XFER, dwc->mask);
 	if (list_empty(&dwc->active_list))
-		goto unlock;
+		return;
 
 	desc = dwc_first_active(dwc);
 	if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags)) {
@@ -304,7 +318,7 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 
 			/* Submit next block, including for polled transfers. */
 			dwc_do_single_block(dwc, child);
-			goto unlock;
+			return;
 		}
 
 		clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
@@ -319,15 +333,8 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
 	}
 
 	dma_cookie_complete(&desc->txd);
-	list_move_tail(&desc->desc_node, &list);
+	list_move_tail(&desc->desc_node, &dwc->completed);
 	dwc_dostart_first_queued(dwc);
-unlock:
-	spin_unlock_irqrestore(&dwc->lock, flags);
-
-	list_for_each_entry_safe(desc, _desc, &list, desc_node) {
-		list_del_init(&desc->desc_node);
-		dwc_descriptor_free(dwc, desc, true);
-	}
 }
 
 static inline void dwc_dump_lli(struct dw_dma_chan *dwc, struct dw_desc *desc)
@@ -344,18 +351,17 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
 {
 	struct dw_desc *bad_desc;
 	struct dw_desc *child;
-	unsigned long flags;
 
-	spin_lock_irqsave(&dwc->lock, flags);
+	lockdep_assert_held(&dwc->lock);
 
 	/* Termination may have cleared the error since the tasklet sampled it. */
 	if (!(dma_readl(dw, RAW.ERROR) & dwc->mask))
-		goto unlock;
+		return;
 
 	dwc_chan_disable(dw, dwc);
 	clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
 	if (list_empty(&dwc->active_list))
-		goto unlock;
+		return;
 
 	/*
 	 * The descriptor currently at the head of the active list is
@@ -381,40 +387,41 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
 
 	/* Pretend the descriptor completed successfully */
 	dma_cookie_complete(&bad_desc->txd);
-	spin_unlock_irqrestore(&dwc->lock, flags);
+	list_add_tail(&bad_desc->desc_node, &dwc->completed);
+}
 
-	dwc_descriptor_free(dwc, bad_desc, true);
-	return;
+static void dwc_handle_channel(struct dw_dma *dw, struct dw_dma_chan *dwc)
+{
+	lockdep_assert_held(&dwc->lock);
 
-unlock:
-	spin_unlock_irqrestore(&dwc->lock, flags);
+	if (dma_readl(dw, RAW.ERROR) & dwc->mask)
+		dwc_handle_error(dw, dwc);
+	else
+		dwc_scan_descriptors(dw, dwc);
 }
 
 static void dw_dma_tasklet(struct tasklet_struct *t)
 {
 	struct dw_dma *dw = from_tasklet(dw, t, tasklet);
 	struct dw_dma_chan *dwc;
-	u32 status_xfer;
-	u32 status_err;
+	unsigned long flags;
 	unsigned int i;
+	u32 status;
 
 	/* Channel release may have drained a polled completion before this run. */
 	if (!dw->in_use)
 		return;
 
-	status_xfer = dma_readl(dw, RAW.XFER);
-	status_err = dma_readl(dw, RAW.ERROR);
-
-	dev_vdbg(dw->dma.dev, "%s: status_err=%x\n", __func__, status_err);
+	status = dma_readl(dw, RAW.XFER) | dma_readl(dw, RAW.ERROR);
 
 	for (i = 0; i < dw->dma.chancnt; i++) {
 		dwc = &dw->chan[i];
-		if (test_bit(DW_DMA_IS_CYCLIC, &dwc->flags))
-			dev_vdbg(dw->dma.dev, "Cyclic xfer is not implemented\n");
-		else if (status_err & (1 << i))
-			dwc_handle_error(dw, dwc);
-		else if (status_xfer & (1 << i))
-			dwc_scan_descriptors(dw, dwc);
+		spin_lock_irqsave(&dwc->lock, flags);
+		/* The snapshot selects work; recheck events under the channel lock. */
+		if (status & dwc->mask)
+			dwc_handle_channel(dw, dwc);
+		spin_unlock_irqrestore(&dwc->lock, flags);
+		dwc_complete(dwc);
 	}
 
 	/* Re-enable interrupts */
@@ -890,6 +897,7 @@ static int dwc_terminate_all(struct dma_chan *chan)
 	list_splice_init(&dwc->active_list, &list);
 	list_for_each_entry(desc, &list, desc_node)
 		dma_cookie_complete(&desc->txd);
+	list_splice_init(&dwc->completed, &list);
 
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
@@ -960,10 +968,9 @@ static u32 dwc_get_residue_and_status(struct dw_dma_chan *dwc, dma_cookie_t cook
 				      enum dma_status *status)
 {
 	struct dw_desc *desc;
-	unsigned long flags;
 	u32 residue;
 
-	spin_lock_irqsave(&dwc->lock, flags);
+	lockdep_assert_held(&dwc->lock);
 
 	desc = dwc_find_desc(dwc, cookie);
 	if (desc) {
@@ -983,7 +990,6 @@ static u32 dwc_get_residue_and_status(struct dw_dma_chan *dwc, dma_cookie_t cook
 		residue = 0;
 	}
 
-	spin_unlock_irqrestore(&dwc->lock, flags);
 	return residue;
 }
 
@@ -993,19 +999,26 @@ dwc_tx_status(struct dma_chan *chan,
 	      struct dma_tx_state *txstate)
 {
 	struct dw_dma_chan	*dwc = to_dw_dma_chan(chan);
+	struct dw_dma		*dw = to_dw_dma(chan->device);
 	enum dma_status		ret;
+	unsigned long		flags;
 
+	spin_lock_irqsave(&dwc->lock, flags);
 	ret = dma_cookie_status(chan, cookie, txstate);
 	if (ret == DMA_COMPLETE)
-		return ret;
+		goto unlock;
 
-	dwc_scan_descriptors(to_dw_dma(chan->device), dwc);
+	/* Poll completion even when the client has masked the controller IRQ. */
+	dwc_handle_channel(dw, dwc);
+	if (!list_empty(&dwc->completed))
+		tasklet_schedule(&dw->tasklet);
 
 	ret = dma_cookie_status(chan, cookie, txstate);
-	if (ret == DMA_COMPLETE)
-		return ret;
+	if (ret != DMA_COMPLETE)
+		dma_set_residue(txstate, dwc_get_residue_and_status(dwc, cookie, &ret));
 
-	dma_set_residue(txstate, dwc_get_residue_and_status(dwc, cookie, &ret));
+unlock:
+	spin_unlock_irqrestore(&dwc->lock, flags);
 	return ret;
 }
 
@@ -1082,8 +1095,11 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
 {
 	struct dw_dma_chan	*dwc = to_dw_dma_chan(chan);
 	struct dw_dma		*dw = to_dw_dma(chan->device);
+	struct dw_desc		*desc, *_desc;
 	unsigned long		flags;
+	LIST_HEAD(list);
 
+	/* Polled completion can leave descriptors waiting for the tasklet. */
 	tasklet_disable(&dw->tasklet);
 
 	dev_dbg(chan2dev(chan), "%s: descs allocated=%u\n", __func__,
@@ -1095,6 +1111,7 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
 	BUG_ON(dma_readl(to_dw_dma(chan->device), CH_EN) & dwc->mask);
 
 	spin_lock_irqsave(&dwc->lock, flags);
+	list_splice_init(&dwc->completed, &list);
 
 	/* Clear custom channel configuration */
 	memset(&dwc->dws, 0, sizeof(struct dw_dma_slave));
@@ -1106,6 +1123,11 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
 
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
+	list_for_each_entry_safe(desc, _desc, &list, desc_node) {
+		list_del_init(&desc->desc_node);
+		dwc_descriptor_free(dwc, desc, false);
+	}
+
 	/* Disable controller in case it was a last user */
 	dw->in_use &= ~dwc->mask;
 	if (!dw->in_use)
@@ -1243,6 +1265,7 @@ int do_dma_probe(struct dw_dma_chip *chip)
 
 		INIT_LIST_HEAD(&dwc->active_list);
 		INIT_LIST_HEAD(&dwc->queue);
+		INIT_LIST_HEAD(&dwc->completed);
 
 		channel_clear_bit(dw, CH_EN, dwc->mask);
 
diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
index 5969d9cc8d7a..c41f18281efe 100644
--- a/drivers/dma/dw/regs.h
+++ b/drivers/dma/dw/regs.h
@@ -279,6 +279,7 @@ struct dw_dma_chan {
 	unsigned long		flags;
 	struct list_head	active_list;
 	struct list_head	queue;
+	struct list_head	completed;
 
 	unsigned int		descs_allocated;
 

-- 
2.53.0

Re: [PATCH v4 2/7] dmaengine: dw: serialize error recovery with channel termination

[sashiko-bot]

> The tasklet samples RAW.ERROR before taking the channel lock. A
> concurrent termination can empty the active list before dwc_handle_error()
> uses that cached error to select a descriptor. The list head is then
> interpreted as a descriptor. Even with an active descriptor, moving
> queue.next unconditionally can put an empty queue's head on the active
> list and pass it to dwc_dostart().
> 
> Recheck the error status under the channel lock and stop the channel
> before selecting its failed descriptor. Only one transaction is issued
> at a time, so take that descriptor directly instead of scanning for
> completions first. The scan can otherwise complete it and start queued
> work before the error handler has identified the failed transaction.
> 
> Detach the failed descriptor under the same lock and use the queued-start
> helper, which leaves an empty queue alone. Clear the software-LLP flag
> before restarting so a failed multiblock transfer does not prevent the
> next transaction from starting. Keep the completion callback outside the
> lock, with the detached descriptor owned by the tasklet.
> 
> Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
> Fixes: f336e42f73d9 ("dw_dmac: Move single descriptor from dwc->queue to dwc->active_list in dwc_complete_all")
> Cc: stable@vger.kernel.org
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-submit-dw-dma-synchronize-v1-v4-0-fea447d9f285@gmail.com?part=2

Re: [PATCH v4 3/7] dmaengine: dw: complete cookies before releasing the channel lock

[sashiko-bot]

> Completion detaches a descriptor from the active list under the channel
> lock, but updates its cookie later when freeing it. Termination can take
> the lock in between and complete newer descriptors. Completing the older
> cookie afterwards moves completed_cookie backwards and makes an already
> terminated transfer appear outstanding again.
> 
> Complete cookies while detaching descriptors in the completion, error
> recovery and termination paths. Keep the active-before-queued ordering
> when terminating so completed_cookie advances in submission order.
> Descriptor freeing and client notification no longer update cookies.
> 
> Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-submit-dw-dma-synchronize-v1-v4-0-fea447d9f285@gmail.com?part=3

Re: [PATCH v4 4/7] dmaengine: dw: calculate residue separately from completion

[sashiko-bot]

> From: Serge Semin <fancer.lancer@gmail.com>
> 
> The descriptor scanner mixes transfer-complete handling with hardware-LLP
> residue calculation. A status query needs the latter for the requested
> cookie even when no transfer-complete event is pending.
> 
> Move hardware-LLP residue calculation into its own helper and call it from
> the status getter under the channel lock. Keep software-LLP progress in
> the scanner and subtract the current block's progress when reporting its
> residue. The scanner now handles transfer-complete events only, retaining
> the distinction between zero source residue and destination completion.
> 
> Also search the queued descriptors so a transfer waiting to start reports
> its full length instead of zero residue.
> 
> Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> [james: retain polled completion and report queued residue]
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-submit-dw-dma-synchronize-v1-v4-0-fea447d9f285@gmail.com?part=4

Re: [PATCH v4 7/7] dmaengine: dw: Prevent tx-status calling DMA-desc callback

[sashiko-bot]

> From: Serge Semin <fancer.lancer@gmail.com>
> 
> The DMA transfer status getter can invoke a completion callback from
> dwc_scan_descriptors(). An 8250 RX timeout queries that status while
> holding the UART port lock, and its DMA callback takes the same lock:
> 
>   serial8250_handle_irq()
>     uart_port_lock_irqsave(port)
>     serial8250_rx_dma_flush()
>       dmaengine_tx_status()
>         dwc_scan_descriptors()
>           dwc_descriptor_free()
>             dma_rx_complete()
>               uart_port_lock_irqsave(port)
> 
> [ ... ]
> Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> [james: defer notifications while preserving polled completion and cleanup]
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-submit-dw-dma-synchronize-v1-v4-0-fea447d9f285@gmail.com?part=7

Re: [PATCH v4 1/7] dmaengine: dw: Fix XFER bit set, but channel not idle error

[sashiko-bot]

> From: Serge Semin <fancer.lancer@gmail.com>
> 
> If a client driver gets to use the DW DMAC engine device tougher
> than usual, with occasional DMA-transfers termination and restart, then
> the next error can be randomly spotted in the system log:
> 
> > dma dma0chan0: BUG: XFER bit set, but channel not idle!
> 
> For instance that happens in case of the 8250 UART port driver handling
> the looped back high-speed traffic (in my case > 1.5Mbaud) by means of the
> DMA-engine interface.
> 
> The error happens due to the two-staged nature of the DW DMAC IRQs
> handling procedure and due to the critical section break in the meantime.
> In particular in case if the DMA-transfer is terminated and restarted:
> [ ... ]
> Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> [james: require a locked XFER event and handle an empty active list]
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-submit-dw-dma-synchronize-v1-v4-0-fea447d9f285@gmail.com?part=1

Re: [PATCH v4 5/7] dmaengine: dw: drain the tasklet before releasing channel resources

[sashiko-bot]

> The controller tasklet invokes callbacks after dropping the channel lock.
> Releasing a channel can therefore clear its configuration and disable the
> controller while that tasklet run is still using them.
> 
> Disable the tasklet before clearing channel resources and re-enable it
> after updating the controller's in-use mask. This waits for an executing
> run while preserving pending schedules for the other channels. Have a
> pending run return before accessing registers when the last channel has
> already been released.
> 
> Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-submit-dw-dma-synchronize-v1-v4-0-fea447d9f285@gmail.com?part=5

Re: [PATCH v4 6/7] dmaengine: dw: synchronize completion callbacks

[sashiko-bot]

> The controller tasklet can detach completed descriptors and invoke their
> callbacks outside the channel lock. Terminating the channel does not wait
> for those callbacks, and there is no device_synchronize hook. A client can
> therefore reuse callback state or release its buffers after
> dmaengine_synchronize() while an old callback still uses them.
> 
> Disable and re-enable the controller tasklet from device_synchronize,
> outside the channel lock. Disabling waits for an executing tasklet,
> including descriptors it already removed from the channel lists. After
> termination, a later tasklet run cannot acquire another descriptor from
> the terminated channel. Clients must continue to exclude new submissions
> on that channel until synchronization finishes.
> 
> The tasklet is shared, so other channels may schedule it while this wait
> is in progress. Preserve those pending runs: tasklet_kill() temporarily
> owns the scheduling bit and can consume a concurrent scheduling request,
> leaving controller interrupts masked. The disable/enable pair leaves the
> scheduling bit and pending tasklet queue intact. Other channels' hardware
> transfers continue, though their callbacks may be briefly deferred.
> 
> Fixes: b36f09c3c441 ("dmaengine: Add transfer termination synchronization support")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/r/sashiko-outbox-180707@kernel.org
> Cc: stable@vger.kernel.org
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-submit-dw-dma-synchronize-v1-v4-0-fea447d9f285@gmail.com?part=6

Re: [PATCH v4 1/7] dmaengine: dw: Fix XFER bit set, but channel not idle error

[Frank Li]

On Thu, Oct 29, 2026 at 04:29:35PM -0600, James Hilliard wrote:
> From: Serge Semin <fancer.lancer@gmail.com>
>
> If a client driver gets to use the DW DMAC engine device tougher
> than usual, with occasional DMA-transfers termination and restart, then
> the next error can be randomly spotted in the system log:
>
> > dma dma0chan0: BUG: XFER bit set, but channel not idle!
>
> For instance that happens in case of the 8250 UART port driver handling
> the looped back high-speed traffic (in my case > 1.5Mbaud) by means of the
> DMA-engine interface.
>
> The error happens due to the two-staged nature of the DW DMAC IRQs
> handling procedure and due to the critical section break in the meantime.
> In particular in case if the DMA-transfer is terminated and restarted:
> 1. after the IRQ-handler submitted the tasklet but before the tasklet
>    started handling the DMA-descriptors in dwc_scan_descriptors();
> 2. after the XFER completion flag was detected in the
>    dwc_scan_descriptors() method, but before the dwc_complete_all() method
>    is called
> the error denoted above is printed due to the overlap of the last transfer
> completion and the new transfer execution stages.
>
> Clear the channel interrupt status when stopping it, and keep the XFER
> check, descriptor detachment and queued-transfer restart in one locked
> section. An empty active list needs no completion processing.
>
> When the locked XFER recheck finds no event, only update residue. A stale
> tasklet snapshot may belong to a terminated transfer, and a zero residue
> only means the source has been read, not that destination writes have
> finished. Requiring RAW.XFER also prevents the fallback scan from freeing
> a descriptor still linked on active_list.
>
> Fixes: 69cea5a00d31 ("dmaengine/dw_dmac: Replace spin_lock* with irqsave variants and enable submission from callback")
> Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> [james: require a locked XFER event and handle an empty active list]
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>

Please follow b4 order. your signed-off should be last one.

Frank

Re: [PATCH v4 3/7] dmaengine: dw: complete cookies before releasing the channel lock

[Frank Li]

On Thu, Oct 29, 2026 at 04:29:37PM -0600, James Hilliard wrote:
> Completion detaches a descriptor from the active list under the channel
> lock, but updates its cookie later when freeing it. Termination can take
> the lock in between and complete newer descriptors. Completing the older
> cookie afterwards moves completed_cookie backwards and makes an already
> terminated transfer appear outstanding again.
>
> Complete cookies while detaching descriptors in the completion, error
> recovery and termination paths. Keep the active-before-queued ordering
> when terminating so completed_cookie advances in submission order.
> Descriptor freeing and client notification no longer update cookies.
>
> Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---

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

>  drivers/dma/dw/core.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
> index 51c5460b4670..1f7afcd01565 100644
> --- a/drivers/dma/dw/core.c
> +++ b/drivers/dma/dw/core.c
> @@ -236,18 +236,17 @@ static void dwc_dostart_first_queued(struct dw_dma_chan *dwc)
>  /*----------------------------------------------------------------------*/
>
>  static void
> -dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc,
> -		bool callback_required)
> +dwc_descriptor_free(struct dw_dma_chan *dwc, struct dw_desc *desc,
> +		    bool callback_required)
>  {
>  	struct dma_async_tx_descriptor	*txd = &desc->txd;
>  	struct dw_desc			*child;
>  	unsigned long			flags;
>  	struct dmaengine_desc_callback	cb;
>
> -	dev_vdbg(chan2dev(&dwc->chan), "descriptor %u complete\n", txd->cookie);
> +	dev_vdbg(chan2dev(&dwc->chan), "freeing descriptor %p\n", desc);
>
>  	spin_lock_irqsave(&dwc->lock, flags);
> -	dma_cookie_complete(txd);
>  	if (callback_required)
>  		dmaengine_desc_get_callback(txd, &cb);
>  	else
> @@ -334,12 +333,14 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
>
>  		/* Detach the completed transfer before starting queued work. */
>  		list_splice_init(&dwc->active_list, &list);
> +		list_for_each_entry(desc, &list, desc_node)
> +			dma_cookie_complete(&desc->txd);
>  		dwc_dostart_first_queued(dwc);
>  		spin_unlock_irqrestore(&dwc->lock, flags);
>
>  		list_for_each_entry_safe(desc, _desc, &list, desc_node) {
>  			list_del_init(&desc->desc_node);
> -			dwc_descriptor_complete(dwc, desc, true);
> +			dwc_descriptor_free(dwc, desc, true);
>  		}
>  		return;
>  	}
> @@ -441,10 +442,11 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
>  	list_for_each_entry(child, &bad_desc->tx_list, desc_node)
>  		dwc_dump_lli(dwc, child);
>
> +	/* Pretend the descriptor completed successfully */
> +	dma_cookie_complete(&bad_desc->txd);
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>
> -	/* Pretend the descriptor completed successfully */
> -	dwc_descriptor_complete(dwc, bad_desc, true);
> +	dwc_descriptor_free(dwc, bad_desc, true);
>  	return;
>
>  unlock:
> @@ -945,12 +947,16 @@ static int dwc_terminate_all(struct dma_chan *chan)
>  	/* active_list entries will end up before queued entries */
>  	list_splice_init(&dwc->queue, &list);
>  	list_splice_init(&dwc->active_list, &list);
> +	list_for_each_entry(desc, &list, desc_node)
> +		dma_cookie_complete(&desc->txd);
>
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>
>  	/* Flush all pending and queued descriptors */
> -	list_for_each_entry_safe(desc, _desc, &list, desc_node)
> -		dwc_descriptor_complete(dwc, desc, false);
> +	list_for_each_entry_safe(desc, _desc, &list, desc_node) {
> +		list_del_init(&desc->desc_node);
> +		dwc_descriptor_free(dwc, desc, false);
> +	}
>
>  	return 0;
>  }
>
> --
> 2.53.0
>

Re: [PATCH v4 4/7] dmaengine: dw: calculate residue separately from completion

[Frank Li]

On Thu, Oct 29, 2026 at 04:29:38PM -0600, James Hilliard wrote:

dmaengine: dw: move residue calculation into helper dwc_get_hard_llp_desc_residue()

> From: Serge Semin <fancer.lancer@gmail.com>
>
> The descriptor scanner mixes transfer-complete handling with hardware-LLP
> residue calculation. A status query needs the latter for the requested
> cookie even when no transfer-complete event is pending.
>
> Move hardware-LLP residue calculation into its own helper and call it from
> the status getter under the channel lock. Keep software-LLP progress in
> the scanner and subtract the current block's progress when reporting its
> residue. The scanner now handles transfer-complete events only, retaining
> the distinction between zero source residue and destination completion.
>
> Also search the queued descriptors so a transfer waiting to start reports
> its full length instead of zero residue.
>
> Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> [james: retain polled completion and report queued residue]
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
>
> ---
>
> Changelog RFC:
> - Instead of just dropping the dwc_scan_descriptors() method invocation
>   calculate the residue in the Tx-status getter.
> ---
>  drivers/dma/dw/core.c | 166 +++++++++++++++++++++-----------------------------
>  1 file changed, 68 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
> index 1f7afcd01565..1d6419e1cd3c 100644
> --- a/drivers/dma/dw/core.c
> +++ b/drivers/dma/dw/core.c
> @@ -274,123 +274,60 @@ static inline u32 dwc_get_sent(struct dw_dma_chan *dwc)
>
>  static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
>  {
> -	dma_addr_t llp;
>  	struct dw_desc *desc, *_desc;
>  	struct dw_desc *child;
> -	u32 status_xfer;
>  	unsigned long flags;
>  	LIST_HEAD(list);
>
>  	spin_lock_irqsave(&dwc->lock, flags);
> -	llp = channel_readl(dwc, LLP);
> -	status_xfer = dma_readl(dw, RAW.XFER);
>
> -	if (status_xfer & dwc->mask) {
> -		/* Everything we've submitted is done */
> -		dma_writel(dw, CLEAR.XFER, dwc->mask);
> -		if (list_empty(&dwc->active_list)) {
> -			spin_unlock_irqrestore(&dwc->lock, flags);
> -			return;
> -		}
> -
> -		if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags)) {
> -			struct list_head *head, *active = dwc->tx_node_active;
> -
> -			/*
> -			 * We are inside first active descriptor.
> -			 * Otherwise something is really wrong.
> -			 */
> -			desc = dwc_first_active(dwc);
> -
> -			head = &desc->tx_list;
> -			if (active != head) {
> -				/* Update residue to reflect last sent descriptor */
> -				if (active == head->next)
> -					desc->residue -= desc->len;
> -				else
> -					desc->residue -= to_dw_desc(active->prev)->len;
> -
> -				child = to_dw_desc(active);
> +	if (!(dma_readl(dw, RAW.XFER) & dwc->mask))
> +		goto unlock;
>
> -				/* Submit next block */
> -				dwc_do_single_block(dwc, child);
> +	dma_writel(dw, CLEAR.XFER, dwc->mask);
> +	if (list_empty(&dwc->active_list))
> +		goto unlock;
>
> -				spin_unlock_irqrestore(&dwc->lock, flags);
> -				return;
> -			}
> +	desc = dwc_first_active(dwc);
> +	if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags)) {
> +		struct list_head *head = &desc->tx_list;
> +		struct list_head *active = dwc->tx_node_active;
>
> -			/* We are done here */
> -			clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
> -		}
> +		if (active != head) {
> +			/* Update residue to reflect last sent descriptor. */
> +			if (active == head->next)
> +				desc->residue -= desc->len;
> +			else
> +				desc->residue -= to_dw_desc(active->prev)->len;
>
> -		if (dma_readl(dw, CH_EN) & dwc->mask) {
> -			dev_err(chan2dev(&dwc->chan),
> -				"BUG: XFER bit set, but channel not idle!\n");
> +			child = to_dw_desc(active);
>
> -			/* Try to continue after resetting the channel... */
> -			dwc_chan_disable(dw, dwc);
> +			/* Submit next block, including for polled transfers. */
> +			dwc_do_single_block(dwc, child);
> +			goto unlock;
>  		}
>
> -		/* Detach the completed transfer before starting queued work. */
> -		list_splice_init(&dwc->active_list, &list);
> -		list_for_each_entry(desc, &list, desc_node)
> -			dma_cookie_complete(&desc->txd);
> -		dwc_dostart_first_queued(dwc);
> -		spin_unlock_irqrestore(&dwc->lock, flags);
> -
> -		list_for_each_entry_safe(desc, _desc, &list, desc_node) {
> -			list_del_init(&desc->desc_node);
> -			dwc_descriptor_free(dwc, desc, true);
> -		}
> -		return;
> +		clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
>  	}
>
> -	if (list_empty(&dwc->active_list)) {
> -		spin_unlock_irqrestore(&dwc->lock, flags);
> -		return;
> -	}
> +	if (dma_readl(dw, CH_EN) & dwc->mask) {
> +		dev_err(chan2dev(&dwc->chan),
> +			"BUG: XFER bit set, but channel not idle!\n");
>
> -	if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags)) {
> -		dev_vdbg(chan2dev(&dwc->chan), "%s: soft LLP mode\n", __func__);
> -		spin_unlock_irqrestore(&dwc->lock, flags);
> -		return;
> +		/* Try to continue after resetting the channel... */
> +		dwc_chan_disable(dw, dwc);
>  	}
>
> -	dev_vdbg(chan2dev(&dwc->chan), "%s: llp=%pad\n", __func__, &llp);
> -
> -	list_for_each_entry_safe(desc, _desc, &dwc->active_list, desc_node) {
> -		/* Initial residue value */
> -		desc->residue = desc->total_len;
> -
> -		/* Check first descriptors addr */
> -		if (desc->txd.phys == DWC_LLP_LOC(llp)) {
> -			spin_unlock_irqrestore(&dwc->lock, flags);
> -			return;
> -		}
> -
> -		/* Check first descriptors llp */
> -		if (lli_read(desc, llp) == llp) {
> -			/* This one is currently in progress */
> -			desc->residue -= dwc_get_sent(dwc);
> -			spin_unlock_irqrestore(&dwc->lock, flags);
> -			return;
> -		}
> -
> -		desc->residue -= desc->len;
> -		list_for_each_entry(child, &desc->tx_list, desc_node) {
> -			if (lli_read(child, llp) == llp) {
> -				/* Currently in progress */
> -				desc->residue -= dwc_get_sent(dwc);
> -				spin_unlock_irqrestore(&dwc->lock, flags);
> -				return;
> -			}
> -			desc->residue -= child->len;
> -		}
> +	dma_cookie_complete(&desc->txd);
> +	list_move_tail(&desc->desc_node, &list);
> +	dwc_dostart_first_queued(dwc);
> +unlock:
> +	spin_unlock_irqrestore(&dwc->lock, flags);
>
> +	list_for_each_entry_safe(desc, _desc, &list, desc_node) {
> +		list_del_init(&desc->desc_node);
> +		dwc_descriptor_free(dwc, desc, true);
>  	}
> -
> -	/* Residue describes source reads; only RAW.XFER completes a transfer. */
> -	spin_unlock_irqrestore(&dwc->lock, flags);
>  }
>
>  static inline void dwc_dump_lli(struct dw_dma_chan *dwc, struct dw_desc *desc)
> @@ -969,9 +906,39 @@ static struct dw_desc *dwc_find_desc(struct dw_dma_chan *dwc, dma_cookie_t c)
>  		if (desc->txd.cookie == c)
>  			return desc;
>
> +	list_for_each_entry(desc, &dwc->queue, desc_node)
> +		if (desc->txd.cookie == c)
> +			return desc;
> +
>  	return NULL;
>  }
>
> +static u32 dwc_get_hard_llp_desc_residue(struct dw_dma_chan *dwc, struct dw_desc *desc)
> +{
> +	u32 residue = desc->total_len;
> +	struct dw_desc *child;
> +	dma_addr_t llp;
> +
> +	llp = channel_readl(dwc, LLP);
> +
> +	/* The first descriptor may still be waiting to be fetched. */
> +	if (desc->txd.phys == DWC_LLP_LOC(llp))
> +		return residue;
> +
> +	if (lli_read(desc, llp) == llp)
> +		return residue - dwc_get_sent(dwc);
> +
> +	residue -= desc->len;
> +	list_for_each_entry(child, &desc->tx_list, desc_node) {
> +		if (lli_read(child, llp) == llp)
> +			return residue - dwc_get_sent(dwc);
> +
> +		residue -= child->len;
> +	}
> +
> +	return residue;
> +}

This patch still too big. You can create new helper()
dwc_get_hard_llp_desc_residue() and replace orginal code exactly, then
change gradulatly.

Frank

Re: [PATCH v4 5/7] dmaengine: dw: drain the tasklet before releasing channel resources

[Frank Li]

On Thu, Oct 29, 2026 at 04:29:39PM -0600, James Hilliard wrote:
> The controller tasklet invokes callbacks after dropping the channel lock.
> Releasing a channel can therefore clear its configuration and disable the
> controller while that tasklet run is still using them.
>
> Disable the tasklet before clearing channel resources and re-enable it
> after updating the controller's in-use mask. This waits for an executing
> run while preserving pending schedules for the other channels. Have a
> pending run return before accessing registers when the last channel has
> already been released.
>
> Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---

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

>  drivers/dma/dw/core.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
> index 1d6419e1cd3c..3bda85adbeee 100644
> --- a/drivers/dma/dw/core.c
> +++ b/drivers/dma/dw/core.c
> @@ -398,6 +398,10 @@ static void dw_dma_tasklet(struct tasklet_struct *t)
>  	u32 status_err;
>  	unsigned int i;
>
> +	/* Channel release may have drained a polled completion before this run. */
> +	if (!dw->in_use)
> +		return;
> +
>  	status_xfer = dma_readl(dw, RAW.XFER);
>  	status_err = dma_readl(dw, RAW.ERROR);
>
> @@ -1067,6 +1071,8 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
>  	struct dw_dma		*dw = to_dw_dma(chan->device);
>  	unsigned long		flags;
>
> +	tasklet_disable(&dw->tasklet);
> +
>  	dev_dbg(chan2dev(chan), "%s: descs allocated=%u\n", __func__,
>  			dwc->descs_allocated);
>
> @@ -1092,6 +1098,8 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
>  	if (!dw->in_use)
>  		do_dw_dma_off(dw);
>
> +	tasklet_enable(&dw->tasklet);
> +
>  	dev_vdbg(chan2dev(chan), "%s: done\n", __func__);
>  }
>
>
> --
> 2.53.0
>

Re: [PATCH v4 7/7] dmaengine: dw: Prevent tx-status calling DMA-desc callback

[Frank Li]

On Thu, Oct 29, 2026 at 04:29:41PM -0600, James Hilliard wrote:
> From: Serge Semin <fancer.lancer@gmail.com>
>
> The DMA transfer status getter can invoke a completion callback from
> dwc_scan_descriptors(). An 8250 RX timeout queries that status while
> holding the UART port lock, and its DMA callback takes the same lock:
>
>   serial8250_handle_irq()
>     uart_port_lock_irqsave(port)
>     serial8250_rx_dma_flush()
>       dmaengine_tx_status()
>         dwc_scan_descriptors()
>           dwc_descriptor_free()
>             dma_rx_complete()
>               uart_port_lock_irqsave(port)
>
> Queue completed descriptors on a channel completion list under the channel
> lock. Let the controller tasklet detach that list and invoke callbacks
> outside the lock. Termination discards notifications still on the shared
> list; the tasklet owns descriptors it has already detached, and the
> synchronization hook waits for that run.
>
> Keep handling raw completion events from tx_status(). Clients such as
> catpt poll transfers without DMA_PREP_INTERRUPT and can mask the controller
> interrupt externally. Polling must still advance software-emulated LLP
> blocks and complete cookies without waiting for tasklet execution. Hold
> the channel lock across event handling and status calculation, then
> schedule the tasklet for deferred notifications.
>
> On channel release, discard queued notifications after draining the
> tasklet, before clearing the controller's in-use state.
>
> Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> [james: defer notifications while preserving polled completion and cleanup]
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
>
> ---
>
> Changelog RFC:
> - Instead of just dropping the dwc_scan_descriptors() method invocation
>   calculate the residue in the Tx-status getter.
> ---
>  drivers/dma/dw/core.c | 111 ++++++++++++++++++++++++++++++--------------------
>  drivers/dma/dw/regs.h |   1 +
>  2 files changed, 68 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
> index 66065d7b26ae..d80be93f0c49 100644
> --- a/drivers/dma/dw/core.c
> +++ b/drivers/dma/dw/core.c
> @@ -262,6 +262,22 @@ dwc_descriptor_free(struct dw_dma_chan *dwc, struct dw_desc *desc,
>  	dmaengine_desc_callback_invoke(&cb, NULL);
>  }
>
> +static void dwc_complete(struct dw_dma_chan *dwc)
> +{

dwc_free() ?

> +	struct dw_desc *desc, *_desc;
> +	unsigned long flags;
> +	LIST_HEAD(list);
> +
> +	spin_lock_irqsave(&dwc->lock, flags);
> +	list_splice_tail_init(&dwc->completed, &list);
> +	spin_unlock_irqrestore(&dwc->lock, flags);
> +
> +	list_for_each_entry_safe(desc, _desc, &list, desc_node) {
> +		list_del_init(&desc->desc_node);
> +		dwc_descriptor_free(dwc, desc, true);
> +	}
> +}
> +
>  /* Returns how many bytes were already received from source */
>  static inline u32 dwc_get_sent(struct dw_dma_chan *dwc)
>  {
> @@ -274,19 +290,17 @@ static inline u32 dwc_get_sent(struct dw_dma_chan *dwc)
>
>  static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
>  {
> -	struct dw_desc *desc, *_desc;
> +	struct dw_desc *desc;
>  	struct dw_desc *child;
> -	unsigned long flags;
> -	LIST_HEAD(list);
>
> -	spin_lock_irqsave(&dwc->lock, flags);
> +	lockdep_assert_held(&dwc->lock);
>
>  	if (!(dma_readl(dw, RAW.XFER) & dwc->mask))
> -		goto unlock;
> +		return;
>
>  	dma_writel(dw, CLEAR.XFER, dwc->mask);
>  	if (list_empty(&dwc->active_list))
> -		goto unlock;
> +		return;
>
>  	desc = dwc_first_active(dwc);
>  	if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags)) {
> @@ -304,7 +318,7 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
>
>  			/* Submit next block, including for polled transfers. */
>  			dwc_do_single_block(dwc, child);
> -			goto unlock;
> +			return;
>  		}
>
>  		clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
> @@ -319,15 +333,8 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
>  	}
>
>  	dma_cookie_complete(&desc->txd);
> -	list_move_tail(&desc->desc_node, &list);
> +	list_move_tail(&desc->desc_node, &dwc->completed);
>  	dwc_dostart_first_queued(dwc);
> -unlock:
> -	spin_unlock_irqrestore(&dwc->lock, flags);
> -
> -	list_for_each_entry_safe(desc, _desc, &list, desc_node) {
> -		list_del_init(&desc->desc_node);
> -		dwc_descriptor_free(dwc, desc, true);
> -	}
>  }
>
>  static inline void dwc_dump_lli(struct dw_dma_chan *dwc, struct dw_desc *desc)
> @@ -344,18 +351,17 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
>  {
>  	struct dw_desc *bad_desc;
>  	struct dw_desc *child;
> -	unsigned long flags;
>
> -	spin_lock_irqsave(&dwc->lock, flags);
> +	lockdep_assert_held(&dwc->lock);
>
>  	/* Termination may have cleared the error since the tasklet sampled it. */
>  	if (!(dma_readl(dw, RAW.ERROR) & dwc->mask))
> -		goto unlock;
> +		return;
>
>  	dwc_chan_disable(dw, dwc);
>  	clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
>  	if (list_empty(&dwc->active_list))
> -		goto unlock;
> +		return;
>
>  	/*
>  	 * The descriptor currently at the head of the active list is
> @@ -381,40 +387,41 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
>
>  	/* Pretend the descriptor completed successfully */
>  	dma_cookie_complete(&bad_desc->txd);
> -	spin_unlock_irqrestore(&dwc->lock, flags);
> +	list_add_tail(&bad_desc->desc_node, &dwc->completed);
> +}
>
> -	dwc_descriptor_free(dwc, bad_desc, true);
> -	return;
> +static void dwc_handle_channel(struct dw_dma *dw, struct dw_dma_chan *dwc)
> +{
> +	lockdep_assert_held(&dwc->lock);
>
> -unlock:
> -	spin_unlock_irqrestore(&dwc->lock, flags);
> +	if (dma_readl(dw, RAW.ERROR) & dwc->mask)
> +		dwc_handle_error(dw, dwc);
> +	else
> +		dwc_scan_descriptors(dw, dwc);
>  }
>
>  static void dw_dma_tasklet(struct tasklet_struct *t)
>  {
>  	struct dw_dma *dw = from_tasklet(dw, t, tasklet);
>  	struct dw_dma_chan *dwc;
> -	u32 status_xfer;
> -	u32 status_err;
> +	unsigned long flags;
>  	unsigned int i;
> +	u32 status;
>
>  	/* Channel release may have drained a polled completion before this run. */
>  	if (!dw->in_use)
>  		return;
>
> -	status_xfer = dma_readl(dw, RAW.XFER);
> -	status_err = dma_readl(dw, RAW.ERROR);
> -
> -	dev_vdbg(dw->dma.dev, "%s: status_err=%x\n", __func__, status_err);
> +	status = dma_readl(dw, RAW.XFER) | dma_readl(dw, RAW.ERROR);
>
>  	for (i = 0; i < dw->dma.chancnt; i++) {
>  		dwc = &dw->chan[i];
> -		if (test_bit(DW_DMA_IS_CYCLIC, &dwc->flags))
> -			dev_vdbg(dw->dma.dev, "Cyclic xfer is not implemented\n");
> -		else if (status_err & (1 << i))
> -			dwc_handle_error(dw, dwc);
> -		else if (status_xfer & (1 << i))
> -			dwc_scan_descriptors(dw, dwc);
> +		spin_lock_irqsave(&dwc->lock, flags);
> +		/* The snapshot selects work; recheck events under the channel lock. */
> +		if (status & dwc->mask)
> +			dwc_handle_channel(dw, dwc);
> +		spin_unlock_irqrestore(&dwc->lock, flags);
> +		dwc_complete(dwc);
>  	}
>
>  	/* Re-enable interrupts */
> @@ -890,6 +897,7 @@ static int dwc_terminate_all(struct dma_chan *chan)
>  	list_splice_init(&dwc->active_list, &list);
>  	list_for_each_entry(desc, &list, desc_node)
>  		dma_cookie_complete(&desc->txd);
> +	list_splice_init(&dwc->completed, &list);
>
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>
> @@ -960,10 +968,9 @@ static u32 dwc_get_residue_and_status(struct dw_dma_chan *dwc, dma_cookie_t cook
>  				      enum dma_status *status)
>  {
>  	struct dw_desc *desc;
> -	unsigned long flags;
>  	u32 residue;
>
> -	spin_lock_irqsave(&dwc->lock, flags);
> +	lockdep_assert_held(&dwc->lock);
>
>  	desc = dwc_find_desc(dwc, cookie);
>  	if (desc) {
> @@ -983,7 +990,6 @@ static u32 dwc_get_residue_and_status(struct dw_dma_chan *dwc, dma_cookie_t cook
>  		residue = 0;
>  	}
>
> -	spin_unlock_irqrestore(&dwc->lock, flags);
>  	return residue;
>  }
>
> @@ -993,19 +999,26 @@ dwc_tx_status(struct dma_chan *chan,
>  	      struct dma_tx_state *txstate)
>  {
>  	struct dw_dma_chan	*dwc = to_dw_dma_chan(chan);
> +	struct dw_dma		*dw = to_dw_dma(chan->device);
>  	enum dma_status		ret;
> +	unsigned long		flags;
>
> +	spin_lock_irqsave(&dwc->lock, flags);
>  	ret = dma_cookie_status(chan, cookie, txstate);
>  	if (ret == DMA_COMPLETE)
> -		return ret;
> +		goto unlock;
>
> -	dwc_scan_descriptors(to_dw_dma(chan->device), dwc);
> +	/* Poll completion even when the client has masked the controller IRQ. */
> +	dwc_handle_channel(dw, dwc);
> +	if (!list_empty(&dwc->completed))
> +		tasklet_schedule(&dw->tasklet);
>
>  	ret = dma_cookie_status(chan, cookie, txstate);
> -	if (ret == DMA_COMPLETE)
> -		return ret;
> +	if (ret != DMA_COMPLETE)
> +		dma_set_residue(txstate, dwc_get_residue_and_status(dwc, cookie, &ret));
>
> -	dma_set_residue(txstate, dwc_get_residue_and_status(dwc, cookie, &ret));
> +unlock:
> +	spin_unlock_irqrestore(&dwc->lock, flags);
>  	return ret;
>  }
>
> @@ -1082,8 +1095,11 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
>  {
>  	struct dw_dma_chan	*dwc = to_dw_dma_chan(chan);
>  	struct dw_dma		*dw = to_dw_dma(chan->device);
> +	struct dw_desc		*desc, *_desc;
>  	unsigned long		flags;
> +	LIST_HEAD(list);
>
> +	/* Polled completion can leave descriptors waiting for the tasklet. */
>  	tasklet_disable(&dw->tasklet);
>
>  	dev_dbg(chan2dev(chan), "%s: descs allocated=%u\n", __func__,
> @@ -1095,6 +1111,7 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
>  	BUG_ON(dma_readl(to_dw_dma(chan->device), CH_EN) & dwc->mask);
>
>  	spin_lock_irqsave(&dwc->lock, flags);
> +	list_splice_init(&dwc->completed, &list);
>
>  	/* Clear custom channel configuration */
>  	memset(&dwc->dws, 0, sizeof(struct dw_dma_slave));
> @@ -1106,6 +1123,11 @@ static void dwc_free_chan_resources(struct dma_chan *chan)
>
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>
> +	list_for_each_entry_safe(desc, _desc, &list, desc_node) {
> +		list_del_init(&desc->desc_node);
> +		dwc_descriptor_free(dwc, desc, false);
> +	}
> +

modify dwc_complete() and call it from here also ?

Frank

>  	/* Disable controller in case it was a last user */
>  	dw->in_use &= ~dwc->mask;
>  	if (!dw->in_use)
> @@ -1243,6 +1265,7 @@ int do_dma_probe(struct dw_dma_chip *chip)
>
>  		INIT_LIST_HEAD(&dwc->active_list);
>  		INIT_LIST_HEAD(&dwc->queue);
> +		INIT_LIST_HEAD(&dwc->completed);
>
>  		channel_clear_bit(dw, CH_EN, dwc->mask);
>
> diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
> index 5969d9cc8d7a..c41f18281efe 100644
> --- a/drivers/dma/dw/regs.h
> +++ b/drivers/dma/dw/regs.h
> @@ -279,6 +279,7 @@ struct dw_dma_chan {
>  	unsigned long		flags;
>  	struct list_head	active_list;
>  	struct list_head	queue;
> +	struct list_head	completed;
>
>  	unsigned int		descs_allocated;
>
>
> --
> 2.53.0
>