diff --git a/.github/workflows/usbhost.yml b/.github/workflows/usbhost.yml index 2edef199..d8176bbd 100644 --- a/.github/workflows/usbhost.yml +++ b/.github/workflows/usbhost.yml @@ -94,6 +94,39 @@ jobs: fi echo "planted fault $fault caught:"; grep FAIL /tmp/planted.log | head -3 done + - name: TinyUSB's root-port enumeration (the GIGA's USB-A port), and its planted fault + # TinyUSB at the commit MicroPython's lib/tinyusb points at, for the + # pinned MicroPython. With patches/lib-tinyusb/ applied, a drive that + # misses its first requests has to mount; with only 0001, the patch + # before the root-port retry, the test has to fail. + run: | + MP=$(python3 ../../scripts/pins.py micropython) + git init -q /tmp/mp + git -C /tmp/mp fetch -q --depth 1 --filter=blob:none https://github.com/micropython/micropython "$MP" + TUSB=$(git -C /tmp/mp ls-tree FETCH_HEAD lib/tinyusb | awk '{print $3}') + git init -q /tmp/tinyusb + git -C /tmp/tinyusb fetch -q --depth 1 https://github.com/hathach/tinyusb "$TUSB" + git -C /tmp/tinyusb checkout -q FETCH_HEAD + build() { + cc -std=c11 -Wall -Wextra -Werror -O1 -g -fsanitize=address,undefined \ + -Itests/tinyusb_host -I/tmp/tinyusb/src -o "$1" tests/test_tuh_enum_retry.c \ + /tmp/tinyusb/src/tusb.c /tmp/tinyusb/src/common/tusb_fifo.c \ + /tmp/tinyusb/src/host/usbh.c /tmp/tinyusb/src/host/hub.c + } + PATCHES=$(realpath ../../patches/lib-tinyusb) + git -C /tmp/tinyusb apply "$PATCHES"/0001-*.patch + build /tmp/test_tuh_enum_planted + if /tmp/test_tuh_enum_planted > /tmp/planted.log 2>&1; then + cat /tmp/planted.log + echo "a root port that gives up after one attempt was NOT caught"; exit 1 + fi + echo "planted fault caught:"; grep FAIL /tmp/planted.log | head -3 + for patch in "$PATCHES"/*.patch; do + case "$patch" in */0001-*) continue ;; esac + git -C /tmp/tinyusb apply "$patch" + done + build /tmp/test_tuh_enum_retry + /tmp/test_tuh_enum_retry - name: The sound card pump block (the meter reads what plays), and its planted fault run: | cc -std=c11 -Wall -Wextra -Werror -O1 -g -fsanitize=address,undefined -Isrc \ diff --git a/modules/usbhost/examples/usb_drive_mount.py b/modules/usbhost/examples/usb_drive_mount.py index 077bdb88..6aa84582 100644 --- a/modules/usbhost/examples/usb_drive_mount.py +++ b/modules/usbhost/examples/usb_drive_mount.py @@ -25,7 +25,9 @@ Tested on: an ESP32-P4-WIFI6-DEV-KIT on MicroPython v1.30.0-preview.160.gb70ae1aca6 (built 2026-10-21), -hosting a PNY 8 GB FAT flash drive behind its onboard hub. +hosting a PNY 8 GB FAT flash drive behind its onboard hub; an Arduino GIGA +R1 WiFi with the GIGA Display Shield on the same MicroPython, with a PNY 8 GB +FAT flash drive in its USB-A port. """ import os diff --git a/modules/usbhost/examples/usbhost_simpletest.py b/modules/usbhost/examples/usbhost_simpletest.py index 2fbc3b59..d8dd56e9 100644 --- a/modules/usbhost/examples/usbhost_simpletest.py +++ b/modules/usbhost/examples/usbhost_simpletest.py @@ -17,8 +17,10 @@ mpremote run usbhost_simpletest.py -Tested on: Waveshare ESP32-P4-WIFI6-DEV-KIT, MicroPython -v1.30.0-preview.160.gb70ae1aca6, with nothing plugged in: the host started, +Tested on: Arduino GIGA R1 WiFi with the GIGA Display Shield, MicroPython +v1.30.0-preview.160.gb70ae1aca6, with a PNY USB flash drive in the USB-A +port: it printed `1 USB device(s) found` and `154b:009f`. Waveshare +ESP32-P4-WIFI6-DEV-KIT, the same MicroPython, with nothing plugged in: it printed `0 USB device(s) found` and stopped cleanly. """ diff --git a/modules/usbhost/tests/test_tuh_enum_retry.c b/modules/usbhost/tests/test_tuh_enum_retry.c new file mode 100644 index 00000000..1e5d6dda --- /dev/null +++ b/modules/usbhost/tests/test_tuh_enum_retry.c @@ -0,0 +1,311 @@ +// SPDX-License-Identifier: MIT +// +// Host-side tests for TinyUSB's enumeration on a root port, as the Arduino +// GIGA R1's USB-A port runs it: TinyUSB's own usbh.c and hub.c, with +// patches/lib-tinyusb/ applied, over a fake controller and a fake device. +// +// A USB flash drive plugged straight into the port doesn't answer its first +// requests: it is still starting up when the host resets it and asks for its +// device descriptor. TinyUSB gave up after that one attempt and never tried +// again, so the drive stayed connected and was never listed. With +// patches/lib-tinyusb/0002 the root port is reset and enumerated again, up to +// three more times. +// +// Built against TinyUSB without that patch, this test has to fail (CI checks +// that it does). +#include +#include + +#include "tusb.h" +#include "host/hcd.h" + +static int failures = 0; + +#define CHECK(cond) do { \ + if (!(cond)) { \ + printf("FAIL %s:%d: %s\n", __FILE__, __LINE__, #cond); \ + failures++; \ + } \ +} while (0) + +// -- the clock + +static uint32_t now_ms; + +uint32_t tusb_time_millis_api(void) { + return now_ms; +} + +// -- the fake device + +static const uint8_t desc_device[18] = { + 18, TUSB_DESC_DEVICE, 0x00, 0x02, 0, 0, 0, 64, + 0x81, 0x07, 0x81, 0x55, // 0781:5581, a SanDisk drive + 0x00, 0x01, 0, 0, 0, 1, +}; + +static const uint8_t desc_config[32] = { + 9, TUSB_DESC_CONFIGURATION, 32, 0, 1, 1, 0, 0x80, 50, + 9, TUSB_DESC_INTERFACE, 0, 0, 2, TUSB_CLASS_MSC, 6, 0x50, 0, + 7, TUSB_DESC_ENDPOINT, 0x81, TUSB_XFER_BULK, 64, 0, 0, + 7, TUSB_DESC_ENDPOINT, 0x02, TUSB_XFER_BULK, 64, 0, 0, +}; + +static const uint8_t desc_langid[4] = { 4, TUSB_DESC_STRING, 0x09, 0x04 }; + +static struct { + bool plugged; + uint32_t powered_at; // when it was plugged in + uint32_t ready_after_ms; // how long it ignores requests after that; 0xFFFFFFFF: always + uint8_t addr; + const uint8_t *reply; // the data stage's + uint16_t reply_len; +} dev; + +// What the host did. +static uint32_t resets, setups, setups_ignored, dev0_opens, dev0_closes; +static uint8_t mounted; + +static void plug(uint32_t ready_after_ms) { + memset(&dev, 0, sizeof(dev)); + dev.plugged = true; + dev.powered_at = now_ms; + dev.ready_after_ms = ready_after_ms; + hcd_event_device_attach(0, false); +} + +static void unplug(void) { + dev.plugged = false; + hcd_event_device_remove(0, false); +} + +static bool dev_ready(void) { + return dev.ready_after_ms != 0xFFFFFFFF && now_ms - dev.powered_at >= dev.ready_after_ms; +} + +// -- the fake controller: every transfer completes on the next task run + +bool hcd_init(uint8_t rhport, const tusb_rhport_init_t *rh_init) { + (void)rhport; + (void)rh_init; + return true; +} + +void hcd_int_handler(uint8_t rhport, bool in_isr) { + (void)rhport; + (void)in_isr; +} + +void hcd_int_enable(uint8_t rhport) { + (void)rhport; +} + +void hcd_int_disable(uint8_t rhport) { + (void)rhport; +} + +uint32_t hcd_frame_number(uint8_t rhport) { + (void)rhport; + return now_ms; +} + +bool hcd_port_connect_status(uint8_t rhport) { + (void)rhport; + return dev.plugged; +} + +void hcd_port_reset(uint8_t rhport) { + (void)rhport; + resets++; + dev.addr = 0; +} + +void hcd_port_reset_end(uint8_t rhport) { + (void)rhport; +} + +tusb_speed_t hcd_port_speed_get(uint8_t rhport) { + (void)rhport; + return TUSB_SPEED_FULL; +} + +void hcd_device_close(uint8_t rhport, uint8_t dev_addr) { + (void)rhport; + if (dev_addr == 0) { + dev0_closes++; + } +} + +bool hcd_edpt_open(uint8_t rhport, uint8_t daddr, tusb_desc_endpoint_t const *ep_desc) { + (void)rhport; + if (daddr == 0 && ep_desc->bEndpointAddress == 0) { + dev0_opens++; + } + return true; +} + +bool hcd_edpt_close(uint8_t rhport, uint8_t daddr, uint8_t ep_addr) { + (void)rhport; + (void)daddr; + (void)ep_addr; + return true; +} + +bool hcd_setup_send(uint8_t rhport, uint8_t daddr, uint8_t const setup_packet[8]) { + (void)rhport; + setups++; + if (!dev.plugged || !dev_ready() || daddr != dev.addr) { + // No handshake: the controller gives up after its own retries. + setups_ignored++; + hcd_event_xfer_complete(daddr, 0x00, 0, XFER_RESULT_FAILED, false); + return true; + } + const tusb_control_request_t *req = (const tusb_control_request_t *)setup_packet; + xfer_result_t result = XFER_RESULT_SUCCESS; + dev.reply = NULL; + dev.reply_len = 0; + if (req->bRequest == TUSB_REQ_GET_DESCRIPTOR) { + switch (req->wValue >> 8) { + case TUSB_DESC_DEVICE: + dev.reply = desc_device; + dev.reply_len = sizeof(desc_device); + break; + case TUSB_DESC_CONFIGURATION: + dev.reply = desc_config; + dev.reply_len = sizeof(desc_config); + break; + case TUSB_DESC_STRING: + dev.reply = desc_langid; + dev.reply_len = sizeof(desc_langid); + break; + default: + result = XFER_RESULT_STALLED; + break; + } + if (dev.reply_len > req->wLength) { + dev.reply_len = req->wLength; + } + } else if (req->bRequest == TUSB_REQ_SET_ADDRESS) { + dev.addr = (uint8_t)req->wValue; + } else if (req->bRequest != TUSB_REQ_SET_CONFIGURATION) { + result = XFER_RESULT_STALLED; + } + hcd_event_xfer_complete(daddr, 0x00, 8, result, false); + return true; +} + +bool hcd_edpt_xfer(uint8_t rhport, uint8_t daddr, uint8_t ep_addr, uint8_t *buffer, uint16_t buflen) { + (void)rhport; + uint16_t n = 0; + if (buffer != NULL && buflen > 0 && (ep_addr & TUSB_DIR_IN_MASK) && dev.reply != NULL) { + n = dev.reply_len < buflen ? dev.reply_len : buflen; + memcpy(buffer, dev.reply, n); + } + hcd_event_xfer_complete(daddr, ep_addr, n, dev.plugged ? XFER_RESULT_SUCCESS : XFER_RESULT_FAILED, false); + return true; +} + +bool hcd_edpt_abort_xfer(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { + (void)rhport; + (void)dev_addr; + (void)ep_addr; + return true; +} + +bool hcd_edpt_clear_stall(uint8_t rhport, uint8_t dev_addr, uint8_t ep_addr) { + (void)rhport; + (void)dev_addr; + (void)ep_addr; + return true; +} + +// -- the application's callbacks + +void tuh_mount_cb(uint8_t daddr) { + mounted = daddr; +} + +void tuh_umount_cb(uint8_t daddr) { + if (mounted == daddr) { + mounted = 0; + } +} + +// -- the tests + +static void run_for(uint32_t ms) { + for (uint32_t end = now_ms + ms; now_ms < end; now_ms++) { + for (int i = 0; i < 4; i++) { + tuh_task_ext(0, false); + } + } +} + +static void reset_counts(void) { + resets = setups = setups_ignored = dev0_opens = dev0_closes = 0; +} + +// A drive that ignores the host for its first 300 ms, as the GIGA's flash +// drive does: the first reset comes 150 ms after the plug, so the first +// attempt fails, and a retry has to find it ready. +static void test_slow_drive_mounts(void) { + reset_counts(); + plug(300); + run_for(3000); + printf("slow drive: %u resets, %u setups (%u ignored), mounted at address %u\n", + (unsigned)resets, (unsigned)setups, (unsigned)setups_ignored, mounted); + CHECK(setups_ignored > 0); + CHECK(mounted != 0); + CHECK(resets >= 2); + CHECK(dev0_opens == dev0_closes); + unplug(); + run_for(50); + CHECK(mounted == 0); +} + +// A device that never answers: the host stops after its retries, rather than +// resetting the port for good, and leaves no endpoint open. +static void test_dead_device_stops(void) { + reset_counts(); + plug(0xFFFFFFFF); + run_for(5000); + uint32_t resets_then = resets; + run_for(5000); + printf("dead device: %u resets, %u setups\n", (unsigned)resets, (unsigned)setups); + CHECK(mounted == 0); + CHECK(resets == resets_then); + CHECK(resets <= 4); + CHECK(dev0_opens == dev0_closes); + unplug(); + run_for(50); +} + +// A device that answers at once mounts on the first attempt, after a dead one +// used up its retries. +static void test_ready_device_after_dead_one(void) { + reset_counts(); + plug(0); + run_for(1000); + printf("ready device: %u resets, mounted at address %u\n", (unsigned)resets, mounted); + CHECK(mounted != 0); + CHECK(resets == 1); + unplug(); + run_for(50); +} + +int main(void) { + const tusb_rhport_init_t init = { .role = TUSB_ROLE_HOST, .speed = TUSB_SPEED_FULL }; + if (!tusb_rhport_init(0, &init)) { + printf("FAIL: tusb_rhport_init\n"); + return 1; + } + test_slow_drive_mounts(); + test_dead_device_stops(); + test_ready_device_after_dead_one(); + if (failures) { + printf("%d check(s) failed\n", failures); + return 1; + } + printf("all passed\n"); + return 0; +} diff --git a/modules/usbhost/tests/tinyusb_host/tusb_config.h b/modules/usbhost/tests/tinyusb_host/tusb_config.h new file mode 100644 index 00000000..92a3e817 --- /dev/null +++ b/modules/usbhost/tests/tinyusb_host/tusb_config.h @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: MIT +// +// TinyUSB's host stack, configured as the Arduino GIGA R1's USB-A port is +// (variants/stm32/ARDUINO_GIGA/DISPLAY_SHIELD/mpconfigboard.h), for +// test_tuh_enum_retry.c to build on a PC against a fake controller. +#ifndef TUSB_CONFIG_H +#define TUSB_CONFIG_H + +#define CFG_TUSB_MCU OPT_MCU_NONE +#define CFG_TUSB_OS OPT_OS_NONE +#define CFG_TUD_ENABLED (0) +#define CFG_TUH_ENABLED (1) +#define CFG_TUH_MAX_SPEED (OPT_MODE_FULL_SPEED) +#define CFG_TUH_HUB (1) +#define CFG_TUH_DEVICE_MAX (4) +#define CFG_TUH_API_EDPT_XFER (1) +#define CFG_TUH_ENUMERATION_BUFSIZE (256) + +#endif diff --git a/patches/lib-tinyusb/0002-usbh-reset-and-enumerate-a-root-port-again-after-a-failed-attach.patch b/patches/lib-tinyusb/0002-usbh-reset-and-enumerate-a-root-port-again-after-a-failed-attach.patch new file mode 100644 index 00000000..9ba78592 --- /dev/null +++ b/patches/lib-tinyusb/0002-usbh-reset-and-enumerate-a-root-port-again-after-a-failed-attach.patch @@ -0,0 +1,98 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Brad Barnett <127794626+bdbarnett@users.noreply.github.com> +Date: Tue, 10 Nov 2026 18:00:00 -0600 +Subject: [PATCH] usbh: reset and enumerate a root port again after a failed + attach. + +A device on a root port gets one enumeration. If its first request fails, +the stack marks the enumeration complete and nothing tries again: the port +stays connected and enabled, and the device is never mounted until it is +unplugged. A USB flash drive plugged straight into a port is still starting +up when the host resets it 150 ms after the plug: it doesn't answer the +device descriptor request 10 ms after the reset, and does 20 ms after it. + +After a failed enumeration on a root port that still has a device, the +attach is posted again, up to three times, so the port is debounced, reset +and enumerated anew. The count goes back to zero when an enumeration +succeeds, the device goes, or the retries run out, so the next attach gets +its own. An attach event that tears down an enumeration in flight doesn't +ask for a retry: it is about to enumerate the new device itself. A failure +before the device has an address also closes dev0's control endpoint, which +was left open and taken again by every later attempt. + +Upstream TinyUSB has the same root-port retry (USBH_ENUM_ROOT_RETRY_MAX) in +a later release; this is the part of it this version needs. +--- +--- a/src/host/usbh.c ++++ b/src/host/usbh.c +@@ -54,6 +54,7 @@ + + enum { + USBH_CONTROL_RETRY_MAX = 3, ++ USBH_ENUM_ROOT_RETRY_MAX = 3, // re-reset and re-enumerate the root port after a failed attach + }; + + //--------------------------------------------------------------------+ +@@ -224,6 +225,8 @@ TU_FIFO_DEF(_usbh_pending_ctrl_q, CFG_TUH_CONTROL_PENDING_QUEUE_SZ * sizeof(usbh + typedef struct { + uint8_t enumerating_daddr; // device address of the device being enumerated + uint8_t attach_debouncing_bm; // bitmask for roothub port attach debouncing ++ uint8_t attach_teardown; // an attach event is tearing down what was on its port ++ uint8_t root_retry_count; // root port re-enumerations used for the current attachment + tuh_bus_info_t dev0_bus; // bus info for dev0 in enumeration + usbh_ctrl_xfer_info_t ctrl_xfer_info; // control transfer + usbh_call_after_t call_after; +@@ -786,7 +789,11 @@ void tuh_task_ext(uint32_t timeout_ms, bool in_isr) { + // Should we miss the hub detach event due to high traffic, Or due to physical debouncing, some devices can + // cause multiple attaches (actually reset) without a detached event. + // Force remove currently mounted with the same bus info (rhport, hub addr, hub port) if exists ++ // Flagged because this teardown fails any in-flight enumeration, and its completion would ++ // otherwise ask for a roothub retry of the very attach being handled here. ++ _usbh_data.attach_teardown = 1; + process_remove_event(&event); ++ _usbh_data.attach_teardown = 0; + + // due to the shared control buffer, we must fully complete enumerating one device first. + if (_usbh_data.enumerating_daddr == TUSB_INDEX_INVALID_8) { +@@ -2229,18 +2236,40 @@ void usbh_driver_set_config_complete(uint8_t dev_addr, uint8_t itf_num) { + } + + static void enum_full_complete(bool success) { +- (void)success; + TU_LOG_USBH("Enumeration complete: success = %u\r\n", success); + ++ const uint8_t daddr = _usbh_data.enumerating_daddr; + _usbh_data.enumerating_daddr = TUSB_INDEX_INVALID_8; // mark enumeration as complete + _usbh_data.call_after.func = NULL; + ++ const uint8_t rhport = _usbh_data.dev0_bus.rhport; ++ if (!success && daddr == 0) { ++ // failed before it had an address: dev0's control endpoint is still open ++ hcd_device_close(rhport, 0); ++ } ++ + #if CFG_TUH_HUB + // Hub status is already requested in case of successful enumeration + if (!success && _usbh_data.dev0_bus.hub_addr != 0) { + hub_edpt_status_xfer(_usbh_data.dev0_bus.hub_addr); + } + #endif ++ ++ if (success || _usbh_data.attach_teardown || _usbh_data.dev0_bus.hub_addr != 0 || ++ !hcd_port_connect_status(rhport)) { ++ // The budget covers one attachment: give it back when that attachment ends. ++ _usbh_data.root_retry_count = 0; ++ } else if (_usbh_data.root_retry_count < USBH_ENUM_ROOT_RETRY_MAX) { ++ // A device behind a hub gets another chance from the hub's next port status change, but nothing ++ // retries the root port: the device stays connected and unenumerated until it is unplugged. A ++ // device that is still starting up after power-on (a flash drive) misses the first requests, so ++ // post the attach again: the port is debounced, reset and enumerated anew. ++ _usbh_data.root_retry_count++; ++ TU_LOG_USBH("Retry root port enumeration %u/%u\r\n", _usbh_data.root_retry_count, USBH_ENUM_ROOT_RETRY_MAX); ++ hcd_event_device_attach(rhport, false); ++ } else { ++ _usbh_data.root_retry_count = 0; // give up on this attach, but let a later one retry again ++ } + } + + #endif