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

[Scott Long]

John Baldwin wrote:
> 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.).
> 

Well, this is why drivers need to treat the maps and opaque and valid in
order to be portable across architectures =-)

Scott