diff mbox

[0/8] fsldma: lockup fixes

Message ID 20110301195208.GC23403@ovro.caltech.edu (mailing list archive)
State Not Applicable
Headers show

Commit Message

Ira Snyder March 1, 2011, 7:52 p.m. UTC 
On Tue, Mar 01, 2011 at 08:55:15AM -0800, Ira W. Snyder wrote:

[ big snip ]

> 
> Thanks, this is exactly what I was going to ask for next. :)
> 
> I really don't understand why the P2020 DMA controller isn't behaving
> nicely after my patches.
> 
> Can you run a git bisect to figure out which patch in the series causes
> the problems. It should take three or four build + test cycles to narrow
> down which patch breaks the driver. When it is finished, send me the
> output of git bisect.
> 
> Like this (assuming you have two branches: master and
> fsldma, where fsldma is master + my patches):
> 
> # setup the bisect
> git bisect start
> git bisect bad fsldma
> git bisect good master
> 
> # build and test the kernel using the same test as before:
> modprobe dmatest max_channels=1 threads_per_chan=1 iterations=4
> 
> # if the test passes:
> git bisect good
> 
> # if the test fails:
> git bisect bad
> 
> # now build + test again, then mark that good or bad. Repeat until
> # finished.
> 
> 
> I really appreciate your help in testing this. You've been great at
> providing everything I've asked for.
> 

I'd still like the bisect if you have a chance. I've re-reviewed the
patch series, and found the places that change register writes to the
controller.

The patch below changes the register operations back to the original
order. It doesn't make any sense why this would be required, but it is
worth a quick try.

I've added an "XXX" mark where you can comment out a single line if this
patch fails. It is highly unlikely to make any difference, but I'm
really having a hard time understanding what is wrong.

Ira


From 9e479ce27f8c1819694d7082bb4a27772b4baf52 Mon Sep 17 00:00:00 2001
From: Ira W. Snyder <iws@ovro.caltech.edu>
Date: Tue, 1 Mar 2011 11:43:00 -0800
Subject: [PATCH] fsldma: try and fix 85xx DMA controller

This is just a random guess at what might be wrong. The datasheet
doesn't say that a completed transfer must be aborted before starting a
new transfer (nor does it make much sense). However, the old code did it
anyway.

NOT AT ALL Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---
 drivers/dma/fsldma.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

Comments

Felix Radensky March 2, 2011, 5:49 a.m. UTC | #1 
Hi Ira,

On 03/01/2011 09:52 PM, Ira W. Snyder wrote:
> On Tue, Mar 01, 2011 at 08:55:15AM -0800, Ira W. Snyder wrote:
>
> [ big snip ]
>
>
> I'd still like the bisect if you have a chance. I've re-reviewed the
> patch series, and found the places that change register writes to the
> controller.
>
> The patch below changes the register operations back to the original
> order. It doesn't make any sense why this would be required, but it is
> worth a quick try.
>
> I've added an "XXX" mark where you can comment out a single line if this
> patch fails. It is highly unlikely to make any difference, but I'm
> really having a hard time understanding what is wrong.
>
>
This patch fixes the problem. See below

