[PATCH 04/17] dmaengine: dw-edma: Clean up vchan descriptors on termination

[Koichiro Den]

dw-edma resets channel state from terminate_all() paths, but pending
virt-dma descriptors can remain on the submitted and issued lists. A
later issue_pending() may then restart work that the client already
terminated, possibly into buffers that were already reused. Descriptors
that are never restarted leak instead.

Move issued and submitted descriptors to the terminated list whenever a
termination request completes. Also release virt-dma resources from
free_chan_resources().

If termination was deferred because the channel was still running, wait
until the STOP path deconfigures the channel before synchronizing or
freeing virt-dma resources. Otherwise dmaengine_terminate_sync() can
return before the deferred STOP cleanup has moved issued descriptors to
the terminated list and before the channel is known to have stopped.

The old free_chan_resources() loop usually broke as soon as
terminate_all() returned zero, so it did not effectively spin until the
timeout. This wait can now last until the existing timeout, so use
cond_resched() instead of busy-polling with cpu_relax(), and warn if the
timeout expires.

Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
---
 drivers/dma/dw-edma/dw-edma-core.c | 78 ++++++++++++++++++++++++------
 1 file changed, 64 insertions(+), 14 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index bedaee6d30ab..2777dc0b2aed 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -15,6 +15,7 @@
 #include <linux/irq.h>
 #include <linux/dma/edma.h>
 #include <linux/dma-mapping.h>
+#include <linux/sched.h>
 #include <linux/string_choices.h>
 
 #include "dw-edma-core.h"
@@ -113,6 +114,28 @@ static void dw_edma_terminate_vdesc(struct virt_dma_desc *vd)
 	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)
 {
@@ -190,20 +213,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) {
@@ -211,6 +239,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;
 }
@@ -544,7 +573,7 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
 			break;
 
 		case EDMA_REQ_STOP:
-			dw_edma_terminate_vdesc(vd);
+			dw_edma_terminate_all_descs(chan);
 			chan->request = EDMA_REQ_NONE;
 			chan->status = EDMA_ST_IDLE;
 			break;
@@ -616,28 +645,49 @@ static int dw_edma_alloc_chan_resources(struct dma_chan *dchan)
 	return 0;
 }
 
+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);
+	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 marks that termination completed.
+	 */
+	while (time_before(jiffies, timeout)) {
+		dw_edma_device_terminate_all(dchan);
+
+		spin_lock_irqsave(&chan->vc.lock, flags);
+		configured = chan->configured;
+		spin_unlock_irqrestore(&chan->vc.lock, flags);
+		if (!configured)
+			return;
+
+		cond_resched();
+	}
+
+	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)
 {
-	unsigned long timeout = jiffies + msecs_to_jiffies(5000);
-	int ret;
-
-	while (time_before(jiffies, timeout)) {
-		ret = dw_edma_device_terminate_all(dchan);
-		if (!ret)
-			break;
-
-		if (time_after_eq(jiffies, timeout))
-			return;
+	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
 
-		cpu_relax();
-	}
+	dw_edma_wait_termination(dchan);
+	vchan_synchronize(&chan->vc);
+	vchan_free_chan_resources(&chan->vc);
 }
 
 static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
-- 
2.51.0