host: recover from hub and roothub failures, and fix low-speed devices behind a hub on ESP32-S2/S3 - #3864
host: recover from hub and roothub failures, and fix low-speed devices behind a hub on ESP32-S2/S3#3864P-R-O-C-H-Y wants to merge 4 commits into
Conversation
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of correctness hazards in the new recovery/gating logic (notably HFNUM mask/bit-width consistency and watchdog ordering vs deferred enumeration) that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR improves TinyUSB host robustness by adding recovery paths for root hub / hub failures and adding an ESP32-S2/S3 DWC2 workaround to prevent low-speed devices behind a full-speed hub from taking the root port down permanently.
Changes:
- Add ESP32-S2/S3-specific DWC2 low-speed “one transaction per frame” gating and trigger re-attach/remove when the core disables the host port.
- Add root-port enumeration retry after failed attach, and ensure dev0 is closed when the whole root port is removed (even if dev0 was behind a downstream hub).
- Add hub status endpoint recovery: periodic watchdog re-arm on a quiet bus and abort of a potentially-lost pending status transfer after downstream teardown.
File summaries
| File | Description |
|---|---|
| src/portable/synopsys/dwc2/hcd_dwc2.c | ESP32-S2/S3 low-speed frame-spacing workaround and root-port error recovery events. |
| src/host/usbh.c | Root-port enumeration retry, correct dev0 teardown on root-port removal, and hub-status recovery hooks. |
| src/host/hub.h | Expose hub status watchdog/abort APIs and re-arm interval configuration. |
| src/host/hub.c | Implement hub status watchdog re-arm and “abort pending status xfer” recovery helper. |
Review details
Suppressed comments (1)
src/portable/synopsys/dwc2/hcd_dwc2.c:285
- ls_frame_reserve() updates the global _ls_frame_end before the driver waits for request-queue availability. If the req_queue_avail() busy-wait spans a frame boundary (or is delayed by other activity), _ls_frame_end can claim a frame that the channel does not actually start in, allowing a later low-speed transaction to start “early” and potentially collide in the same frame. To make the gating reliable, the reservation/commit point needs to be tied to when HCCHAR.CHENA is actually set (or split into a ‘wait until free’ + ‘commit’ step) so _ls_frame_end reflects the real start frame.
#if DWC2_LS_ONE_XACT_PER_FRAME
ls_frame_reserve(dwc2, channel);
#endif
while (0 == req_queue_avail(dwc2, channel_is_periodic(channel->hcchar))) {
// blocking wait for request queue available
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 539f757990
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| void hub_status_abort_pending(uint8_t daddr) { | ||
| hub_interface_t* p_hub = get_hub_itf(daddr); | ||
| TU_VERIFY(p_hub->ep_in != 0 && p_hub->bNbrPorts != 0, ); | ||
| tuh_edpt_abort_xfer(daddr, p_hub->ep_in); // skipped internally if the endpoint is not busy |
There was a problem hiding this comment.
When a child is removed, process_new_status() has already re-armed the parent hub's interrupt IN before queuing the remove event, so this call normally targets an active transfer. tuh_edpt_abort_xfer() immediately clears the USBH busy state, but DWC2's abort only requests a channel halt; its HCINT_HALTED path then retries or reschedules the transfer because no aborted/closing state was recorded. The watchdog can consequently submit a second transfer on the same endpoint while the original remains live, corrupting hub status polling. The endpoint must remain unavailable until the HCD confirms the abort, or DWC2 must mark the transfer as aborted before halting it.
Useful? React with 👍 / 👎.
| uint32_t spin = DWC2_LS_FRAME_SPIN_MAX; | ||
| while (spin-- && !ls_frame_free(dwc2, offset)) {} |
There was a problem hiding this comment.
On ESP32-S2/S3, ls_frame_reserve() is reached from channel-interrupt NAK/NYET retry paths through channel_send_in_token(). If another low-speed transaction owns the frame, this empty loop holds the interrupt handler until the hardware frame counter advances or 200,000 iterations expire, potentially once per poll for idle interrupt endpoints; that can delay unrelated interrupts and scheduler activity for nearly a frame. Defer the retry to SOF/task context rather than busy-waiting in the ISR.
AGENTS.md reference: AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
| rearm.odd_frame = 1 - (now & 1); // waiting may have crossed a frame boundary | ||
| channel->hcchar = rearm.value; | ||
| } | ||
| _ls_frame_end = (now + offset + span) & DWC2_LS_FRAME_MASK; |
There was a problem hiding this comment.
ls_frame_reserve() runs both from task-context transfer submission and from channel/SOF interrupt paths, but the free check and _ls_frame_end update are unsynchronized. If an interrupt preempts a task after the loop observes a free frame but before this assignment, both paths reserve from the old value and the resumed task can overwrite the ISR's later reservation; another low-speed transaction can then be enabled in the same frame, recreating the HPRT.PENA failure this gate is intended to prevent. Protect the check-and-update as one critical operation or perform all reservations in one context.
AGENTS.md reference: AGENTS.md:L3-L3
Useful? React with 👍 / 👎.
| // the core runs the packets of a multi-packet transfer back-to-back, which at low speed can spill | ||
| // past the end of the frame it started in | ||
| const uint32_t span = (!is_period && hctsiz.packet_count > 1) ? 1u : 0u; |
There was a problem hiding this comment.
For a non-periodic low-speed OUT transfer, this reserves at most the starting frame plus one extra frame regardless of packet_count. DWC2 programs the entire transfer at once, and a legal large control OUT can contain dozens of 8-byte low-speed packets, occupying more than two 1 ms frames; after _ls_frame_end expires, a periodic transfer from another low-speed device may be enabled while the OUT channel is still transmitting, recreating the same two-transactions-per-frame condition that clears HPRT.PENA. Derive the reservation from the actual packet count/duration or extend it as packets complete.
Useful? React with 👍 / 👎.
Every patch here is headed for TinyUSB master, and the day one lands is the day git apply starts failing and takes the component update with it, even though the clone is in exactly the state we wanted. Detect that with apply --reverse --check and skip. A patch that neither applies nor is present is genuinely stale and still fails, which is the case that needs a human. Reset and clean the clone rather than checking out tracked files: a patch that adds a file leaves it behind otherwise, and untracked files can block the fast-forward that has to happen before patching. Refresh the host patch from the upstream branch it was split into (hathach/tinyusb#3864) so the two do not drift: the retry budget for root port enumeration is now tracked per port rather than in one counter shared by every rhport. The README was still describing three changes against the commit this was first written on, and repeating a claim about how ESP-IDF spaces low-speed transfers that only its log line supports. Co-authored-by: Cursor <cursoragent@cursor.com>
Hardware-in-the-loop (HIL) Test Reporthfp.jsonSkipped by PR selection: no affected boards on this rig. tinyusb-esp.jsonScoped run: 2 board(s) — espressif_p4_function_ev, espressif_s3_devkitm. Boards/tests not listed were not run. ✅ 4 passed · ❌ 4 failed · ⚪ 0 skipped · blank not run
tinyusb.jsonScoped run: 8 board(s) — adafruit_fruit_jam, frdm_k64f, metro_m4_express, mimxrt1064_evk, raspberry_pi_pico, raspberry_pi_pico2, raspberry_pi_pico_w, stm32f723disco. Boards/tests not listed were not run. ✅ 30 passed · ❌ 4 failed · ⚪ 2 skipped · blank not run
|
|
| target | .text | .rodata | .data | .bss | total | % diff |
|---|---|---|---|---|---|---|
| metro_m0_express/hid_controller | 14,968 → 15,124 (+156) | — | — | — | 15,100 → 15,256 (+156) | +1.0% |
| metro_m0_express/device_info | 15,780 → 15,940 (+160) | — | — | — | 15,936 → 16,096 (+160) | +1.0% |
| metro_m0_express/midi_rx | 16,612 → 16,776 (+164) | — | — | — | 16,764 → 16,928 (+164) | +1.0% |
| ch32v203c_r0_1v0/device_info | 19,284 → 19,476 (+192) | — | — | — | 19,712 → 19,904 (+192) | +1.0% |
| metro_m0_express/bare_api | 16,532 → 16,692 (+160) | — | — | — | 16,684 → 16,844 (+160) | +1.0% |
| ch32v203c_r0_1v0/hid_controller | 18,548 → 18,728 (+180) | — | — | — | 18,944 → 19,124 (+180) | +1.0% |
| metro_m0_express/midi2_host | 17,300 → 17,464 (+164) | — | — | — | 17,452 → 17,616 (+164) | +0.9% |
| ch32v203c_r0_1v0/bare_api | 20,172 → 20,364 (+192) | — | — | — | 20,600 → 20,792 (+192) | +0.9% |
| metro_m4_express/hid_controller | 15,340 → 15,484 (+144) | — | — | — | 15,480 → 15,624 (+144) | +0.9% |
| ea4088_quickstart/hid_controller | 14,932 → 15,076 (+144) | — | — | — | 15,496 → 15,640 (+144) | +0.9% |
Review of hathach/tinyusb#3864 found the hub status watchdog could never run for anyone calling tuh_task(), which waits forever: a deaf hub is exactly the case where no event arrives to wake the task. It worked here only because the Arduino host worker polls with a timeout. The watchdog now caps that wait itself, and only while a status endpoint is idle, so a healthy bus keeps sleeping. Also from review: close dev0 before re-posting a root port retry, since hcd_edpt_open() allocates rather than reuses and each retry was leaking an endpoint entry; return the retry budget whenever an attachment ends rather than only when the port changes; run the watchdog after deferred attaches are drained instead of before, where it could collide with the enumeration it is meant to stay out of; reserve the low-speed frame after the request queue wait, which can itself cross a frame boundary; and report an unmount only for a device that reached tuh_mounted(), so a failed enumeration no longer announces the removal of a device that never appeared. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 279bf77bc8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| hub_status_watchdog(); | ||
| } | ||
| // tuh_task() waits forever, which a hub with no status transfer pending would never survive | ||
| wait_ms = tu_min32(wait_ms, hub_status_watchdog_wait_ms()); |
There was a problem hiding this comment.
When a configured hub's status endpoint is idle while another device is enumerating or a control transfer is active, the watchdog call above is intentionally skipped, but this unconditional wait calculation returns 0 after _status_next_ms expires. The RTOS host loops in examples/host/*_freertos/src/main.c immediately call tuh_task() again, so the host task busy-spins until enumeration/control activity finishes, consuming a core and potentially starving lower-priority work. Only cap wait_ms when the same quiet-bus conditions permit hub_status_watchdog() to run.
Useful? React with 👍 / 👎.
| if (failed_daddr == 0) { | ||
| usbh_device_close(rhport, 0); | ||
| } | ||
| hcd_event_device_attach(rhport, false); |
There was a problem hiding this comment.
When a root attach event arrives while enumeration has an active control transfer—for example, the new DWC2 port-disable recovery event—process_remove_event() closes the enumerating device, and usbh_device_close() synchronously completes that control transfer as failed. Its enumeration callback reaches this line and queues a retry even though the original attach handler subsequently starts a new enumeration itself. The queued retry is then processed immediately, cancels that new debounce/enumeration, consumes one retry-budget entry, and restarts it again; distinguish teardown caused by an already-pending attach from a standalone enumeration failure before posting another attach.
Useful? React with 👍 / 👎.
Re-review of hathach/tinyusb#3864 caught two problems in the first round of fixes. Capping the host task's wait outside the watchdog's own gate made the cap zero on every pass while the watchdog was held off, since nothing advances the deadline until it runs, so the task spun for as long as the bus stayed busy instead of waiting for an event. And an attach event tears down whatever was on its port before enumerating, which fails any in-flight enumeration; that failure asked for a roothub retry of the attach being handled right then, which later restarted a healthy enumeration and spent a retry from the budget. Also document the port-disable recovery the patch has carried since the start: a port the core disables on its own stops SOF, and that case used to reach a commented-out assert and be ignored. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Hi @P-R-O-C-H-Y, ba8d20f and ba8d20f(with post fix) looks good. For 29fd95c I'd like to know the know the cause instead of adding a polling loop:
For ce9b3a4 is this limitation from hardware config (GHWCFGn) or an errata ? So we can check it instead of hard coding device family. |
|
Hi @HiFiPhile, thanks for the review.
it's the first case, the transfer jams with no I know because the watchdog on its own did not fix it. While the endpoint It always dies in the same place: the channel teardown in So the polling is covering for a lost transfer rather than fixing it.
#3815 touches abort handling and channel disable, so it may be the same bug. I'll
neither, as far as I can tell. It isn't in the S2/S3 ESP-IDF hits the same limit and works around it with a 1 ms delay in the ISR, So the condition is detectable even though the defect isn't, and it's exactly Let me know what seems to be better and in meantime I will test the #3815 |
|
@HiFiPhile Good news, tested against #3815. The hub commit isn't needed with it. Build: #3815 changes onto master + the other three commits of this PR on top (the hub status abort + watchdog removed). ESP32-S3, FS hub with an LS keyboard (03f0:034a), LS mouse (413c:301a) and an FS flash drive (058f:6387) behind it. 15 connect/disconnect events across the three devices, including removals of the flash drive. The teardown that used to leave the hub deaf and one removal after three minutes of an idle bus, so nothing was keeping the host task awake. Every edge was reported and every device kept running. So the status transfer was jammed rather than never re-armed, and the channel teardown rework in #3815 keeps it alive. Happy to drop hub: recover a hub whose status endpoint stopped being re-armed and rebase the rest onto #3815 changes. Is there any plan to merge that soon so it is part of the master? |
That's nice, I'm still fighting with a unplug/replug race. Maybe Thach is off for Vietnam's National Day. |
|
@P-R-O-C-H-Y I'm done with #3815 |
|
@HiFiPhile Do you want me to retest with final version of your PR? Once merged, I will rebase my PR and remove the commits no longer needed. |
Yes please |
@HiFiPhile Retested your latest #3815 (e461c6f) plus the other three commits of this PR, hub watchdog still out. All good, everything works as expected. So I’m dropping hub: recover a hub whose status endpoint stopped being re-armed. I’ll rebase the rest onto #3815 once it lands. The dwc2 commit hooks through your |
|
@P-R-O-C-H-Y if your schedule is tight @me-no-dev or @tore-espressif can review and approve #3815. |
A remove for hub_addr = 0, hub_port = 0 covers everything below the root port, but process_remove_event() only closed dev0 when the event named the exact bus address it was enumerating on. Mid-enumeration behind a downstream hub then left enumerating_daddr at 0 and deferred every later attach. Co-authored-by: Cursor <cursoragent@cursor.com>
enum_full_complete() already closes the failed device (from hathach#3815). Nothing retries the root port itself: the host then sits idle until reboot. Re-post the attach up to USBH_ENUM_ROOT_RETRY_MAX times while the port still reports a connection. The budget is per port and given back when the attachment ends, including when an attach event tears down an in-flight enumeration. tuh_umount_cb() is only called for a device that reached tuh_mounted(). Co-authored-by: Cursor <cursoragent@cursor.com>
Those full-speed cores cannot run two preamble transactions in the same 1 ms frame. A second one clears HPRT.PENA and the bus stays down. ESP-IDF handles the same limit with a 1 ms delay (espressif/esp-idf#15683). Reserve one frame per low-speed channel start, including NAK retries and the slave-mode OUT path. Periodic INs from SOF are deferred a frame rather than spun on. A port the core disables on its own now posts attach or remove instead of being silently ignored. Hooks through channel_enable() from hathach#3815; the reservation sits outside the interrupt-masked window. Co-authored-by: Cursor <cursoragent@cursor.com>
An attach already tears down the existing root-port tree, so posting remove is unnecessary. Skip when conn_detected is already set so the same IRQ does not queue attach twice. Co-authored-by: Cursor <cursoragent@cursor.com>
|
PR was rebased, removed no longer needed commits as #3815 is merged + updated the PR description. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9db4010ed3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // where this channel would start: periodic channels are armed for the next frame (odd_frame), | ||
| // non-periodic ones go out in the current frame. | ||
| TU_ATTR_ALWAYS_INLINE static inline bool ls_frame_free(const dwc2_regs_t* dwc2, uint32_t offset) { | ||
| return ls_frame_diff(ls_frame_now(dwc2) + offset, _ls_frame_end) > 0; |
There was a problem hiding this comment.
On ESP32-S2/S3, when no preamble transaction updates _ls_frame_end for more than half of the 14-bit frame-counter cycle (8.192 seconds), this signed modular comparison interprets the expired reservation as still being in the future. A later periodic low-speed transfer routed through ls_edpt_deferred() can therefore be deferred on every SOF until the counter wraps, delaying the transfer by up to another 8.192 seconds; direct submissions instead burn the full spin limit before proceeding. Since a reservation spans at most a few frames, track whether it is active or explicitly expire stale values rather than applying a half-range ordering indefinitely.
Useful? React with 👍 / 👎.
|
Retested on S3 and P4 with the same setup, everything works as expected. |
|
@P-R-O-C-H-Y I've tried another approach in https://github.com/hathach/tinyusb/tree/pr3864-alt d099929:
|
@HiFiPhile So you have turned the gate for STM? Currently in the PR it's compiled only for ESP32-S2 and S3. So it shouldn't affect any other chips. I have tested your alt branch with all the commits on ESP32-S3 (FS hub, LS keyboard, LS mouse, FS MSC). Quiet plug/unplug of each device is fine. The stress case is not. If I unplug the MSC, plug it back, and immediately use the mouse and keyboard, the MSC never mounts / never prints anything. In one run, plugging the MSC while typing and clicking left the mouse dead after a few seconds. So the SOF one-packet scheduler looks OK on an idle bus, but concurrent MSC enum/bulk plus live LS HID still loses the drive, and can silence the mouse. Happy to retest if you have a new revision. |
|
@HiFiPhile To be sure I have retested the same stress test on my PR changes, and there its fine. |
|
For tomorrow I have a local-only follow-up on top of this PR, not pushed yet:
Plan is to capture on S3 for idle kbd+mouse, typing/moving, and the same during MSC enum/bulk. Goal is ≤1 PRE per SOF. If I can confirm on the analyzer, I will push it; if not I will drop it and stay on 9db4010. |
That's nice.
It's only a temporary test to see how it works.
So >= 2 PRE doesn't always crash the controller ? |
Analyzer follow-up: Two PRE in the same 1 ms frame, ~42–45 µs apart, is what we see when The PR gate (S2/S3 only, one LS channel start per frame) is enough for the stress case I retested. I also tried a local-only follow-up to force ≤1 PRE (one packet per activation, then defer every new LS start to SOF). That did get long control stages down to one PRE per SOF, but it did not eliminate the remaining 2-PRE frames (those are two independent starts in the same frame, e.g. HID poll next to a control stage), and the SOF-side defer deadlocked the host (hub SETUP timeout, HID/MSC stuck). I am dropping that follow-up and staying on this PR as-is. The |
Drop the hub watchdog now that #3815 is on master, and match the four commits currently on hathach/tinyusb#3864. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@HiFiPhile @hathach Anything I can do to move with this PR? Thanks |
Ah I thought you still have works pending. Please test b09f21f, 47a432a, 6f2824e on current tree, b09f21f is for this PR's USBH enumeration handling, others 2s are post fixup for #3815 |
@HiFiPhile I have tested your alt branch already, didn't pass the stress test. Is there anything new in that branch? So it's worth retesting again? I don't have anything to add, I have commented that all my new approaches I have tried didn't worked out, there were always multiple PREs and it was working worse than commits in this PR. The PR as is works perfectly for me and ESP32S3. |
|
@P-R-O-C-H-Y As I said earlier (also detected by Chatgpt) The call chain is: A simple trigger is a control IN NAK in the frame already reserved by that transfer. The immediate retry checks the same frame, finds it occupied, and spins inside the ISR until the frame becomes free or the spin limit expires. |
…ability + include fix (#393) * feat(tinyusb): Apply local TinyUSB patches when updating components USB host on ESP32-S2/S3 cannot keep two low-speed devices alive behind a full-speed hub: the DWC2 core disables the root port when two preamble transactions land in one frame, and neither the root port nor a hub's status endpoint has anything that retries afterwards. The fixes belong upstream, but the core needs them now, so carry them as a patch that is applied on every component update. patches/tinyusb/*.diff is applied after the clone or pull, which also means the clone is build output from here on: it is reset before pulling so the previous run's patches cannot block a fast-forward. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(tinyusb): Stop including tusb_option.h from tusb_config.h TinyUSB includes tusb_config.h from tusb_option.h, after the OPT_* values it depends on are defined. Pulling tusb_option.h in from the other side makes the config apply its own defaults first, so every value the SDK then sets is a redefinition: CFG_TUD_ENABLED, CFG_TUSB_OS, CFG_TUD_MAX_SPEED and friends. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(tinyusb): Skip patches whose fix already landed upstream Every patch here is headed for TinyUSB master, and the day one lands is the day git apply starts failing and takes the component update with it, even though the clone is in exactly the state we wanted. Detect that with apply --reverse --check and skip. A patch that neither applies nor is present is genuinely stale and still fails, which is the case that needs a human. Reset and clean the clone rather than checking out tracked files: a patch that adds a file leaves it behind otherwise, and untracked files can block the fast-forward that has to happen before patching. Refresh the host patch from the upstream branch it was split into (hathach/tinyusb#3864) so the two do not drift: the retry budget for root port enumeration is now tracked per port rather than in one counter shared by every rhport. The README was still describing three changes against the commit this was first written on, and repeating a claim about how ESP-IDF spaces low-speed transfers that only its log line supports. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(tinyusb): Fold upstream review fixes into the host patch Review of hathach/tinyusb#3864 found the hub status watchdog could never run for anyone calling tuh_task(), which waits forever: a deaf hub is exactly the case where no event arrives to wake the task. It worked here only because the Arduino host worker polls with a timeout. The watchdog now caps that wait itself, and only while a status endpoint is idle, so a healthy bus keeps sleeping. Also from review: close dev0 before re-posting a root port retry, since hcd_edpt_open() allocates rather than reuses and each retry was leaking an endpoint entry; return the retry budget whenever an attachment ends rather than only when the port changes; run the watchdog after deferred attaches are drained instead of before, where it could collide with the enumeration it is meant to stay out of; reserve the low-speed frame after the request queue wait, which can itself cross a frame boundary; and report an unmount only for a device that reached tuh_mounted(), so a failed enumeration no longer announces the removal of a device that never appeared. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(tinyusb): Take the second round of upstream review fixes Re-review of hathach/tinyusb#3864 caught two problems in the first round of fixes. Capping the host task's wait outside the watchdog's own gate made the cap zero on every pass while the watchdog was held off, since nothing advances the deadline until it runs, so the task spun for as long as the bus stayed busy instead of waiting for an event. And an attach event tears down whatever was on its port before enumerating, which fails any in-flight enumeration; that failure asked for a roothub retry of the attach being handled right then, which later restarted a healthy enumeration and spent a retry from the budget. Also document the port-disable recovery the patch has carried since the start: a port the core disables on its own stops SOF, and that case used to reach a commented-out assert and be ignored. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(tinyusb): Correct why the slave-mode OUT path carries low-speed The patch and its README both said the slave-mode branch is where these transfers go because esp32s2/s3 have no host DMA. The core does have internal DMA; the branch is taken because CFG_TUH_DWC2_DMA_ENABLE defaults to 0. The gate covers the DMA path too, so only the reasoning was wrong. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: Update readme with tinyusb hub fix #3815 * fix(tinyusb): Refresh host patch to TinyUSB #3864 9db4010ed Drop the hub watchdog now that #3815 is on master, and match the four commits currently on hathach/tinyusb#3864. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
Depends on #3815, now merged. That PR keeps the hub status transfer alive, so the hub watchdog commit is gone.
Three leftover fixes from a full-speed DWC2 host dying when an LS mouse and LS keyboard share one FS hub.
host: close dev0 when the whole roothub port goes away
A remove for
hub_addr = 0, hub_port = 0covers everything below the root port, butprocess_remove_event()only closed dev0 when the event named the exact bus address it was enumerating on. Mid-enumeration behind a downstream hub then leftenumerating_daddrat 0 and deferred every later attach.host: retry enumeration of the roothub port after a failed attach
enum_full_complete()already closes the failed device (#3815). Nothing retries the root port: the host sits idle until reboot. Re-post the attach up toUSBH_ENUM_ROOT_RETRY_MAXtimes while the port still reports a connection. Budget is per port and given back when the attachment ends, including when an attach event tears down an in-flight enumeration.tuh_umount_cb()only runs for a device that reachedtuh_mounted().dwc2: space low-speed transactions one per frame on esp32s2/s3
ESP32-S2/S3 cannot run two preamble transactions in the same 1 ms frame. A second one clears
HPRT.PENAand the bus stays down. Not an errata and not aGHWCFGbit — ESP-IDF hits the same limit (IDF-12986 / espressif/esp-idf#15683). Gated at runtime byls_needs_preamble()(FS port + LS device, no split), compiled in only for those MCUs.Reservation hooks through
channel_enable()from #3815, after the request-queue wait and outside the interrupt-masked window. Periodic INs from SOF defer a frame rather than spin. A port the core disables on its own now posts attach or remove; that used to hit a commented-outTU_ASSERT(false, )and stay dead.Testing
03f0:034a413c:301a058f:6387ESP32-S3 and ESP32-P4, all three on one hub. Cold boot and unplug/replug of each, including the drive, on #3815 plus these three commits. P4 compiles the frame gate out and exercises the two generic commits on HS.