cvs commit: src/sys/dev/usb ehci.c ohci.c

[John Baldwin]

On Wednesday 23 April 2008 05:03:27 pm Sam Leffler 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.
> >   
> Thanks for explaining, I will look.  Please hold off for a bit.

Also, other archs may not be crashing because some of them use a NULL map for 
bus_dmamem_alloc() as a sentinel of sorts so that sync operations are nops 
(since bus_dmamem_alloc() won't alloc memory that needs to be bounced, etc.).

-- 
John Baldwin