Date: Wed, 23 Apr 2008 14:08:52 -0700 From: Sam Leffler <sam@freebsd.org> To: Scott Long <scottl@samsco.org> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org, Marius Strobl <marius@alchemy.franken.de> Subject: Re: cvs commit: src/sys/dev/usb ehci.c ohci.c Message-ID: <480FA564.5000008@freebsd.org> In-Reply-To: <480FA4B4.50006@samsco.org> References: <200803201619.m2KGJQr7033985@repoman.freebsd.org> <20080412193358.GA44768@alchemy.franken.de> <20080423203622.GA66545@alchemy.franken.de> <480F9F8B.5050209@freebsd.org> <20080423205943.GG99566@alchemy.franken.de> <480FA4B4.50006@samsco.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Scott Long wrote: > Marius Strobl wrote: >> On Wed, Apr 23, 2008 at 01:43:55PM -0700, Sam Leffler wrote: >>> Marius Strobl wrote: >>>> On Sat, Apr 12, 2008 at 09:33:58PM +0200, Marius Strobl wrote: >>>> >>>>> On Thu, Mar 20, 2008 at 04:19:26PM +0000, Sam Leffler wrote: >>>>> >>>>>> sam 2008-03-20 16:19:25 UTC >>>>>> >>>>>> FreeBSD src repository >>>>>> >>>>>> Modified files: >>>>>> sys/dev/usb ehci.c ohci.c Log: >>>>>> Workaround design botch in usb: blindly mixing bus_dma with PIO >>>>>> does not >>>>>> work on architectures with a write-back cache as the PIO writes >>>>>> end up >>>>>> in the cache which the sync(BUS_DMASYNC_POSTREAD) in >>>>>> usb_transfer_complete >>>>>> then discards; compensate in the xfer methods that do PIO by >>>>>> pushing the >>>>>> writes out of the cache before usb_transfer_complete is called. >>>>>> >>>>>> This fixes USB on xscale and likely other places. >>>>>> >>>>>> Sponsored by: hobnob >>>>>> Reviewed by: cognet, imp >>>>>> MFC after: 1 month >>>>>> >>>>>> Revision Changes Path >>>>>> 1.62 +16 -0 src/sys/dev/usb/ehci.c >>>>>> 1.171 +16 -0 src/sys/dev/usb/ohci.c >>>>>> >>>>> This causes a crash during boot on sparc64. Looks like map is still >>>>> NULL at that point. >>>>> >>>>> >>>> Are you ok with the change below or would that also prevent >>>> your kludge from taking effect? >>>> >>>> Marius >>>> >>>> Index: ehci.c >>>> =================================================================== >>>> RCS file: /usr/data/bsd/cvs/fbsd/src/sys/dev/usb/ehci.c,v >>>> retrieving revision 1.62 >>>> diff -u -r1.62 ehci.c >>>> --- ehci.c 20 Mar 2008 16:19:25 -0000 1.62 >>>> +++ ehci.c 23 Apr 2008 20:23:58 -0000 >>>> @@ -664,6 +664,8 @@ >>>> usbd_pipe_handle pipe = xfer->pipe; >>>> bus_dma_tag_t tag = pipe->device->bus->buffer_dmatag; >>>> struct usb_dma_mapping *dmap = &xfer->dmamap; >>>> + if (dmap->map == NULL) >>>> + return; >>>> bus_dmamap_sync(tag, dmap->map, BUS_DMASYNC_PREWRITE); >>>> } >>>> >>>> Index: ohci.c >>>> =================================================================== >>>> RCS file: /usr/data/bsd/cvs/fbsd/src/sys/dev/usb/ohci.c,v >>>> retrieving revision 1.171 >>>> diff -u -r1.171 ohci.c >>>> --- ohci.c 20 Mar 2008 16:19:25 -0000 1.171 >>>> +++ ohci.c 21 Apr 2008 19:13:54 -0000 >>>> @@ -1571,6 +1571,8 @@ >>>> usbd_pipe_handle pipe = xfer->pipe; >>>> bus_dma_tag_t tag = pipe->device->bus->buffer_dmatag; >>>> struct usb_dma_mapping *dmap = &xfer->dmamap; >>>> + if (dmap->map == NULL) >>>> + return; >>>> bus_dmamap_sync(tag, dmap->map, BUS_DMASYNC_PREWRITE); >>>> } >>>> >>>> >>>> >>>> >>> You have not identified why you don't have a dma map. I don't have >>> a way to diagnose your problem and so far as I know no other >>> platform had an issue w/ the change. I suggest you figure out why >>> your map is not setup instead of adding a hack. >>> >> >> It's because the usb(4) code doesn't create DMA maps for >> zero-length transfers, see usbd_transfer(). In the case of >> the backtrace I posted not for usbd_set_address(), which >> does USETW(req.wLength, 0) so later on size is 0 in >> usbd_transfer() hence no DMA map. I don't know why your >> hack doesn't also crash other platforms. >> >> > > If there's a transfer without a DMA operation, then why is the > DMA API being used? > It should not be; that's the whole crux of the problem. It doesn't have different "transport methods" to split out PIO from DMA; it just sends everything through one set of routines. That's why I had to add my kludge. Sam
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?480FA564.5000008>