linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* cppi41: pending patches
@ 2013-10-22 10:14 Sebastian Andrzej Siewior
  2013-10-22 10:14 ` [PATCH 1/4] dma: cppi41: restore more registers Sebastian Andrzej Siewior
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-10-22 10:14 UTC (permalink / raw)
  To: Vinod Koul; +Cc: Dan Williams, linux-kernel, Felipe Balbi, linux-omap

Hi Vinod,

this series contains patches which are floating on the mainling list so
I hope it is easier to collect them. It contains two of Daniel's which
were not yet applied and two of mine.
The patch "redo descriptor collection in abort case" was posted earlier
and tested by Daniel, in this version here I removed an unused variable.

The patches are also available in a git tree based on your next tree prior
the the "dma_complete" merge because DMA_COMPLETE & DMA_SUCCESS contain a
diferent value.

The following changes since commit cc94dac27e15df9211394467e13fdfa2366e3593:

  Merge branch 'for-linus' into next (2013-10-21 12:57:31 +0530)

are available in the git repository at:


  git://breakpoint.cc/bigeasy/linux tags/cppi41-next

for you to fetch changes up to e389973c52dbea967a9254798600c35c8f94b2c3:

  dma: cppi41: return code > 0 of pm_runtime_get_sync() is not an error (2013-10-22 12:00:45 +0200)

----------------------------------------------------------------
A handfull of cppi41 patches including pm support by Daniel Mack.

----------------------------------------------------------------
Daniel Mack (2):
      dma: cppi41: restore more registers
      dma: cppi41: use cppi41_pop_desc() where possible

Sebastian Andrzej Siewior (2):
      dma: cppi41: redo descriptor collection in abort case
      dma: cppi41: return code > 0 of pm_runtime_get_sync() is not an error

 drivers/dma/cppi41.c | 82 +++++++++++++++++++++++++++-------------------------
 1 file changed, 43 insertions(+), 39 deletions(-)

Sebastian

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

* [PATCH 1/4] dma: cppi41: restore more registers
  2013-10-22 10:14 cppi41: pending patches Sebastian Andrzej Siewior
@ 2013-10-22 10:14 ` Sebastian Andrzej Siewior
  2013-10-22 10:14 ` [PATCH 2/4] dma: cppi41: use cppi41_pop_desc() where possible Sebastian Andrzej Siewior
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-10-22 10:14 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Dan Williams, linux-kernel, Felipe Balbi, linux-omap, Daniel Mack,
	Sebastian Andrzej Siewior

From: Daniel Mack <zonque@gmail.com>

With active users over suspend/resume cycles, it turns out that
more registers, in particular DMA_TDFDQ and RXHPCRA0, have to be
restored on resume.

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

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 167c022..e53292d 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -141,6 +141,9 @@ struct cppi41_dd {
 	const struct chan_queues *queues_rx;
 	const struct chan_queues *queues_tx;
 	struct chan_queues td_queue;
+
+	/* context for suspend/resume */
+	unsigned int dma_tdfdq;
 };
 
 #define FIST_COMPLETION_QUEUE	93
@@ -1050,6 +1053,7 @@ static int cppi41_suspend(struct device *dev)
 {
 	struct cppi41_dd *cdd = dev_get_drvdata(dev);
 
+	cdd->dma_tdfdq = cppi_readl(cdd->ctrl_mem + DMA_TDFDQ);
 	cppi_writel(0, cdd->usbss_mem + USBSS_IRQ_CLEARR);
 	disable_sched(cdd);
 
@@ -1059,12 +1063,23 @@ static int cppi41_suspend(struct device *dev)
 static int cppi41_resume(struct device *dev)
 {
 	struct cppi41_dd *cdd = dev_get_drvdata(dev);
+	struct cppi41_channel *c;
 	int i;
 
 	for (i = 0; i < DESCS_AREAS; i++)
 		cppi_writel(cdd->descs_phys, cdd->qmgr_mem + QMGR_MEMBASE(i));
 
+	list_for_each_entry(c, &cdd->ddev.channels, chan.device_node)
+		if (!c->is_tx)
+			cppi_writel(c->q_num, c->gcr_reg + RXHPCRA0);
+
 	init_sched(cdd);
+
+	cppi_writel(cdd->dma_tdfdq, cdd->ctrl_mem + DMA_TDFDQ);
+	cppi_writel(cdd->scratch_phys, cdd->qmgr_mem + QMGR_LRAM0_BASE);
+	cppi_writel(QMGR_SCRATCH_SIZE, cdd->qmgr_mem + QMGR_LRAM_SIZE);
+	cppi_writel(0, cdd->qmgr_mem + QMGR_LRAM1_BASE);
+
 	cppi_writel(USBSS_IRQ_PD_COMP, cdd->usbss_mem + USBSS_IRQ_ENABLER);
 
 	return 0;
-- 
1.8.4.rc3

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

* [PATCH 2/4] dma: cppi41: use cppi41_pop_desc() where possible
  2013-10-22 10:14 cppi41: pending patches Sebastian Andrzej Siewior
  2013-10-22 10:14 ` [PATCH 1/4] dma: cppi41: restore more registers Sebastian Andrzej Siewior