__dma_request_channel: success (dma0chan0)
dmatest: Started 1 threads using dma0chan0
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372000 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372060 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3720c0 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372120 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372180 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3721e0 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372240 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3722a0 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: assign cookies: start=0
of:fsl-elo-dma ffe0c300.dma: chan0: assign cookies: end=8
of:fsl-elo-dma ffe0c300.dma: chan0: idle, starting controller
of:fsl-elo-dma ffe0c300.dma: chan0: 85xx, running workaround
of:fsl-elo-dma ffe0c300.dma: chan0: dma_halt mode=0x08000140
of:fsl-elo-dma ffe0c300.dma: chan0: irq: stat = 0x8
of:fsl-elo-dma ffe0c300.dma: chan0: irq: End-of-link INT
of:fsl-elo-dma ffe0c300.dma: chan0: irq: Exit
of:fsl-elo-dma ffe0c300.dma: chan0: tasklet entry
of:fsl-elo-dma ffe0c300.dma: chan0: completed_cookie=8
of:fsl-elo-dma ffe0c300.dma: chan0: no pending LDs
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372000 callback
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372000 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372060 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3720c0 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372120 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372180 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3721e0 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372240 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3722a0 free
of:fsl-elo-dma ffe0c300.dma: chan0: tasklet exit
dma0chan0-copy0: verifying source buffer...
dma0chan0-copy0: verifying dest buffer...
dma0chan0-copy0: #0: No errors with src_off=0x10a8 dst_off=0x1914 
len=0x1dca
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3722a0 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372240 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: assign cookies: start=8
of:fsl-elo-dma ffe0c300.dma: chan0: assign cookies: end=10
of:fsl-elo-dma ffe0c300.dma: chan0: idle, starting controller
of:fsl-elo-dma ffe0c300.dma: chan0: 85xx, running workaround
of:fsl-elo-dma ffe0c300.dma: chan0: dma_halt mode=0x08000141
of:fsl-elo-dma ffe0c300.dma: chan0: irq: stat = 0x8
of:fsl-elo-dma ffe0c300.dma: chan0: irq: End-of-link INT
of:fsl-elo-dma ffe0c300.dma: chan0: irq: Exit
of:fsl-elo-dma ffe0c300.dma: chan0: tasklet entry
of:fsl-elo-dma ffe0c300.dma: chan0: completed_cookie=10
of:fsl-elo-dma ffe0c300.dma: chan0: no pending LDs
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3722a0 callback
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3722a0 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372240 free
of:fsl-elo-dma ffe0c300.dma: chan0: tasklet exit
dma0chan0-copy0: verifying source buffer...
dma0chan0-copy0: verifying dest buffer...
dma0chan0-copy0: #1: No errors with src_off=0xdb8 dst_off=0xc14 len=0x526
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372240 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3722a0 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3721e0 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372180 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372120 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3720c0 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372060 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: assign cookies: start=10
of:fsl-elo-dma ffe0c300.dma: chan0: assign cookies: end=17
of:fsl-elo-dma ffe0c300.dma: chan0: idle, starting controller
of:fsl-elo-dma ffe0c300.dma: chan0: 85xx, running workaround
of:fsl-elo-dma ffe0c300.dma: chan0: dma_halt mode=0x08000141
of:fsl-elo-dma ffe0c300.dma: chan0: irq: stat = 0x8
of:fsl-elo-dma ffe0c300.dma: chan0: irq: End-of-link INT
of:fsl-elo-dma ffe0c300.dma: chan0: irq: Exit
of:fsl-elo-dma ffe0c300.dma: chan0: tasklet entry
of:fsl-elo-dma ffe0c300.dma: chan0: completed_cookie=17
of:fsl-elo-dma ffe0c300.dma: chan0: no pending LDs
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372240 callback
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372240 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3722a0 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3721e0 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372180 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372120 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3720c0 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372060 free
of:fsl-elo-dma ffe0c300.dma: chan0: tasklet exit
dma0chan0-copy0: verifying source buffer...
dma0chan0-copy0: verifying dest buffer...
dma0chan0-copy0: #2: No errors with src_off=0xf48 dst_off=0x85d len=0x188f
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372060 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3720c0 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372120 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372180 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3721e0 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3722a0 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372240 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372000 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372300 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372360 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3723c0 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372420 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372480 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3724e0 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372540 allocated
of:fsl-elo-dma ffe0c300.dma: chan0: assign cookies: start=17
of:fsl-elo-dma ffe0c300.dma: chan0: assign cookies: end=32
of:fsl-elo-dma ffe0c300.dma: chan0: idle, starting controller
of:fsl-elo-dma ffe0c300.dma: chan0: 85xx, running workaround
of:fsl-elo-dma ffe0c300.dma: chan0: dma_halt mode=0x08000141
of:fsl-elo-dma ffe0c300.dma: chan0: irq: stat = 0x8
of:fsl-elo-dma ffe0c300.dma: chan0: irq: End-of-link INT
of:fsl-elo-dma ffe0c300.dma: chan0: irq: Exit
of:fsl-elo-dma ffe0c300.dma: chan0: tasklet entry
of:fsl-elo-dma ffe0c300.dma: chan0: completed_cookie=32
of:fsl-elo-dma ffe0c300.dma: chan0: no pending LDs
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372060 callback
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372060 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3720c0 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372120 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372180 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3721e0 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3722a0 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372240 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372000 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372300 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372360 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3723c0 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372420 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372480 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef3724e0 free
of:fsl-elo-dma ffe0c300.dma: chan0: LD ef372540 free
of:fsl-elo-dma ffe0c300.dma: chan0: tasklet exit
dma0chan0-copy0: verifying source buffer...
dma0chan0-copy0: verifying dest buffer...
dma0chan0-copy0: #3: No errors with src_off=0x4ff dst_off=0x453 len=0x395d
dma0chan0-copy0: terminating after 4 tests, 0 failures (status 0)

