Re: PXA270 SSP DMA Corruption

[Ned Forrester <nforrester-/d+BM93fTQY@public.gmane.org>]

J. Scott Merritt wrote:
> On Wed, 12 Nov 2008 18:10:01 -0500
> Ned Forrester <nforrester-/d+BM93fTQY@public.gmane.org> wrote:
> 
>> 2. spidev uses the same buffer for tx and rx.  That is supposed to be
>> allowed, but I don't think pxa2xx_spi handles this case correctly.
>> pxa2xx_spi handles NULL buffers (for uni-directional transfers), and it
>> uses dma_map_single to map the rx buffers as DMA_FROM_DEVICE, and the tx
>> buffer as DMA_TO_DEVICE, without checking whether the rx and tx buffers
>> are the same.  Thus if they are the same, the memory is mapped twice.
>> "Linux Device Drivers", Corbet, et al. does not address this
>> possibility, but I bet it is not a good thing to do.
>>
>> If you are willing, it looks simple to modify spidev to use two buffers,
>> and test whether that works better.  I agree that spidev should work
>> either way, but this would be a quick test.  If that works, I will
>> submit a patch for pxa2xx_spi to fix the case of same buffers.
>>
>> Alternatively, you might prefer to try fixing map_dma_buffers() and
>> unmap_dma_buffers() in pxa2xx_spi.c to test this theory.  If it works,
>> that would be the proper fix, anyway.  Basically it needs to trap the
>> case of same address (or even overlapping ranges) and do one
>> dma_map_single using the DMA_BIDIRECTIONAL flag.  Unmapping has to use
>> the same flag.  I think I would choose to make it fail on overlapped but
>> unequal ranges, and perform correctly for the cases of separate or equal.
>>
> [snip performance suggestions]
> 
> Ned,  Thank for your thorough and thoughtful review.
> 
> It looks like your concerns regarding the duplicate mapping of the
> overlapping buffers were correct.  I tried both suggestions - namely,
> using two buffers in spidev, as well as cutting back to a single buffer
> mapped with DMA_BIDIRECTIONAL in pxa2xx_spi.c.  Each of these changes
> (by themselves) seemed to eliminate the data corruption in my simple
> test program !

That's good.  I will write a patch for pxa2xx_spi, unless you would
rather do it.

> However, I have some lingering concerns regarding the latter solution.
>>From what little I have read, it appears the DMA-BIDIRECTIONAL is intended
> for situations where the transfer direction is not know ... it is not
> immediately clear (to me) that it also handles our case, which might be
> better described as DMA_SIMULTANEOUS.  It is possible that using
> DMA_BIDIRECTIONAL is sufficient, but without a much deeper understanding
> of the buffering and caching that is going on between the two independent
> DMA channel it is difficult for me to speculate.

According to "Linux Device Drivers":

"If data can move in either direction, use DMA_BIDIRECTIONAL."

and

"Incidentally, bounce buffers are one reason why it is important to get
the direction right. DMA_BIDIRECTIONAL bounce buffers are copied both
before and after the operation, which is often an unnecessary waste of
CPU cycles."

>From the general discussion in the book, it is clear that dma mapping
takes care of issues like flushing cache (for DMA_TO_DEVICE),
invalidating cache (for DMA_FROM_DEVICE), setting up any I/O memory
mapping and scatter/gather, etc.  Once stream mapped (which
dma_map_single() does), the CPU is not allowed to touch the memory, and
once unmapped, the device cannot touch the memory.  I think it is clear
that DMA_BIDIRECTIONAL is for exactly the case where the device will do
some DMA in each direction before giving the memory back to the CPU.

-- 
Ned Forrester                                       nforrester-/d+BM93fTQY@public.gmane.org
Oceanographic Systems Lab                                  508-289-2226
Applied Ocean Physics and Engineering Dept.
Woods Hole Oceanographic Institution          Woods Hole, MA 02543, USA
http://www.whoi.edu/sbl/liteSite.do?litesiteid=7212
http://www.whoi.edu/hpb/Site.do?id=1532
http://www.whoi.edu/page.do?pid=10079


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/