From owner-cvs-all@FreeBSD.ORG Tue Nov 28 12:02:34 2006 Return-Path: X-Original-To: cvs-all@FreeBSD.org Delivered-To: cvs-all@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 66EA216A47B for ; Tue, 28 Nov 2006 12:02:34 +0000 (UTC) (envelope-from doginou@dong.ci0.org) Received: from dong.ci0.org (cognet.ci0.org [80.65.224.102]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0C7FE43CC9 for ; Tue, 28 Nov 2006 12:01:55 +0000 (GMT) (envelope-from doginou@dong.ci0.org) Received: from dong.ci0.org (localhost.ci0.org [127.0.0.1]) by dong.ci0.org (8.13.7/8.13.4) with ESMTP id kASCJKsK020988; Tue, 28 Nov 2006 13:19:20 +0100 (CET) (envelope-from doginou@dong.ci0.org) Received: (from doginou@localhost) by dong.ci0.org (8.13.7/8.13.4/Submit) id kASCJHPe020987; Tue, 28 Nov 2006 13:19:17 +0100 (CET) (envelope-from doginou) Date: Tue, 28 Nov 2006 13:19:17 +0100 From: Olivier Houchard To: Ian Dowse Message-ID: <20061128121917.GA20946@ci0.org> References: <20061128.000354.1653105648.imp@bsdimp.com> <200611280939.aa56740@nowhere.iedowse.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200611280939.aa56740@nowhere.iedowse.com> User-Agent: Mutt/1.4.1i Cc: scottl@samsco.org, src-committers@FreeBSD.org, cvs-src@FreeBSD.org, cvs-all@FreeBSD.org, "M. Warner Losh" Subject: Re: cvs commit: src/sys/dev/usb usbdi.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Nov 2006 12:02:34 -0000 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 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