Date: Tue, 28 Nov 2006 13:19:17 +0100 From: Olivier Houchard <cognet@ci0.org> To: Ian Dowse <iedowse@iedowse.com> Cc: scottl@samsco.org, src-committers@FreeBSD.org, cvs-src@FreeBSD.org, cvs-all@FreeBSD.org, "M. Warner Losh" <imp@bsdimp.com> Subject: Re: cvs commit: src/sys/dev/usb usbdi.c Message-ID: <20061128121917.GA20946@ci0.org> In-Reply-To: <200611280939.aa56740@nowhere.iedowse.com> References: <20061128.000354.1653105648.imp@bsdimp.com> <200611280939.aa56740@nowhere.iedowse.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Nov 28, 2006 at 09:39:16AM +0000, Ian Dowse wrote: > In message <20061128.000354.1653105648.imp@bsdimp.com>, "M. Warner Losh" writes > : > >In message: <456BD0ED.4050305@samsco.org> > > Scott Long <scottl@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20061128121917.GA20946>