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
Open

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

Show file tree
Hide file tree
Changes from all commits
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
7 changes: 7 additions & 0 deletions libusb/include/usb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,13 @@ typedef struct {

/* generic descriptor
* used when there is no defined descriptor (e.g. HID descriptor or Report descriptor) */
typedef struct {
uint8_t bLength;
uint8_t bDescriptorType;
uint8_t wData[];
} __attribute__((packed)) usb_generic_desc_t;


typedef struct {
uint8_t bFunctionLength;
uint8_t bDescriptorType;
Expand Down
23 changes: 12 additions & 11 deletions usb/dev.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ static int usb_getConfiguration(usb_dev_t *dev)
uint8_t len = ((struct usb_desc_header *)ptr)->bLength;

if ((len < sizeof(struct usb_desc_header)) || (len > size)) {
log_error("Invalid descriptor size: %u\n", len);
log_error("Invalid descriptor size: %u", len);
break;
}

Expand All @@ -286,7 +286,7 @@ static int usb_getConfiguration(usb_dev_t *dev)
}
}
else {
log_error("Interface descriptor with invalid size\n");
log_error("Interface descriptor with invalid size");
ret = -1;
}
break;
Expand Down Expand Up @@ -314,7 +314,7 @@ static int usb_getConfiguration(usb_dev_t *dev)
}
}
else {
log_error("Endpoint descriptor with invalid size\n");
log_error("Endpoint descriptor with invalid size");
ret = -1;
}
break;
Expand All @@ -327,17 +327,19 @@ static int usb_getConfiguration(usb_dev_t *dev)
dev->desc.bDeviceProtocol = ((usb_interface_association_desc_t *)ptr)->bFunctionProtocol;
}
else {
log_error("Interface assoctiation descriptor with invalid size\n");
log_error("Interface association descriptor with invalid size");
ret = -1;
}
break;

case USB_DESC_CS_INTERFACE:
/* TODO: save Class-Specific Functional Descriptors to be used by device drivers - silently ignored for now */
break;

default:
log_error("Ignoring unkonown descriptor type: 0x%02x\n", ((struct usb_desc_header *)ptr)->bDescriptorType);
/* assume unknown descriptor type is a class-specific or vendor-specific functional descriptor */
if (lastAlternateSetting != 0) {
/* TODO: handle alternate setting maybe */
}
else if (lastIfNum >= 0) {
dev->ifs[lastIfNum].func = (usb_generic_desc_t *)ptr;
}
break;
}

Expand All @@ -346,7 +348,7 @@ static int usb_getConfiguration(usb_dev_t *dev)
}

for (size_t i = 0; i < dev->nifs; i++) {
if ((dev->ifs[i].desc == NULL) || (dev->ifs[i].eps == NULL)) {
if ((dev->ifs[i].desc == NULL) || ((dev->ifs[i].eps == NULL) && (dev->ifs[i].func == NULL))) {
/* Data missing */
ret = -1;
break;
Expand Down Expand Up @@ -677,7 +679,6 @@ int usb_devEnumerate(usb_dev_t *dev)
else if (usb_drvBind(dev, usb_devOnDrvBindCb) != 0) {
log_msg("Fail to match drivers for device\n");
/* TODO: make device orphaned */
return -1;
}

return 0;
Expand Down
1 change: 1 addition & 0 deletions usb/dev.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ typedef struct {
typedef struct {
usb_interface_desc_t *desc;
usb_endpoint_desc_t *eps;
usb_generic_desc_t *func;
void *classDesc;
usb_lenStr_t name;

Expand Down
93 changes: 61 additions & 32 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,8 +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();
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().

_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 @@ -495,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
Loading