@ 2013-10-22 10:14 ` Sebastian Andrzej Siewior
  2013-10-22 10:14 ` [PATCH 3/4] dma: cppi41: redo descriptor collection in abort case Sebastian Andrzej Siewior
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-10-22 10:14 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Dan Williams, linux-kernel, Felipe Balbi, linux-omap, Daniel Mack,
	Sebastian Andrzej Siewior

From: Daniel Mack <zonque@gmail.com>

Use cppi41_pop_desc() when appropriate instead of open-coding the same
functionality again. That makes the code more readable. The function has
to be moved some lines up for this change.

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

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index e53292d..42d1c58 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -266,6 +266,15 @@ static u32 pd_trans_len(u32 val)
 	return val & ((1 << (DESC_LENGTH_BITS_NUM + 1)) - 1);
 }
 
+static u32 cppi41_pop_desc(struct cppi41_dd *cdd, unsigned queue_num)
+{
+	u32 desc;
+
+	desc = cppi_readl(cdd->qmgr_mem + QMGR_QUEUE_D(queue_num));
+	desc &= ~0x1f;
+	return desc;
+}
+
 static irqreturn_t cppi41_irq(int irq, void *data)
 {
 	struct cppi41_dd *cdd = data;
@@ -303,8 +312,7 @@ static irqreturn_t cppi41_irq(int irq, void *data)
 			q_num = __fls(val);
 			val &= ~(1 << q_num);
 			q_num += 32 * i;
-			desc = cppi_readl(cdd->qmgr_mem + QMGR_QUEUE_D(q_num));
-			desc &= ~0x1f;
+			desc = cppi41_pop_desc(cdd, q_num);
 			c = desc_to_chan(cdd, desc);
 			if (WARN_ON(!c)) {
 				pr_err("%s() q %d desc %08x\n", __func__,
@@ -520,15 +528,6 @@ static void cppi41_compute_td_desc(struct cppi41_desc *d)
 	d->pd0 = DESC_TYPE_TEARD << DESC_TYPE;
 }
 
-static u32 cppi41_pop_desc(struct cppi41_dd *cdd, unsigned queue_num)
-{
-	u32 desc;
-
-	desc = cppi_readl(cdd->qmgr_mem + QMGR_QUEUE_D(queue_num));
-	desc &= ~0x1f;
-	return desc;
-}
-
 static int cppi41_tear_down_chan(struct cppi41_channel *c)
 {
 	struct cppi41_dd *cdd = c->cdd;
@@ -612,7 +611,7 @@ static int cppi41_tear_down_chan(struct cppi41_channel *c)
 
 	WARN_ON(!c->td_retry);
 	if (!c->td_desc_seen) {
-		desc_phys = cppi_readl(cdd->qmgr_mem + QMGR_QUEUE_D(c->q_num));
+		desc_phys = cppi41_pop_desc(cdd, c->q_num);
 		WARN_ON(!desc_phys);
 	}
 
-- 
1.8.4.rc3

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

* [PATCH 3/4] dma: cppi41: redo descriptor collection in abort case
  2013-10-22 10:14 cppi41: pending patches Sebastian Andrzej Siewior
  2013-10-22 10:14 ` [PATCH 1/4] dma: cppi41: restore more registers Sebastian Andrzej Siewior
  2013-10-22 10:14 ` [PATCH 2/4] dma: cppi41: use cppi41_pop_desc() where possible Sebastian Andrzej Siewior
@ 2013-10-22 10:14 ` Sebastian Andrzej Siewior
  2013-10-22 10:14 ` [PATCH 4/4] dma: cppi41: return code > 0 of pm_runtime_get_sync() is not an error Sebastian Andrzej Siewior
  2013-11-12  8:59 ` cppi41: pending patches Vinod Koul
  4 siblings, 0 replies; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-10-22 10:14 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Dan Williams, linux-kernel, Felipe Balbi, linux-omap,
	Sebastian Andrzej Siewior

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.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/dma/cppi41.c | 42 ++++++++++++++++--------------------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 42d1c58..561de4f 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -563,36 +563,26 @@ static int cppi41_tear_down_chan(struct cppi41_channel *c)
 		c->td_retry = 100;
 	}
 
-	if (!c->td_seen) {
-		unsigned td_comp_queue;
+	if (!c->td_seen || !c->td_desc_seen) {
 
-		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

* [PATCH 4/4] dma: cppi41: return code > 0 of pm_runtime_get_sync() is not an error
  2013-10-22 10:14 cppi41: pending patches Sebastian Andrzej Siewior
                   ` (2 preceding siblings ...)
  2013-10-22 10:14 ` [PATCH 3/4] dma: cppi41: redo descriptor collection in abort case Sebastian Andrzej Siewior
@ 2013-10-22 10:14 ` Sebastian Andrzej Siewior
  2013-11-12  8:59 ` cppi41: pending patches Vinod Koul
  4 siblings, 0 replies; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-10-22 10:14 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Dan Williams, linux-kernel, Felipe Balbi, linux-omap,
	Sebastian Andrzej Siewior

Return code of pm_runtime_get_sync() > 0 is not an error and may happen.
Noticed during rmmod & modprobe testing.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/dma/cppi41.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 561de4f..c706d21 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -956,7 +956,7 @@ static int cppi41_dma_probe(struct platform_device *pdev)
 
 	pm_runtime_enable(dev);
 	ret = pm_runtime_get_sync(dev);
-	if (ret)
+	if (ret < 0)
 		goto err_get_sync;
 
 	cdd->queues_rx = glue_info->queues_rx;
-- 
1.8.4.rc3

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

* Re: cppi41: pending patches
  2013-10-22 10:14 cppi41: pending patches Sebastian Andrzej Siewior
                   ` (3 preceding siblings ...)
  2013-10-22 10:14 ` [PATCH 4/4] dma: cppi41: return code > 0 of pm_runtime_get_sync() is not an error Sebastian Andrzej Siewior
@ 2013-11-12  8:59 ` Vinod Koul
  4 siblings, 0 replies; 6+ messages in thread
From: Vinod Koul @ 2013-11-12  8:59 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Dan Williams, linux-kernel, Felipe Balbi, linux-omap

On Tue, Oct 22, 2013 at 12:14:02PM +0200, Sebastian Andrzej Siewior wrote:
> Hi Vinod,
> 
> this series contains patches which are floating on the mainling list so
> I hope it is easier to collect them. It contains two of Daniel's which
> were not yet applied and two of mine.
> The patch "redo descriptor collection in abort case" was posted earlier
> and tested by Daniel, in this version here I removed an unused variable.
> 
> The patches are also available in a git tree based on your next tree prior
> the the "dma_complete" merge because DMA_COMPLETE & DMA_SUCCESS contain a
> diferent value.
Applied, thanks

--
~Vinod
> 
> The following changes since commit cc94dac27e15df9211394467e13fdfa2366e3593:
> 
>   Merge branch 'for-linus' into next (2013-10-21 12:57:31 +0530)
> 
> are available in the git repository at:
> 
> 
>   git://breakpoint.cc/bigeasy/linux tags/cppi41-next
> 
> for you to fetch changes up to e389973c52dbea967a9254798600c35c8f94b2c3:
> 
>   dma: cppi41: return code > 0 of pm_runtime_get_sync() is not an error (2013-10-22 12:00:45 +0200)
> 
> ----------------------------------------------------------------
> A handfull of cppi41 patches including pm support by Daniel Mack.
> 
> ----------------------------------------------------------------
> Daniel Mack (2):
>       dma: cppi41: restore more registers
>       dma: cppi41: use cppi41_pop_desc() where possible
> 
> Sebastian Andrzej Siewior (2):
>       dma: cppi41: redo descriptor collection in abort case
>       dma: cppi41: return code > 0 of pm_runtime_get_sync() is not an error
> 
>  drivers/dma/cppi41.c | 82 +++++++++++++++++++++++++++-------------------------
>  1 file changed, 43 insertions(+), 39 deletions(-)
> 
> Sebastian

-- 

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

end of thread, other threads:[~2013-11-12  8:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-22 10:14 cppi41: pending patches Sebastian Andrzej Siewior
2013-10-22 10:14 ` [PATCH 1/4] dma: cppi41: restore more registers Sebastian Andrzej Siewior
2013-10-22 10:14 ` [PATCH 2/4] dma: cppi41: use cppi41_pop_desc() where possible Sebastian Andrzej Siewior
2013-10-22 10:14 ` [PATCH 3/4] dma: cppi41: redo descriptor collection in abort case Sebastian Andrzej Siewior
2013-10-22 10:14 ` [PATCH 4/4] dma: cppi41: return code > 0 of pm_runtime_get_sync() is not an error Sebastian Andrzej Siewior
2013-11-12  8:59 ` cppi41: pending patches Vinod Koul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).