On Fri, Jan 07, 2011 at 09:10:20AM -0800, Tony Lindgren wrote:
> Grant,
> 
> Care to queue or ack the following fix?

If you bounce the patch to me, then I'll pick it up into the spi tree
and send it on to Linus right away.  (It didn't get sent to either me
or the spi list, so I don't have the original).

Or if you want to pick it up:

Acked-by: Grant Likely <grant.lik...@secretlab.ca>

Nothing in my tree touches omap_mcspi.c for this merge window, so
there won't be a merge conflict (I'm assuming you want this fix to go
into 2.6.28).

g.

> 
> * Russell King - ARM Linux <li...@arm.linux.org.uk> [110107 07:48]:
> > On Fri, Jan 07, 2011 at 07:56:34PM +0530, Santosh Shilimkar wrote:
> > > > -----Original Message-----
> > > > From: Russell King - ARM Linux [mailto:li...@arm.linux.org.uk]
> > > > Sent: Friday, January 07, 2011 7:55 PM
> > > > To: Santosh Shilimkar
> > > > Cc: linux-omap@vger.kernel.org; Tony Lindgren
> > > > Subject: Re: 4430SDP boot failure
> > > >
> > > > On Fri, Jan 07, 2011 at 06:39:06PM +0530, Santosh Shilimkar wrote:
> > > > > Have sent you latest bootloaders and full bootlog on my ES1.0
> > > > > OMAP4430SDP. 2.6.37 just boots fine for me.
> > > > >
> > > > > Low level debug as you reported seems to be broken though.
> > > >
> > > > Many thanks, the new mlo and uboot gets dhcp/tftp working nicely on
> > > > the board - which should ease the debugging problem as it no longer
> > > > requires anything but the reset button pressed to test a new kernel.
> > > 
> > > Glad to hear that.
> > 
> > And, with one ARM errata disabled, the kernel finally boots.  So the
> > "X-Loader hangs" message means that we did something that non-secure
> > mode prevented us from doing.
> > 
> > It would be a good idea if X-loader could tell dump out the exception,
> > register dump, and for data/prefetch aborts, the relevent FSR and FAR
> > registers - that would make debugging this kind of failure a lot
> > easier.
> > 
> > =================
> > Subject: Fix DMA API usage in OMAP MCSPI driver
> > 
> > Running the latest kernel on the 4430SDP board with DMA API debugging
> > enabled results in this:
> > 
> > WARNING: at lib/dma-debug.c:803 check_unmap+0x19c/0x6f0()
> > NULL NULL: DMA-API: device driver tries to free DMA memory it has not 
> > allocated
> > [device address=0x000000008129901a] [size=260 bytes]
> > Modules linked in:
> > Backtrace:
> > [<c003cbe0>] (dump_backtrace+0x0/0x10c) from [<c0278da8>] 
> > (dump_stack+0x18/0x1c)
> >  r7:c1839dc0 r6:c0198578 r5:c0304b17 r4:00000323
> > [<c0278d90>] (dump_stack+0x0/0x1c) from [<c005b158>] 
> > (warn_slowpath_common+0x58/0x70)
> > [<c005b100>] (warn_slowpath_common+0x0/0x70) from [<c005b214>] 
> > (warn_slowpath_fmt+0x38/0x40)
> >  r8:c1839e40 r7:00000000 r6:00000104 r5:00000000 r4:8129901a
> > [<c005b1dc>] (warn_slowpath_fmt+0x0/0x40) from [<c0198578>] 
> > (check_unmap+0x19c/0x6f0)
> >  r3:c03110de r2:c0304e6b
> > [<c01983dc>] (check_unmap+0x0/0x6f0) from [<c0198cd8>] 
> > (debug_dma_unmap_page+0x74/0x80)
> > [<c0198c64>] (debug_dma_unmap_page+0x0/0x80) from [<c01d5ad8>] 
> > (omap2_mcspi_work+0x514/0xbf0)
> > [<c01d55c4>] (omap2_mcspi_work+0x0/0xbf0) from [<c006dfb0>] 
> > (process_one_work+0x294/0x400)
> > [<c006dd1c>] (process_one_work+0x0/0x400) from [<c006e50c>] 
> > (worker_thread+0x220/0x3f8)
> > [<c006e2ec>] (worker_thread+0x0/0x3f8) from [<c00738d0>] (kthread+0x88/0x90)
> > [<c0073848>] (kthread+0x0/0x90) from [<c005e924>] (do_exit+0x0/0x5fc)
> >  r7:00000013 r6:c005e924 r5:c0073848 r4:c1829ee0
> > ---[ end trace 1b75b31a2719ed20 ]---
> > 
> > I've no idea why this driver uses NULL for dma_unmap_single instead of
> > the &spi->dev that is laying around just waiting to be used in that
> > function - but it's an easy fix.
> > 
> > Also replace this comment with a FIXME comment:
> >                 /* Do DMA mapping "early" for better error reporting and
> >                  * dcache use.  Note that if dma_unmap_single() ever starts
> >                  * to do real work on ARM, we'd need to clean up mappings
> >                  * for previous transfers on *ALL* exits of this loop...
> >                  */
> > as the comment is not true - we do work in dma_unmap() functions,
> > particularly on ARMv6 and above.  I've corrected the existing unmap
> > functions but if any others are required they must be added ASAP.
> > 
> > Signed-off-by: Russell King <rmk+ker...@arm.linux.org.uk>
> 
> Acked-by: Tony Lindgren <t...@atomide.com>
> 
> 
> > ---
> > I'm surprised this driver got merged as it has such a comment, and is
> > abusing the DMA API... well, at least with the DMA API debugging we
> > can now catch this stuff.
> > 
> > diff --git a/drivers/spi/omap2_mcspi.c b/drivers/spi/omap2_mcspi.c
> > index 951a160..abb1ffb 100644
> > --- a/drivers/spi/omap2_mcspi.c
> > +++ b/drivers/spi/omap2_mcspi.c
> > @@ -397,7 +397,7 @@ omap2_mcspi_txrx_dma(struct spi_device *spi, struct 
> > spi_transfer *xfer)
> >  
> >     if (tx != NULL) {
> >             wait_for_completion(&mcspi_dma->dma_tx_completion);
> > -           dma_unmap_single(NULL, xfer->tx_dma, count, DMA_TO_DEVICE);
> > +           dma_unmap_single(&spi->dev, xfer->tx_dma, count, DMA_TO_DEVICE);
> >  
> >             /* for TX_ONLY mode, be sure all words have shifted out */
> >             if (rx == NULL) {
> > @@ -412,7 +412,7 @@ omap2_mcspi_txrx_dma(struct spi_device *spi, struct 
> > spi_transfer *xfer)
> >  
> >     if (rx != NULL) {
> >             wait_for_completion(&mcspi_dma->dma_rx_completion);
> > -           dma_unmap_single(NULL, xfer->rx_dma, count, DMA_FROM_DEVICE);
> > +           dma_unmap_single(&spi->dev, xfer->rx_dma, count, 
> > DMA_FROM_DEVICE);
> >             omap2_mcspi_set_enable(spi, 0);
> >  
> >             if (l & OMAP2_MCSPI_CHCONF_TURBO) {
> > @@ -1025,11 +1025,6 @@ static int omap2_mcspi_transfer(struct spi_device 
> > *spi, struct spi_message *m)
> >             if (m->is_dma_mapped || len < DMA_MIN_BYTES)
> >                     continue;
> >  
> > -           /* Do DMA mapping "early" for better error reporting and
> > -            * dcache use.  Note that if dma_unmap_single() ever starts
> > -            * to do real work on ARM, we'd need to clean up mappings
> > -            * for previous transfers on *ALL* exits of this loop...
> > -            */
> >             if (tx_buf != NULL) {
> >                     t->tx_dma = dma_map_single(&spi->dev, (void *) tx_buf,
> >                                     len, DMA_TO_DEVICE);
> > @@ -1046,7 +1041,7 @@ static int omap2_mcspi_transfer(struct spi_device 
> > *spi, struct spi_message *m)
> >                             dev_dbg(&spi->dev, "dma %cX %d bytes error\n",
> >                                             'R', len);
> >                             if (tx_buf != NULL)
> > -                                   dma_unmap_single(NULL, t->tx_dma,
> > +                                   dma_unmap_single(&spi->dev, t->tx_dma,
> >                                                     len, DMA_TO_DEVICE);
> >                             return -EINVAL;
> >                     }
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to