Felix.
Ira Snyder March 2, 2011, 4:42 p.m. UTC | #2 
On Wed, Mar 02, 2011 at 07:49:57AM +0200, Felix Radensky wrote:
> Hi Ira,
> 
> On 03/01/2011 09:52 PM, Ira W. Snyder wrote:
> > On Tue, Mar 01, 2011 at 08:55:15AM -0800, Ira W. Snyder wrote:
> >
> > [ big snip ]
> >
> >
> > I'd still like the bisect if you have a chance. I've re-reviewed the
> > patch series, and found the places that change register writes to the
> > controller.
> >
> > The patch below changes the register operations back to the original
> > order. It doesn't make any sense why this would be required, but it is
> > worth a quick try.
> >
> > I've added an "XXX" mark where you can comment out a single line if this
> > patch fails. It is highly unlikely to make any difference, but I'm
> > really having a hard time understanding what is wrong.
> >
> >
> This patch fixes the problem. See below
> 

Excellent! I know what is happening now. The 85xx controller doesn't
clear the "channel start" bit at the end of a transfer. Sure enough,
buried near the end of the chapter, the datasheet implies this in a
table very far away from the register definitions. The 83xx datasheet
explicitly states that it clears this bit automatically.

I'll post an updated patch series later today. Thank you so much for
being patient and trying out all of these patches.

Ira
diff mbox

Patch

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index e4d9d17..d8eedbc 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -213,6 +213,7 @@  static void dma_halt(struct fsldma_chan *chan)
 	int i;
 
 	mode = DMA_IN(chan, &chan->regs->mr, 32);
+	dev_dbg(chan->dev, "%s: dma_halt mode=0x%.8x\n", chan->name, mode);
 	mode |= FSL_DMA_MR_CA;
 	DMA_OUT(chan, &chan->regs->mr, mode, 32);
 
@@ -921,10 +922,24 @@  static void fsl_chan_xfer_ld_queue(struct fsldma_chan *chan)
 	list_splice_tail_init(&chan->ld_pending, &chan->ld_running);
 
 	/*
+	 * XXX: Guess at problems
+	 *
+	 * The 85xx requires that you run this routine before you try to start
+	 * the next DMA for an as yet unknown reason. Maybe.
+	 */
+	if ((chan->feature & FSL_DMA_IP_MASK) == FSL_DMA_IP_85XX) {
+		dev_dbg(chan->dev, "%s: 85xx, running workaround\n", name);
+		dma_halt(chan);
+	}
+
+	/*
 	 * Program the descriptor's address into the DMA controller,
 	 * then start the DMA transaction
 	 */
 	set_cdar(chan, desc->async_tx.phys);
+
+
+	/* XXX: if that doesn't work, comment the "get_cdar()" line below */
 	get_cdar(chan);
 
 	dma_start(chan);