Skip to content

usb: fix TT bugs and class-specific functional descriptor discovery #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
adamgreloch wants to merge 4 commits into
base: master
Choose a base branch
from
+81 −43
Open

usb: fix TT bugs and class-specific functional descriptor discovery #38

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
65075cb
usb/hub: fix deadlock on successful TT enumeration
adamgreloch Sep 10, 2025
c511e39
usb/hub: make TT workaround opt-in
adamgreloch Sep 10, 2025
3896e32
usb/dev: handle class-specific functional descriptors
adamgreloch Sep 10, 2025
ee3e540
usb/dev: ignore failure to bind the driver
adamgreloch Sep 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 61 additions & 41 deletions usb/hub.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@
#undef USB_LOG_TAG
#define USB_LOG_TAG "usbhub"

#ifndef USB_HUB_TT_WORKAROUND
#define USB_HUB_TT_WORKAROUND 0
#endif


typedef struct _hub_event {
struct _hub_event *next, *prev;
Expand All @@ -59,6 +63,9 @@ struct {
} hub_common;


static void hub_portstatus(usb_dev_t *hub, int port);


static int hub_getDesc(usb_dev_t *hub, char *buf, size_t len)
{
usb_setup_packet_t setup = (usb_setup_packet_t) {
Expand Down Expand Up @@ -177,13 +184,6 @@ static int hub_interruptInit(usb_dev_t *hub)
}


static int hub_isTT(usb_dev_t *dev)
{
/* TODO support MTTs */
return dev->desc.bDeviceClass == USB_CLASS_HUB && dev->desc.bDeviceProtocol == USB_HUB_PROTO_SINGLE_TT;
}


static int hub_requestStatus(usb_dev_t *hub)
{
return usb_transferSubmit(hub->statusTransfer, hub->irqPipe, NULL);
Expand Down Expand Up @@ -262,6 +262,14 @@ static int hub_portDebounce(usb_dev_t *hub, int port)
}


#if USB_HUB_TT_WORKAROUND
static int hub_isTT(usb_dev_t *dev)
{
/* TODO support MTTs */
return dev->desc.bDeviceClass == USB_CLASS_HUB && dev->desc.bDeviceProtocol == USB_HUB_PROTO_SINGLE_TT;
}


static void hub_ttAdd(usb_dev_t *hub)
{
mutexLock(hub_common.lock);
Expand All @@ -278,12 +286,45 @@ static void hub_ttRemove(usb_dev_t *hub)
}


static void _hub_ttStatus(void)
{
usb_dev_t *hub;
int i;

hub = hub_common.tts;
if (hub == NULL) {
return;
}

do {
for (i = 0; i < hub->nports; i++) {
mutexUnlock(hub_common.lock);
/*
* hub_portstatus may lead to usb_devEnumerate call which may call hub_conf that
* tries to obtain this lock via hub_ttAdd:
*
* hub_portstatus -> hub_connectstatus -> hub_devConnected -> usb_devEnumerate -> hub_conf -> hub_ttAdd
*
* FIXME? It may be better to separate the enumeration
* from the connect status polling logic, but as this polling loop is a
* part of L2 TT workaround, the problem may be easier to solve itself in the root
*/
hub_portstatus(hub, i + 1);
mutexLock(hub_common.lock);
}
} while ((hub = hub->next) != hub_common.tts);
}
#endif


static void hub_devDisconnect(usb_dev_t *dev, bool silent)
{
usb_devDisconnected(dev, silent);
#if USB_HUB_TT_WORKAROUND
if (hub_isTT(dev)) {
hub_ttRemove(dev);
}
#endif
usb_devDisconnected(dev, silent);
}


Expand Down Expand Up @@ -399,24 +440,6 @@ static uint32_t hub_getStatus(usb_dev_t *hub)
}


static void hub_ttStatus(void)
{
usb_dev_t *hub;
int i;

hub = hub_common.tts;
if (hub == NULL) {
return;
}

do {
for (i = 0; i < hub->nports; i++) {
hub_portstatus(hub, i + 1);
}
} while ((hub = hub->next) != hub_common.tts);
}


static void hub_thread(void *args)
{
hub_event_t *ev;
Expand All @@ -426,17 +449,12 @@ static void hub_thread(void *args)
for (;;) {
mutexLock(hub_common.lock);
while (hub_common.events == NULL) {
#if USB_HUB_TT_WORKAROUND
condWait(hub_common.cond, hub_common.lock, HUB_TT_POLL_DELAY_MS);

/*
* hub_ttStatus may lead to usb_devEnumerate call which may call hub_conf that
* tries to obtain this lock. FIXME? It may be better to separate the enumeration
* from the connect status polling logic, but as this polling loop is a
* part of L2 TT workaround, the problem may be easier to solve itself in the root
*/
mutexUnlock(hub_common.lock);
hub_ttStatus();
Copy link
Contributor

@ziemleszcz ziemleszcz Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another approach that avoids the deadlock is to iterate over all hubs and inject an artificial event for each, rather than invoking hub_portstatus().

mutexLock(hub_common.lock);
_hub_ttStatus();
#else
condWait(hub_common.cond, hub_common.lock, 0);
#endif
}
ev = hub_common.events;
LIST_REMOVE(&hub_common.events, ev);
Expand Down Expand Up @@ -504,14 +522,16 @@ int hub_conf(usb_dev_t *hub)
if (hub_interruptInit(hub) != 0)
return -EINVAL;

#if USB_HUB_TT_WORKAROUND
if (hub_isTT(hub)) {
// FIXME Interrupt pipe notifications from tier 2 TTs never come, which is
// either a quirk in currently tested hw or an issue in the hcd driver

// In addition to request an interrupt request, actively poll the status as well
// for as a workaround
/*
* FIXME Interrupt pipe notifications from tier 2 TTs never come on ia32,
* which is either a quirk in currently tested hw or an issue in the hcd driver.
* As a workaround, try to receive interrupt requests AND actively poll the status.
*/
hub_ttAdd(hub);
}
#endif

return hub_requestStatus(hub);
}
Expand Down