cvs commit: src/sys/dev/usb usbdi.c

[Olivier Houchard]

On Tue, Nov 28, 2006 at 09:39:16AM +0000, Ian Dowse wrote:
> In message <20061128.000354.1653105648.imp at bsdimp.com>, "M. Warner Losh" writes
> :
> >In message: <456BD0ED.4050305 at samsco.org>
> >            Scott Long <scottl at samsco.org> writes:
> >: Marius Strobl wrote:
> >: > On Mon, Nov 27, 2006 at 10:32:12PM +0000, Ian Dowse wrote:
> >: >> Thanks for spotting the typo - note though that the recently added
> >: >> bus_dmamap_sync() call appears to be using the wrong bus_dma tag
> >: >> and a potentially uninitialised map, so it is likely to only work
> >: >> on architectures where bus_dmamap_sync doesn't depend on the tag
> >: >> and map.
> >
> >That's weird, because it works on arm which does depend on it.
> 
> There is a cpu_drain_writebuf() call in _bus_dmamap_sync() that
> might be doing the trick on arm (assuming I'm looking at the right
> file). The other operations there appear to rely on being passed
> the correct map.
> 
> >The original problem that motivated the change was that there was data
> >that was returned on some transfers.  This data wasn't sync, which
> >caused us to get weird results when enumerating the device's data
> >structures.  The flush was needed because we don't map the data
> >transfers as coherent (I believe, I've not studied it in a while, and
> >my memory may be failing me.
> 
> The SETUP packets that were mentioned in relation to the change are
> allocated using usb_block_allocmem(), which does specify BUS_DMA_COHERENT
> as far as I can tell (the contents of xfer->request are memcpy'd
> into the reqdma buffer and it is then used). The other bus_dmamap_sync()
> calls in usbdi.c all related to buffers passed in from outside USB
> code, so the bus_dmamap_sync() calls are needed.
> 
> >: BUS_DMA_COHERENT is meant to be a hint in terms of cross-platform 
> >: portability.  A platform that supports it is supposed to then know
> >: to short-circuit the sync calls if they aren't needed.
> >
> >COHERENT is still expensive to implement on some systems as it
> >sometimes get mapped to uncached.  This is OK for small data
> >structures that the usb stack likes to push around internally, but
> >isn't suitable for data that's pushed over the endpoints.
> >
> >NetBSD also offers a BUS_DMA_NOCACHE flag as well.  I don't know how
> >useful it is or where.
> 
> If the arm platform requires bus_dmamap_sync() calls to flush out
> writes even with BUS_DMA_COHERENT allocations, then that would
> explain the original problem. We'll need to add bus_dmamap_sync()
> calls in a number of places in each host controller driver where
> there are data ordering requirements. Can someone confirm that the
> arm platform needs these for correct write ordering?
> 

Arm doesn't need bus_dmamap_sync() with BUS_DMA_COHERENT allocations, except
if the BUS_DMA_COHERENT allocation failed (which shouldn't happen frequently),
it will then silently switch to normal allocation (which shouldn't be a problem,
because the bus_dma documentation clearly says that BUS_DMA_COHERENT doesn't
remove the need for bus_dmamap_sync).

Olivier