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

[Sam Leffler]

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