On Thu, 4 Feb 2010, Albert Herranz wrote:
> Hi Alan,
>
> Alan Stern wrote:
> > This description sounds hopelessly confused. Maybe you're just
> > misusing the term "coherent". The patch itself doesn't affect the
> > coherent DMA mappings anyway; it affects the streaming mappings. Or to
> > put it another way, what's the justification for replacing a call to
> > dma_map_single() with a call to dma_alloc_coherent()?
> >
> > Since the patch doesn't affect any of the coherent mappings (see for
> > example the calls to dma_pool_create() in ehci-mem.c), I don't see how
> > it can possibly do what you claim.
> >
>
> Thanks for your comments. Let's try to hopefully clarify this a bit.
>
> I've used the term "coherent" as described in Documentation/DMA-API.txt (aka
> "consistent" as used in PCI-related functions).
> I've tried to describe first the limitations of the platform that I'm working
> on. Basically, one of the annoying things of that platform is that writes to
> uncached memory (as used in "coherent" memory) can only be reliably performed
> in 32-bit accesses.
>
> The USB subsystem ends up using "coherent" memory for buffers and/or other
> structures in different ways.
>
> The "coherent" memory allocated in dma_pool_create() in ehci-mem.c that you
> report is not a problem at all because it is always accessed in 32-bit chunks
> (it hasn't been always like that but since commit
> 3807e26d69b9ad3864fe03224ebebc9610d5802e "USB: EHCI: split ehci_qh into hw
> and sw parts" this got addressed as a side effect, so I didn't need to post
> another patch for that).
On a 64-bit processor, some of the accesses will be 64 bits wide
instead of 32. Does that matter for your purposes?
What about ohci-hcd and uhci-hcd? They both use non-32-bit accesses to
structures in coherent memory.
> Other possible interactions with "coherent" memory are those involving
> buffers used in USB transactions, which may be allocated via the USB
> subsystem (at usb_buffer_alloc() or when bounced via hcd_alloc_coherent()) or
> which may come already allocated and ready for use
> (URB_NO_{SETUP,TRANSFER}_DMA_MAP).
Ah yes, quite correct. And this indicates that you need to concentrate
on usb_buffer_alloc(). On your system (or rather, whenever the
HCD_NO_COHERENT_MEM flag is set) it should allocate normal memory and
set the DMA pointer to NULL.
Then map_urb_for_dma() should check the urb->setup_dma and
urb->transfer_dma pointers; if a pointer is NULL then the
corresponding urb->URB_NO_SETUP_DMA_MAP or urb->NO_TRANSFER_DMA_MAP
flag should be ignored (and probably should be cleared so as to
avoid confusing unmap_urb_for_dma()).
> The patch, as posted, allocates normal memory for USB buffers _within_ the
> USB subsystem and invariably bounces all buffers to new "coherent" buffers.
> So, basically, what the patch claims (avoid 32-bit writes for "coherent"
> memory within the USB subsystem) is "done" (hey, it actually works ;-).
>
> But I think you have raised valid points here :)
>
> If the "coherent" memory is already allocated and passed (as already
> dma-mapped) to the USB subsystem then there is no gain in bouncing the buffer:
> - if a non-32 bit write was done to that "coherent" memory the damage is
> already done
> - if the "coherent" memory was written always in 32-bit accesses then we can
> just safely use it
> So bouncing here should be avoided as it is unneeded.
>
> On the other hand, we can control USB buffers managed by the USB subsystem
> itself.
> That's what the patch "does". It avoids access restrictions to USB buffers by
> allocating them from normal memory (leaving USB drivers free to access those
> buffers in whatever bus width they need, as they do today) ... and bouncing
> them.
> The thing here is that it makes no sense to bounce them to "coherent" memory
> if they can be dma-mapped directly (as you point in your
> dma_map_single-vs-dma_alloc_coherent comment).
>
> So... that's what RFCs are for :)
If you do it as described above then the buffers you're worried about
won't be allocated in coherent memory to begin with, so no problems
will arise.
Alan Stern
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
