From 982b55c209998ea7ddd81929a4f06b35dfd42367 Mon Sep 17 00:00:00 2001 From: sam blenny <68084116+samblenny@users.noreply.github.com> Date: Wed, 10 Sep 2025 00:51:42 +0000 Subject: [PATCH] run TinyUSB tasks more aggressively This attempts to fix a problem where a Fruit Jam board crashes if you access the CIRCUITPY drive while it's rapidly polling a gamepad using usb.core.Device.read(). The fix is incomplete. --- shared-bindings/usb/core/__init__.c | 9 ++++++- shared-module/usb/core/Device.c | 41 ++++++++++++++++++++++++++--- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/shared-bindings/usb/core/__init__.c b/shared-bindings/usb/core/__init__.c index 545e182ea99a3..fc9e9b46899a1 100644 --- a/shared-bindings/usb/core/__init__.c +++ b/shared-bindings/usb/core/__init__.c @@ -16,6 +16,8 @@ #include "shared-bindings/usb/core/__init__.h" #include "shared-bindings/usb/core/Device.h" +#include "lib/tinyusb/src/tusb.h" + //| """USB Core //| //| This is a subset of the PyUSB core module. @@ -74,9 +76,14 @@ typedef struct { // This is an internal iterator type to use with find. static mp_obj_t _next_device(usb_core_devices_obj_t *iter) { + // Force TinyUSB tasks to run (RUN_BACKGROUND_TASKS does not work for this) + #if CIRCUITPY_USB_DEVICE + tud_task(); + #endif + tuh_task(); // Brute force check all possible device numbers for one that matches. usb_core_device_obj_t temp_device; - for (size_t i = iter->next_index; i < 256; i++) { + for (size_t i = iter->next_index; i <= CFG_TUH_DEVICE_MAX + CFG_TUH_HUB; i++) { if (!common_hal_usb_core_device_construct(&temp_device, i)) { continue; } diff --git a/shared-module/usb/core/Device.c b/shared-module/usb/core/Device.c index c07ffda1149c9..d7e7b6d24b721 100644 --- a/shared-module/usb/core/Device.c +++ b/shared-module/usb/core/Device.c @@ -9,6 +9,7 @@ #include "tusb_config.h" +#include "lib/tinyusb/src/tusb.h" #include "lib/tinyusb/src/host/hcd.h" #include "lib/tinyusb/src/host/usbh.h" #include "py/runtime.h" @@ -31,9 +32,24 @@ void tuh_umount_cb(uint8_t dev_addr) { _mounted_devices &= ~(1 << dev_addr); } +static void _force_tinyusb_tasks_to_run(void) { + // TinyUSB uses state machines and task queues that require tud_task() + // and tuh_task() to be called very frequently. If you try to use tuh_* API + // functions when something interesting has happened (device unplugged, etc), + // things can go badly wrong if you wait too long to call the task runners. + // For example, although RUN_BACKGROUND_TASKS schedules TinyUSB background + // tasks at 1024 Hz, that doesn't seem to be sufficient if you're rapidly + // polling usb.core.Device.read(). + #if CIRCUITPY_USB_DEVICE + tud_task(); + #endif + tuh_task(); +} + static xfer_result_t _xfer_result; static size_t _actual_len; bool common_hal_usb_core_device_construct(usb_core_device_obj_t *self, uint8_t device_address) { + _force_tinyusb_tasks_to_run(); if (!tuh_inited()) { mp_raise_RuntimeError(MP_ERROR_TEXT("No usb host port initialized")); } @@ -58,6 +74,7 @@ void common_hal_usb_core_device_deinit(usb_core_device_obj_t *self) { if (common_hal_usb_core_device_deinited(self)) { return; } + _force_tinyusb_tasks_to_run(); size_t open_size = sizeof(self->open_endpoints); for (size_t i = 0; i < open_size; i++) { if (self->open_endpoints[i] != 0) { @@ -66,11 +83,13 @@ void common_hal_usb_core_device_deinit(usb_core_device_obj_t *self) { } } self->device_address = 0; + _force_tinyusb_tasks_to_run(); } uint16_t common_hal_usb_core_device_get_idVendor(usb_core_device_obj_t *self) { uint16_t vid; uint16_t pid; + _force_tinyusb_tasks_to_run(); if (!tuh_vid_pid_get(self->device_address, &vid, &pid)) { mp_raise_usb_core_USBError(NULL); } @@ -80,6 +99,7 @@ uint16_t common_hal_usb_core_device_get_idVendor(usb_core_device_obj_t *self) { uint16_t common_hal_usb_core_device_get_idProduct(usb_core_device_obj_t *self) { uint16_t vid; uint16_t pid; + _force_tinyusb_tasks_to_run(); if (!tuh_vid_pid_get(self->device_address, &vid, &pid)) { mp_raise_usb_core_USBError(NULL); } @@ -97,8 +117,8 @@ static void _transfer_done_cb(tuh_xfer_t *xfer) { static bool _wait_for_callback(void) { while (!mp_hal_is_interrupted() && _xfer_result == XFER_RESULT_INVALID) { - // The background tasks include TinyUSB which will call the function - // we provided above. In other words, the callback isn't in an interrupt. + // The background tasks include TinyUSB which will call _transfer_done_cb(). + // In other words, the callback isn't in an interrupt. RUN_BACKGROUND_TASKS; } if (mp_hal_is_interrupted()) { @@ -142,8 +162,8 @@ static size_t _handle_timed_transfer_callback(tuh_xfer_t *xfer, mp_int_t timeout while ((timeout == 0 || supervisor_ticks_ms32() - start_time < (uint32_t)timeout) && !mp_hal_is_interrupted() && _xfer_result == XFER_RESULT_INVALID) { - // The background tasks include TinyUSB which will call the function - // we provided above. In other words, the callback isn't in an interrupt. + // The background tasks include TinyUSB which will call _transfer_done_cb(). + // In other words, the callback isn't in an interrupt. RUN_BACKGROUND_TASKS; } if (mp_hal_is_interrupted()) { @@ -203,6 +223,7 @@ static void _get_langid(usb_core_device_obj_t *self) { mp_obj_t common_hal_usb_core_device_get_serial_number(usb_core_device_obj_t *self) { uint16_t temp_buf[127]; tusb_desc_device_t descriptor; + _force_tinyusb_tasks_to_run(); // First, be sure not to ask TinyUSB for a non-existent string (avoid error) if (!tuh_descriptor_get_device_local(self->device_address, &descriptor)) { return mp_const_none; @@ -224,6 +245,7 @@ mp_obj_t common_hal_usb_core_device_get_serial_number(usb_core_device_obj_t *sel mp_obj_t common_hal_usb_core_device_get_product(usb_core_device_obj_t *self) { uint16_t temp_buf[127]; tusb_desc_device_t descriptor; + _force_tinyusb_tasks_to_run(); // First, be sure not to ask TinyUSB for a non-existent string (avoid error) if (!tuh_descriptor_get_device_local(self->device_address, &descriptor)) { return mp_const_none; @@ -245,6 +267,7 @@ mp_obj_t common_hal_usb_core_device_get_product(usb_core_device_obj_t *self) { mp_obj_t common_hal_usb_core_device_get_manufacturer(usb_core_device_obj_t *self) { uint16_t temp_buf[127]; tusb_desc_device_t descriptor; + _force_tinyusb_tasks_to_run(); // First, be sure not to ask TinyUSB for a non-existent string (avoid error) if (!tuh_descriptor_get_device_local(self->device_address, &descriptor)) { return mp_const_none; @@ -266,6 +289,7 @@ mp_obj_t common_hal_usb_core_device_get_manufacturer(usb_core_device_obj_t *self mp_int_t common_hal_usb_core_device_get_bus(usb_core_device_obj_t *self) { tuh_bus_info_t bus_info; + _force_tinyusb_tasks_to_run(); if (!tuh_bus_info_get(self->device_address, &bus_info)) { return 0; } @@ -274,6 +298,7 @@ mp_int_t common_hal_usb_core_device_get_bus(usb_core_device_obj_t *self) { mp_obj_t common_hal_usb_core_device_get_port_numbers(usb_core_device_obj_t *self) { tuh_bus_info_t bus_info; + _force_tinyusb_tasks_to_run(); if (!tuh_bus_info_get(self->device_address, &bus_info)) { return mp_const_none; } @@ -297,6 +322,7 @@ mp_obj_t common_hal_usb_core_device_get_port_numbers(usb_core_device_obj_t *self mp_int_t common_hal_usb_core_device_get_speed(usb_core_device_obj_t *self) { tuh_bus_info_t bus_info; + _force_tinyusb_tasks_to_run(); if (!tuh_bus_info_get(self->device_address, &bus_info)) { return 0; } @@ -315,6 +341,7 @@ mp_int_t common_hal_usb_core_device_get_speed(usb_core_device_obj_t *self) { void common_hal_usb_core_device_set_configuration(usb_core_device_obj_t *self, mp_int_t configuration) { // We assume that the config index is one less than the value. uint8_t config_index = configuration - 1; + _force_tinyusb_tasks_to_run(); // Get the configuration descriptor and cache it. We'll use it later to open // endpoints. @@ -395,6 +422,7 @@ static bool _open_endpoint(usb_core_device_obj_t *self, mp_int_t endpoint) { } mp_int_t common_hal_usb_core_device_write(usb_core_device_obj_t *self, mp_int_t endpoint, const uint8_t *buffer, mp_int_t len, mp_int_t timeout) { + _force_tinyusb_tasks_to_run(); if (!_open_endpoint(self, endpoint)) { mp_raise_usb_core_USBError(NULL); return 0; @@ -408,6 +436,7 @@ mp_int_t common_hal_usb_core_device_write(usb_core_device_obj_t *self, mp_int_t } mp_int_t common_hal_usb_core_device_read(usb_core_device_obj_t *self, mp_int_t endpoint, uint8_t *buffer, mp_int_t len, mp_int_t timeout) { + _force_tinyusb_tasks_to_run(); if (!_open_endpoint(self, endpoint)) { mp_raise_usb_core_USBError(NULL); return 0; @@ -441,6 +470,7 @@ mp_int_t common_hal_usb_core_device_ctrl_transfer(usb_core_device_obj_t *self, .complete_cb = _transfer_done_cb, }; + _force_tinyusb_tasks_to_run(); _prepare_for_transfer(); if (!tuh_control_xfer(&xfer)) { mp_raise_usb_core_USBError(NULL); @@ -450,6 +480,7 @@ mp_int_t common_hal_usb_core_device_ctrl_transfer(usb_core_device_obj_t *self, } bool common_hal_usb_core_device_is_kernel_driver_active(usb_core_device_obj_t *self, mp_int_t interface) { + _force_tinyusb_tasks_to_run(); #if CIRCUITPY_USB_KEYBOARD_WORKFLOW if (usb_keyboard_in_use(self->device_address, interface)) { return true; @@ -459,12 +490,14 @@ bool common_hal_usb_core_device_is_kernel_driver_active(usb_core_device_obj_t *s } void common_hal_usb_core_device_detach_kernel_driver(usb_core_device_obj_t *self, mp_int_t interface) { + _force_tinyusb_tasks_to_run(); #if CIRCUITPY_USB_KEYBOARD_WORKFLOW usb_keyboard_detach(self->device_address, interface); #endif } void common_hal_usb_core_device_attach_kernel_driver(usb_core_device_obj_t *self, mp_int_t interface) { + _force_tinyusb_tasks_to_run(); #if CIRCUITPY_USB_KEYBOARD_WORKFLOW usb_keyboard_attach(self->device_address, interface); #endif