FS#12497 - Reorganize USB initialization to make it work with FreeBSD

Attached to Project: Rockbox
Opened by Bartosz Fabianowski (undo) - Friday, 30 December 2011, 20:34 GMT
Last edited by Frank Gevaerts (fg) - Wednesday, 04 January 2012, 22:21 GMT
Task Type Patches
Category Operating System/Drivers
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Release 3.10
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


When USB_DETECT_BY_CORE is defined (as is the case for almost all targets), the Rockbox USB stack distinguishes between connections to a USB charger and a real USB host. Initially, a charger is assumed and all USB drivers are disabled. If a real USB host is present, the first control request it sends triggers the usb_drv_usb_detect_event() callback which in turn enables the USB HID and/or mass storage drivers. Unfortunately, the call to allocate_interfaces_and_endpoints() which should allocate the endpoints required by these drivers has already occurred at that point and cannot simply be repeated due to its side effects. Instead, the USB stack relies on the host issuing a reset that will cause allocate_interfaces_and_endpoints() to be run again. This reset is issued by Linux, Windows and OS X but not by other operating systems such as FreeBSD.

The attached patch implements a fix as discussed on IRC. A new function usb_set_host_present() ensures that USB HID and/or mass storage drivers are enabled *before* the handling of the first control request begins and allocate_interfaces_and_endpoints() is called. This new function is located in usb.c, cleaning up the USB code by removing the need for a callback from usb_core.c. The define that controls this functionality is therefore renamed from USB_DETECT_BY_CORE to USB_DETECT_BY_REQUEST.

This patch has been tested with FreeBSD and Linux USB hosts.
This task depends upon

Closed by  Frank Gevaerts (fg)
Wednesday, 04 January 2012, 22:21 GMT
Reason for closing:  Accepted
Additional comments about closing:  Committed as r31582
Comment by Frank Gevaerts (fg) - Friday, 30 December 2011, 20:40 GMT
I've tested this on fuzev2, e200v1 and gigabeat f with both linux (3.1) and windows (7). As far as I'm concerned this can go in as soon as we've had testing on a non-USB_DETECT_BY_CORE/USB_DETECT_BY_REQUEST software usb device (such as onda, nano2g, or classic)
Comment by Michael Sevakis (MikeS) - Saturday, 31 December 2011, 02:09 GMT
I posted a more extemporaneous version of this on IRC. Noone said anything. It also got rid of USB_HOSTED entirely.
Comment by Michael Sevakis (MikeS) - Saturday, 31 December 2011, 03:03 GMT
Looks good, works good. Tested on a stack device and bridge chip device. :)

Here's a minor tweak to have a consistent manner of setting usb_state (inside main thread loop) and keeping the code that changes usb_num_acks_to_expect nearby for easier viewing.

@frank: I don't see any change worth noting for "promiscuous" detection. It basically the same stuff as a bridge IC.

Also worth noting since I happened to notice when testing: the USB stack still isn't big enough. Screendump on gigabeat F went into an infinite loop, creating lots of .bmp images. Beast usage is 96% after screendump.
Comment by Michael Sevakis (MikeS) - Saturday, 31 December 2011, 09:05 GMT
I did test the clip v1 bootloader USB mode which does not define USB_DETECT_BY_REQUEST and it was ok.
Comment by amaury pouly (pamaury) - Saturday, 31 December 2011, 10:50 GMT
Looks fine to me, I'll test it on the fuze+ but I'm confident it will work since the usb driver is the same as the e200v1.