Date: Tue, 28 Nov 2006 09:39:16 +0000 From: Ian Dowse <iedowse@iedowse.com> To: "M. Warner Losh" <imp@bsdimp.com> Cc: cvs-src@FreeBSD.org, scottl@samsco.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org, marius@alchemy.franken.de Subject: Re: cvs commit: src/sys/dev/usb usbdi.c Message-ID: <200611280939.aa56740@nowhere.iedowse.com> In-Reply-To: Your message of "Tue, 28 Nov 2006 00:03:54 MST." <20061128.000354.1653105648.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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? BTW, it looks like at the time the newly added bus_dmamap_sync() call is executed, the SETUP packet data hasn't yet been copied into the BUS_DMA_COHERENT buffer it will eventually use (that happens in the pipe->methods->transfer() call afterwards). Is there more information such as USB_DEBUG traces showing the original problem? Ian
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200611280939.aa56740>