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

[Scott Long]

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.
> 
>    Sam
> 

Btw, the dmamap is opaque.  Drivers should _always_ treat it as having a
valid value, and also as having a value that they cannot test.  A map
__MUST__ be created for every concurrent transaction that will be
outstanding in the subsystem.  I have plans to make the map carry the
S/G list instead of the tag, and once that happens, drivers that are
sloppy with their tag (assuming that they know what the value is, or
assuming that one map can handle multiple transactions) will stop
working.

Scott


Scott