Skip to content

max3421: prevent USB host hang at startup#3748

Open
mikeysklar wants to merge 2 commits into
hathach:masterfrom
mikeysklar:fix-max3421-oscok-hang
Open

max3421: prevent USB host hang at startup#3748
mikeysklar wants to merge 2 commits into
hathach:masterfrom
mikeysklar:fix-max3421-oscok-hang

Conversation

@mikeysklar

@mikeysklar mikeysklar commented Jun 30, 2026

Copy link
Copy Markdown

The MAX3421E can fail to report its oscillator ready after a soft reset, so hcd_init() waits on USBIRQ_OSCOK forever and the host hangs at startup. This bounds the wait so it proceeds instead (no-op when OSCOK arrives normally).

Needed on the Adafruit Feather ESP32 V2 + USB Host FeatherWing. Relates to adafruit/circuitpython#10053; complements adafruit/circuitpython#11081.

On some boards the MAX3421E doesn't report its oscillator ready after a
soft reset (CHIPRES), so hcd_init() waits on USBIRQ_OSCOK forever and the
host hangs at startup before any USB device can be used. Bound the wait so
it proceeds after a timeout; it is a no-op where OSCOK arrives normally.

Needed on the Adafruit Feather ESP32 V2 (classic ESP32) + USB Host
FeatherWing, which otherwise hangs on essentially every cold boot.

Relates to adafruit/circuitpython#10053.
@hathach hathach force-pushed the fix-max3421-oscok-hang branch from 3daa572 to 2957c10 Compare July 1, 2026 09:37
@hathach hathach requested a review from Copilot July 1, 2026 09:42
@hathach

hathach commented Jul 1, 2026

Copy link
Copy Markdown
Owner

@claude review

@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished @hathach's task in 3m 4s —— View job


Claude finished @hathach's task

Tasks

  • Gather context (diff, surrounding code, git history of both commits)
  • Understand the request (@claude review)
  • Review code for bugs, safety, style
  • Post review feedback

Review

Change is small and sound. hcd_init() at src/portable/analog/max3421/hcd_max3421.c:504-518 now bounds the OSCOK wait with a 4 ms wall-clock timeout instead of spinning forever, fixing the startup hang on boards where CHIPRES doesn't re-latch OSCOK (no nRST wired). A few notes:

