Skip to content

Host: Add support for multi-level usb hubs#1480

Merged
hathach merged 6 commits intohathach:masterfrom
Ryzee119:multihub_rebase
Jun 16, 2022
Merged

Host: Add support for multi-level usb hubs#1480
hathach merged 6 commits intohathach:masterfrom
Ryzee119:multihub_rebase

Conversation

@Ryzee119
Copy link
Copy Markdown
Contributor

@Ryzee119 Ryzee119 commented May 29, 2022

Describe the PR

  • Add support for multiple levels of USB hubs in the usb host stack. For your consideration Hathach 😄

To get this working reliably I had to relax the assert on failed port_status control transfer. Currently with multiple hubs it may perform a control xfer if the ctrl buffer is busy.

Additional context

  1. Developed on imxrt (Teensy41) with EHCI.
  2. Tested with 3 USB hubs levels, combination of low speed and full speed devices. (HID devices) Not tested with Bulk or Iso but this is not expected to be different from upstream.
  3. Quite susceptible to 5V rail supply issues. So an externally powered usb hub is recommended.

Known issues

  1. If you unplug a downstream hub, it will hit this assert which makes sense and can easily be fixed by relaxing this assert to a TU_VERIFY, but not included in this PR. I think this assert is quite strict as is, but will leave for feedback.
    TU_ASSERT(result == XFER_RESULT_SUCCESS);

May help to review on a per commit basis.

Below is a working log of 2 daisy chained usb hubs, with 3 usb drives connected. One drive in the first hub, 2 in the 2nd hub.
After enumeration of all 5 devices, I unplug the root hub so you can see all downstream devices disconnected successfully.
multihub-log.txt

Thanks for this great USB stack 🚀

@Ryzee119 Ryzee119 changed the title Multihub rebase Host: Add support for multi-level usb hubs May 29, 2022
@Ryzee119
Copy link
Copy Markdown
Contributor Author

Ryzee119 commented Jun 9, 2022

I can hit issues if we disconnect a device from a hub when we are enumerating another device, the hub status control transfer will begin, then we'll assert in enumeration due to a busy control buffer.

I think there's a few conflicts with hub, enumerating and control buffers to be resolved still for this to be very resilient.

Copy link
Copy Markdown
Owner

@hathach hathach left a comment

Choose a reason for hiding this comment

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

Superb ! thank you very much for your effort and patience. I have tested with mcb1800 (same chipidea controller as imxrt) and it works like a charm. The recursive call to process unplugged hub device is working but not a great approach in long term IMHO. I make an commit to add FIXME to this and also make an issue for reminder as well. Thanks again for your work.

PS: finally I could hide the fact that I am lazy to support this :)
PSS: #1511 is created

if (hub_port_get_status(dev_addr, port, &p_hub->port_status, connection_get_status_complete, 0) == false)
{
//Hub status control transfer failed, retry
hub_edpt_status_xfer(dev_addr);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

indeed, for compatibility, usbh only carry 1 control transfer at a time since unlike ehci most custom host is rather limited. Currently there is no control queue and when the control is busy, usbh simply return false for try-again.

// If the device itself is a usb hub, unplug downstream devices.
if (dev_addr > CFG_TUH_DEVICE_MAX)
{
process_device_unplugged(rhport, dev_addr, 0);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

an recursive call in limited MCU can cause stack overflow. Though I am ok with this initial support. In long run, we should find an way to un-roll the recursive chain. After all, the device loop is rather small (4-10) only.

@hathach hathach merged commit d7b579a into hathach:master Jun 16, 2022
@Ryzee119 Ryzee119 deleted the multihub_rebase branch September 20, 2022 23:09
7FM pushed a commit to 7FM/tinyusb that referenced this pull request Aug 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants