[coreboot] USB changes

Patrick Georgi patrick at georgi-clan.de
Wed Jul 22 12:10:59 CEST 2009


Peter, thank you for your review!

Peter Stuge schrieb:
>> Subject: [PATCH 3/5] usb: API change, control receive endpoint_t
>>
>> Changed the usb API where the control function first parameter now
>> is a pointer of endpoint_t instead of a pointer of usbdevice_t.
>>     
>
> Changing the API like this is a good thing.
>   
The old API was based on a misunderstanding of the spec on my part, and
the little detail that so far the stack doesn't have drivers for devices
with more than one control pipe. So yes, this is a very good thing! :-)
(and necessary because OHCI does things in different ways than UHCI in
that area)
>> @@ -109,7 +109,7 @@ set_feature (usbdev_t *dev, int endp, int feature, int rtype)
>> +	dev->controller->control (&dev->endpoints[endp], OUT, sizeof (dr), &dr, 0, 0);
>>     
>
> This is good. (Or is it? Is endp specified in the API to be the
> index, and not the endpoint number?)
>   
There's no correlation between endpoints[x] and endpoint number x in the
code. Simply because there can be two endpoints x(in) and x(out).
The only one that's guaranteed is endpoints[0] which points to endpoint
0 - but that one is a special case as direction doesn't matter for it
(according to the spec).
>> @@ -183,7 +184,7 @@ set_configuration (usbdev_t *dev)
>> +	dev->controller->control (&dev->endpoints[0], OUT, sizeof (dr), &dr, 0, 0);
>>     
>
> Is index 0 in endpoints guaranteed to always be the default endpoint?
>   
drivers/usb/usb.c(set_address) initializes endpoints[0] to the default
endpoint (type control, endpoint 0, proper maxPacketSize).

Given that this is the endpoint used to do basic initialization, I doubt
any device vendor is stupid enough to move the default endpoint to
somewhere else after that.
And if there is such a device, the special device driver for that device
could still change endpoints[0] easily.


Regards,
Patrick




More information about the coreboot mailing list