Correctness — looks right

  • Unsigned subtraction (tusb_time_millis_api() - oscok_start_ms) handles millis() wraparound correctly.
  • The loop still exits immediately/normally when OSCOK does arrive, so this is a true no-op on healthy hardware — matches the PR description.
  • I verified this doesn't introduce a new hard dependency risk: tusb_time_millis_api() is already required elsewhere in the host stack (e.g. src/host/usbh.c:407,684,736,747 for NAK/timeout handling), and every in-tree board's family.c provides a strong (non-weak) definition regardless of CFG_TUSB_OS, so this won't silently regress into an infinite loop or fail to link for supported boards. Worth double-checking on the CircuitPython ESP32 port specifically, but since other host timeout logic there must already rely on this same function, it should already be wired up.
  • Timing check is wall-clock based rather than iteration/retry-count based (the first commit's approach), which is more robust across boards with varying SPI transaction overhead per iteration.

Minor, non-blocking suggestions

  • Consider a TU_LOG2/TU_LOG1 when the timeout branch is actually hit (hcd_max3421.c:516), so boards that rely on this fallback are visible in logs rather than silent — useful for anyone debugging a similar issue on new hardware.
  • The 4 ms bound is a literal; a small named constant (e.g. MAX3421_OSCOK_TIMEOUT_MS) would make the datasheet-derived rationale easier to spot at the call site, though inline is consistent with this file's existing style (no other named timeout macros in src/host), so this is purely optional.

No bugs found that would block merging.

@hathach hathach left a comment

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.

thank you, I change the timeout to time based that machtes the datasheet 3ms (+1 in case we are in the last us). tusb_time_millis_api() must be implement, which is actually requirement for host stack on non-rtos.

Image

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR prevents a startup hang in the MAX3421E host controller init path by bounding the wait for the oscillator-ready (OSCOK) IRQ, allowing host initialization to continue even when OSCOK never latches after a soft reset.

Changes:

  • Replace the unbounded OSCOK polling loop in hcd_init() with a millisecond-based timeout (4 ms).
  • Add in-code rationale and datasheet timing reference to explain why proceeding after timeout is acceptable.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +507 to +510
// Wait for the oscillator to stabilize (OSCOK), bounded by a timeout. On boards where the
// MAX3421E is only soft-reset via CHIPRES (no hardware reset on the nRST pin), the OSCOK IRQ
// may never latch and would otherwise hang the host forever. The oscillator is generally
// running regardless, so proceed once the timeout elapses.
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

No memory changes detected across 2404 targets. View Project Dashboard →

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

Size Difference Report

Because TinyUSB code size varies by port and configuration, the metrics below represent the averaged totals across all example builds.

Note: If there is no change, only one value is shown.

Changes >1% in size

No entries.

Changes <1% in size

No entries.

No changes
file .text .rodata .data .bss size % diff
audio_device.c 2890 0 1259 1623 4508 +0.0%
cdc_device.c 1236 16 1092 728 1963 +0.0%
cdc_host.c 6399 487 15 971 7594 +0.0%
dcd_ch32_usbfs.c 1659 0 0 2444 4103 +0.0%
dcd_ch32_usbhs.c 1892 0 0 481 2373 +0.0%
dcd_ci_fs.c 1924 0 0 1290 3214 +0.0%
dcd_ci_hs.c 1756 0 0 1344 2534 +0.0%
dcd_da146xx.c 3067 0 0 144 3211 +0.0%
dcd_dwc2.c 4223 19 0 265 4506 +0.0%
dcd_eptri.c 2273 0 0 259 2532 +0.0%
dcd_ft9xx.c 3284 0 0 172 3456 +0.0%
dcd_khci.c 1952 0 0 1290 3242 +0.0%
dcd_lpc17_40.c 1481 0 0 648 1805 +0.0%
dcd_lpc_ip3511.c 1463 0 0 264 1683 +0.0%
dcd_mm32f327x_otg.c 1477 0 0 1290 2767 +0.0%
dcd_msp430x5xx.c 1801 0 0 176 1977 +0.0%
dcd_musb.c 2595 0 0 179 2773 +0.0%
dcd_nrf5x.c 2939 0 0 292 3231 +0.0%
dcd_nuc120.c 1096 0 0 78 1174 +0.0%
dcd_nuc121.c 1170 0 0 101 1270 +0.0%
dcd_nuc505.c 0 0 1533 157 1690 +0.0%
dcd_rp2040.c 840 0 764 653 2257 +0.0%
dcd_rusb2.c 2918 0 0 156 3074 +0.0%
dcd_samd.c 1036 0 0 266 1302 +0.0%
dcd_samg.c 1322 0 0 72 1394 +0.0%
dcd_stm32_fsdev.c 2543 0 0 291 2834 +0.0%
dfu_device.c 776 28 712 136 912 +0.0%
dfu_rt_device.c 157 0 134 0 157 +0.0%
dwc2_common.c 603 22 0 0 615 +0.0%
ecm_rndis_device.c 1059 0 1 2759 3818 +0.0%
ehci.c 2763 0 0 6274 7783 +0.0%
fsdev_common.c 180 0 0 0 180 +0.0%
hcd_ch32_usbfs.c 2491 0 0 502 2993 +0.0%
hcd_ci_hs.c 181 0 0 0 181 +0.0%
hcd_dwc2.c 5071 25 1 545 5642 +0.0%
hcd_khci.c 2443 0 0 454 2897 +0.0%
hcd_musb.c 3071 0 0 157 3228 +0.0%
hcd_pio_usb.c 262 0 240 0 502 +0.0%
hcd_rp2040.c 1996 17 4 321 2338 +0.0%
hcd_rusb2.c 2923 0 0 245 3168 +0.0%
hcd_samd.c 2220 0 0 324 2544 +0.0%
hcd_stm32_fsdev.c 3248 0 1 420 3670 +0.0%
hid_device.c 1124 44 997 119 1242 +0.0%
hid_host.c 1246 0 0 1270 2516 +0.0%
hub.c 1384 8 8 30 1419 +0.0%
midi2_device.c 2628 52 1175 561 3211 +0.0%
midi2_host.c 1802 0 0 5921 7723 +0.0%
midi_device.c 1149 0 1007 619 1765 +0.0%
midi_host.c 1339 7 7 3538 4880 +0.0%
msc_device.c 2527 108 2293 804 3331 +0.0%
msc_host.c 1633 0 0 395 2028 +0.0%
mtp_device.c 1713 22 743 589 2309 +0.0%
ncm_device.c 1761 28 819 4393 6167 +0.0%
ohci.c 1925 0 0 2503 4428 +0.0%
printer_device.c 828 0 706 560 1387 +0.0%
rp2040_usb.c 386 35 619 11 1051 +0.0%
rusb2_common.c 160 0 16 0 176 +0.0%
tusb.c 455 0 387 3 457 +0.0%
tusb_fifo.c 855 0 486 0 850 +0.0%
typec_stm32.c 1230 8 2 19 1255 +0.0%
usbc.c 500 2 20 166 688 +0.0%
usbd.c 3530 57 90 355 3948 +0.0%
usbh.c 4967 57 82 1165 6238 +0.0%
usbtmc_device.c 2194 24 68 313 2539 +0.0%
vendor_device.c 639 0 534 559 1197 +0.0%
video_device.c 4433 5 1235 480 4905 +0.0%
TOTAL 125088 1071 17050 52144 178805 +0.0%

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

Hardware-in-the-loop (HIL) Test Report

hfp.json

✅ 52 passed · ❌ 0 failed · ⚪ 0 skipped · blank not run

Board cdc_dual_ports cdc_msc dfu cdc_msc_throughput audio_test_freertos dfu_runtime cdc_msc_freertos hid_boot_interface msc_dual_lun hid_generic_inout printer_to_cdc midi_test mtp
stm32l412nucleo ✅ CDC 626k/395k MSC 821k/754k
stm32f746disco ✅ CDC 13.6M/9.3M MSC 15.5M/28.4M
stm32f746disco-DMA ✅ CDC 13.7M/9M MSC 18.6M/30.2M
lpcxpresso43s67 ✅ CDC 12.9M/12.2M MSC 30M/32.6M

tinyusb.json

✅ 339 passed · ❌ 0 failed · ⚪ 11 skipped · blank not run

Board cdc_dual_ports cdc_msc dfu cdc_msc_throughput audio_test_freertos dfu_runtime cdc_msc_freertos hid_boot_interface msc_dual_lun hid_generic_inout printer_to_cdc midi_test mtp host_info_to_device_cdc cdc_msc_hid msc_file_explorer msc_file_explorer_freertos device_info hid_composite_freertos
ek_tm4c123gxl ✅ CDC 761k/961k MSC 891k/998k
espressif_p4_function_ev rd 409KB/s
espressif_p4_function_ev-DMA rd 409KB/s
espressif_s3_devkitm rd 409KB/s
espressif_s3_devkitm-DMA rd 409KB/s
feather_nrf52840_express ✅ CDC 464k/565k MSC 562k/548k
max32666fthr ✅ CDC 7M/13.9M MSC 14.3M/17.5M
metro_m4_express ✅ CDC 476k/371k MSC 402k/473k
mimxrt1015_evk ✅ CDC 14.3M/9.8M MSC 25.1M/19.7M
mimxrt1064_evk ✅ CDC 12.4M/7.1M MSC 23.6M/18.5M rd 1365KB/s rd 1365KB/s
lpcxpresso11u37 ✅ CDC 420k/327k MSC 537k/530k
ra4m1_ek ✅ CDC 430k/444k MSC 422k/518k
raspberry_pi_pico ✅ CDC 513k/508k MSC 521k/514k rd 62KB/s
raspberry_pi_pico_w rd 1106KB/s rd 1022KB/s
raspberry_pi_pico2 rd 1110KB/s rd 1022KB/s
adafruit_fruit_jam ✅ CDC 508k/511k MSC 565k/518k rd 62KB/s rd 62KB/s
stm32f072disco ✅ CDC 307k/343k MSC 332k/471k
stm32f407disco ✅ CDC 731k/542k MSC 678k/939k
stm32f723disco ✅ CDC 525k/796k MSC 476k/759k rd 18078KB/s rd 4096KB/s
stm32f723disco-DMA ✅ CDC 1M/841k MSC 1.1M/1M rd 19418KB/s rd 4096KB/s
stm32h743nucleo ✅ CDC 722k/740k MSC 760k/949k
stm32h743nucleo-DMA ✅ CDC 364k/662k MSC 585k/725k
stm32g0b1nucleo ✅ CDC 745k/528k MSC 878k/552k
stm32l476disco ✅ CDC 385k/496k MSC 416k/629k
stm32u083nucleo ✅ CDC 501k/521k MSC 560k/535k
nanoch32v203-fsdev ✅ CDC 960k/576k MSC 1M/1M
nanoch32v203-usbfs ✅ CDC 958k/574k MSC 1M/1M
ch32v103r_r1_1v0 ✅ CDC 550k/505k MSC 492k/581k
ch582m_evt ✅ CDC 210k/220k MSC 465k/484k

@hathach hathach force-pushed the fix-max3421-oscok-hang branch from 2957c10 to 4202e1e Compare July 1, 2026 10:50
// hangs if the edge is still missed on some board - the clock is running regardless.
const uint32_t oscok_start_ms = tusb_time_millis_api();
while (!(reg_read(rhport, USBIRQ_ADDR, false) & USBIRQ_OSCOK_IRQ) &&
(tusb_time_millis_api() - oscok_start_ms < TIME_TO_EXIT_SUSPEND_MS)) {}

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.

@mikeysklar after checking the datasheet/programming guide, I think the issue is probably we are writing the CHIPRES too fast within ~us that reset signal isn't long enough for the osc is fully stopped to assert the bit. Would you mind trying to comment out the (tusb_time_millis_api() - oscok_start_ms < TIME_TO_EXIT_SUSPEND_MS) the delay in this loop to see if that the above delay work. We will add this anyway, since it prevent actual host hang, any other race condition

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Adafruit 🌸 Reported by an Adafruit member Port MAX3421

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants