[PATCH 1/2] dma: cppi41: redo descriptor collection in abort case

* [PATCH 1/2] dma: cppi41: redo descriptor collection in abort case
@ 2013-10-17 14:19 Sebastian Andrzej Siewior
  2013-10-17 14:19 ` [PATCH 2/2] dma: cppi41: return code > 0 of pm_runtime_get_sync() is not an error Sebastian Andrzej Siewior
  2013-10-17 14:20 ` [PATCH 1/2] dma: cppi41: redo descriptor collection in abort case Sebastian Andrzej Siewior
  0 siblings, 2 replies; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-10-17 14:19 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Dan Williams, linux-kernel, Felipe Balbi, linux-usb, kishon,
	Sebastian Andrzej Siewior, Daniel Mack

Most of the logic here is try and error since what actually happens does
not match the trm or I miss read it.
My first assumption was that the queue on which the tear-down descriptor
completes (their own complete queue vs "active descriptor" complete
queue) depends on the transfer direction. This seems not to be true
because I manage to trigger
|  WARN_ON(c->desc_phys != desc_phys);
and the other few were fine means the tear-down descriptor was valid but
on different queue.

This patch changes the logic here to look on both queues for the
descriptor.

Cc: Daniel Mack <zonque@gmail.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/dma/cppi41.c | 41 ++++++++++++++++-------------------------
 1 file changed, 16 insertions(+), 25 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 42d1c58..272969c 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -563,36 +563,27 @@ static int cppi41_tear_down_chan(struct cppi41_channel *c)
 		c->td_retry = 100;
 	}
 
-	if (!c->td_seen) {
+	if (!c->td_seen || !c->td_desc_seen) {
 		unsigned td_comp_queue;
 
-		if (c->is_tx)
-			td_comp_queue =  cdd->td_queue.complete;
-		else
-			td_comp_queue =  c->q_comp_num;
+		desc_phys = cppi41_pop_desc(cdd, cdd->td_queue.complete);
+		if (!desc_phys)
+			desc_phys = cppi41_pop_desc(cdd, c->q_comp_num);
 
-		desc_phys = cppi41_pop_desc(cdd, td_comp_queue);
-		if (desc_phys) {
-			__iormb();
+		if (desc_phys == c->desc_phys) {
+			c->td_desc_seen = 1;
+
+		} else if (desc_phys == td_desc_phys) {
+			u32 pd0;
 
-			if (desc_phys == td_desc_phys) {
-				u32 pd0;
-				pd0 = td->pd0;
-				WARN_ON((pd0 >> DESC_TYPE) != DESC_TYPE_TEARD);
-				WARN_ON(!c->is_tx && !(pd0 & TD_DESC_IS_RX));
-				WARN_ON((pd0 & 0x1f) != c->port_num);
-			} else {
-				WARN_ON_ONCE(1);
-			}
-			c->td_seen = 1;
-		}
-	}
-	if (!c->td_desc_seen) {
-		desc_phys = cppi41_pop_desc(cdd, c->q_comp_num);
-		if (desc_phys) {
 			__iormb();
-			WARN_ON(c->desc_phys != desc_phys);
-			c->td_desc_seen = 1;
+			pd0 = td->pd0;
+			WARN_ON((pd0 >> DESC_TYPE) != DESC_TYPE_TEARD);
+			WARN_ON(!c->is_tx && !(pd0 & TD_DESC_IS_RX));
+			WARN_ON((pd0 & 0x1f) != c->port_num);
+			c->td_seen = 1;
+		} else if (desc_phys) {
+			WARN_ON_ONCE(1);
 		}
 	}
 	c->td_retry--;
-- 
1.8.4.rc3


^ permalink raw reply related	[flat|nested] 6+ messages in thread