From 1229b00d7109a27763658257bd9684528fdc7cc2 Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Fri, 11 Sep 2026 20:00:45 -0700 Subject: [PATCH 1/6] supervisor/usb: drain CDC console input on the TinyUSB task On espressif, tud_task() runs in its own FreeRTOS task at the same priority as the VM. TinyUSB releases the OUT endpoint before cdcd_xfer_cb copies the received packet into its fifo, so a tud_cdc_read from the VM in that window re-arms DMA into the same buffer: one 64-byte packet is lost and the next one repeated (hathach/tinyusb#1292). Measured as 3.5% of whole-file raw-REPL pastes failing with a SyntaxError on Metro ESP32-S3. Move all console CDC reads onto the task that runs tud_task(): tud_cdc_rx_cb drains the fifo into a small single-producer/single-consumer ring buffer and serial_read()/serial_bytes_available() read that. If the ring is full the leftover is drained after the next tud_task(), which on espressif now times out instead of blocking so a full ring cannot stall. Ctrl-C clears the ring too. This is the structure MicroPython uses (micropython#14462). Co-Authored-By: Claude Fable 5.1 --- ports/espressif/supervisor/usb.c | 6 ++- supervisor/shared/serial.c | 6 +-- supervisor/shared/usb/usb.c | 3 ++ supervisor/shared/usb/usb_device.c | 70 ++++++++++++++++++++++++++++-- supervisor/usb.h | 10 +++++ 5 files changed, 87 insertions(+), 8 deletions(-) diff --git a/ports/espressif/supervisor/usb.c b/ports/espressif/supervisor/usb.c index 7c3b1561a44..4210475d804 100644 --- a/ports/espressif/supervisor/usb.c +++ b/ports/espressif/supervisor/usb.c @@ -51,8 +51,12 @@ static void usb_device_task(void *param) { while (1) { // tinyusb device task if (tusb_inited()) { - tud_task(); + // Time out so the ring buffer is drained even when no USB event arrives. + tud_task_ext(10, false); tud_cdc_write_flush(); + #if CIRCUITPY_USB_CDC + usb_cdc_rx_background(); + #endif } // Yield with zero delay to switch to any other tasks at same priority. port_task_yield(); diff --git a/supervisor/shared/serial.c b/supervisor/shared/serial.c index 1bfb8d73b3c..dc88c65217f 100644 --- a/supervisor/shared/serial.c +++ b/supervisor/shared/serial.c @@ -327,8 +327,8 @@ char serial_read(void) { return -1; } #endif - #if CIRCUITPY_TINYUSB && CIRCUITPY_USB_DEVICE - return (char)tud_cdc_read_char(); + #if CIRCUITPY_TINYUSB && CIRCUITPY_USB_DEVICE && CIRCUITPY_USB_CDC + return (char)usb_cdc_rx_get(); #endif return -1; @@ -362,7 +362,7 @@ uint32_t serial_bytes_available(void) { #if CIRCUITPY_TINYUSB && CIRCUITPY_USB_DEVICE && CIRCUITPY_USB_CDC if (usb_cdc_console_enabled()) { - count += tud_cdc_available(); + count += usb_cdc_rx_available(); } #endif diff --git a/supervisor/shared/usb/usb.c b/supervisor/shared/usb/usb.c index 51222be0e59..9877f882ad6 100644 --- a/supervisor/shared/usb/usb.c +++ b/supervisor/shared/usb/usb.c @@ -161,6 +161,9 @@ void usb_background(void) { if (usb_enabled()) { #if CFG_TUSB_OS == OPT_OS_NONE || CFG_TUSB_OS == OPT_OS_PICO tud_task(); + #if CIRCUITPY_USB_DEVICE && CIRCUITPY_USB_CDC + usb_cdc_rx_background(); + #endif #if CIRCUITPY_USB_HOST || CIRCUITPY_MAX3421E tuh_task(); #endif diff --git a/supervisor/shared/usb/usb_device.c b/supervisor/shared/usb/usb_device.c index e3f1a0d4e26..0f49c217f32 100644 --- a/supervisor/shared/usb/usb_device.c +++ b/supervisor/shared/usb/usb_device.c @@ -173,6 +173,7 @@ void tud_cdc_rx_wanted_cb(uint8_t itf, char wanted_char) { // Compare mp_interrupt_char with wanted_char and ignore if not matched if (mp_interrupt_char == wanted_char) { tud_cdc_n_read_flush(itf); // flush read fifo + usb_cdc_rx_clear(); mp_sched_keyboard_interrupt(); } } @@ -184,10 +185,71 @@ void tud_cdc_send_break_cb(uint8_t itf, uint16_t duration_ms) { } } +#endif // MICROPY_KBD_EXCEPTION && CIRCUITPY_USB_CDC + +#if CIRCUITPY_USB_CDC +// Console input is moved out of TinyUSB's fifo here, on the task that runs +// tud_task(). Reading it from the VM task instead re-arms the OUT endpoint while +// TinyUSB may still be copying the previous packet into its fifo +// (hathach/tinyusb#1292): one packet is lost and the next one repeated. Only +// espressif runs tud_task() in its own task, but the ring buffer is harmless +// elsewhere. Single producer (tud_task) and single consumer (the VM), so the +// two indices need no lock. +#define CDC_RX_RING_SIZE (256) +static uint8_t _cdc_rx_buf[CDC_RX_RING_SIZE]; +static volatile uint16_t _cdc_rx_head; // written by the producer only +static volatile uint16_t _cdc_rx_tail; // written by the consumer only +static volatile bool _cdc_rx_pending; // fifo still holds bytes the ring could not take + +static inline uint16_t _cdc_rx_count(void) { + return (uint16_t)(_cdc_rx_head - _cdc_rx_tail); +} + +void usb_cdc_rx_drain(void) { + while (tud_cdc_available() > 0) { + if (_cdc_rx_count() >= CDC_RX_RING_SIZE) { + _cdc_rx_pending = true; + return; + } + int c = tud_cdc_read_char(); + if (c < 0) { + break; + } + _cdc_rx_buf[_cdc_rx_head % CDC_RX_RING_SIZE] = (uint8_t)c; + _cdc_rx_head++; + } + _cdc_rx_pending = false; +} + +void usb_cdc_rx_background(void) { + if (_cdc_rx_pending) { + usb_cdc_rx_drain(); + } +} + +int usb_cdc_rx_get(void) { + if (_cdc_rx_count() == 0) { + return -1; + } + uint8_t c = _cdc_rx_buf[_cdc_rx_tail % CDC_RX_RING_SIZE]; + _cdc_rx_tail++; + return c; +} + +size_t usb_cdc_rx_available(void) { + return _cdc_rx_count(); +} + +void usb_cdc_rx_clear(void) { + _cdc_rx_tail = _cdc_rx_head; +} + void tud_cdc_rx_cb(uint8_t itf) { - (void)itf; - // Workaround for "press any key to enter REPL" response being delayed on espressif. - // Wake main task when any key is pressed. + if (itf == 0) { + usb_cdc_rx_drain(); + } + // Wake the main task so it sees the input right away. On espressif it is a + // different FreeRTOS task from the one running TinyUSB. port_wake_main_task(); } -#endif +#endif // CIRCUITPY_USB_CDC diff --git a/supervisor/usb.h b/supervisor/usb.h index d6e4236c9b4..e0634692149 100644 --- a/supervisor/usb.h +++ b/supervisor/usb.h @@ -19,6 +19,16 @@ void usb_background(void); // Schedule usb background void usb_background_schedule(void); +#if CIRCUITPY_USB_CDC +// Console (CDC itf 0) input ring buffer, filled from tud_cdc_rx_cb on the task +// that runs tud_task() and read by the VM. See usb_device.c. +void usb_cdc_rx_drain(void); +void usb_cdc_rx_background(void); +int usb_cdc_rx_get(void); +size_t usb_cdc_rx_available(void); +void usb_cdc_rx_clear(void); +#endif + // Ports must call this from their particular USB IRQ handler void usb_irq_handler(int instance); From 20e2d87d9f8105ddefb98cf7b6ba4dd51011e62e Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Sat, 12 Sep 2026 05:41:51 -0700 Subject: [PATCH 2/6] supervisor/usb: apply the Ctrl-C ring flush on the reader side On espressif tud_cdc_rx_wanted_cb runs on the TinyUSB task, so moving the ring tail there gave the tail two writers. Ctrl-C now sets a flag and the VM moves the tail on its next read. Co-Authored-By: Claude Opus 5 (1M context) --- supervisor/shared/usb/usb_device.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/supervisor/shared/usb/usb_device.c b/supervisor/shared/usb/usb_device.c index 0f49c217f32..a052a4a7a4f 100644 --- a/supervisor/shared/usb/usb_device.c +++ b/supervisor/shared/usb/usb_device.c @@ -200,6 +200,7 @@ static uint8_t _cdc_rx_buf[CDC_RX_RING_SIZE]; static volatile uint16_t _cdc_rx_head; // written by the producer only static volatile uint16_t _cdc_rx_tail; // written by the consumer only static volatile bool _cdc_rx_pending; // fifo still holds bytes the ring could not take +static volatile bool _cdc_rx_flush; // set on Ctrl-C by the producer, applied by the consumer static inline uint16_t _cdc_rx_count(void) { return (uint16_t)(_cdc_rx_head - _cdc_rx_tail); @@ -227,7 +228,16 @@ void usb_cdc_rx_background(void) { } } +// Called by the consumer only, so the tail keeps a single writer. +static inline void _cdc_rx_apply_flush(void) { + if (_cdc_rx_flush) { + _cdc_rx_flush = false; + _cdc_rx_tail = _cdc_rx_head; + } +} + int usb_cdc_rx_get(void) { + _cdc_rx_apply_flush(); if (_cdc_rx_count() == 0) { return -1; } @@ -237,11 +247,12 @@ int usb_cdc_rx_get(void) { } size_t usb_cdc_rx_available(void) { + _cdc_rx_apply_flush(); return _cdc_rx_count(); } void usb_cdc_rx_clear(void) { - _cdc_rx_tail = _cdc_rx_head; + _cdc_rx_flush = true; } void tud_cdc_rx_cb(uint8_t itf) { From 759e763f2d3801c9c6c3f223b0b71de659fc4809 Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Sat, 12 Sep 2026 06:39:01 -0700 Subject: [PATCH 3/6] supervisor/usb: use py/ringbuf for CDC console input Replace the custom ring with the existing ringbuf, guarded with common_hal_mcu_disable_interrupts() the same way the espressif BLE CharacteristicBuffer guards its ringbuf. The USB task reads the TinyUSB fifo outside the critical section, since that takes a mutex, and puts the chunk into the ringbuf inside it. Ctrl-C clears the ringbuf directly. Co-Authored-By: Claude Opus 5 (1M context) --- supervisor/shared/usb/usb_device.c | 72 +++++++++++++----------------- 1 file changed, 31 insertions(+), 41 deletions(-) diff --git a/supervisor/shared/usb/usb_device.c b/supervisor/shared/usb/usb_device.c index a052a4a7a4f..1ddb9780101 100644 --- a/supervisor/shared/usb/usb_device.c +++ b/supervisor/shared/usb/usb_device.c @@ -14,6 +14,8 @@ #endif #if CIRCUITPY_USB_CDC +#include "py/ringbuf.h" +#include "shared-bindings/microcontroller/__init__.h" #include "shared-module/usb_cdc/__init__.h" #include "lib/tinyusb/src/class/cdc/cdc_device.h" #endif @@ -188,36 +190,32 @@ void tud_cdc_send_break_cb(uint8_t itf, uint16_t duration_ms) { #endif // MICROPY_KBD_EXCEPTION && CIRCUITPY_USB_CDC #if CIRCUITPY_USB_CDC -// Console input is moved out of TinyUSB's fifo here, on the task that runs -// tud_task(). Reading it from the VM task instead re-arms the OUT endpoint while -// TinyUSB may still be copying the previous packet into its fifo -// (hathach/tinyusb#1292): one packet is lost and the next one repeated. Only -// espressif runs tud_task() in its own task, but the ring buffer is harmless -// elsewhere. Single producer (tud_task) and single consumer (the VM), so the -// two indices need no lock. -#define CDC_RX_RING_SIZE (256) -static uint8_t _cdc_rx_buf[CDC_RX_RING_SIZE]; -static volatile uint16_t _cdc_rx_head; // written by the producer only -static volatile uint16_t _cdc_rx_tail; // written by the consumer only -static volatile bool _cdc_rx_pending; // fifo still holds bytes the ring could not take -static volatile bool _cdc_rx_flush; // set on Ctrl-C by the producer, applied by the consumer - -static inline uint16_t _cdc_rx_count(void) { - return (uint16_t)(_cdc_rx_head - _cdc_rx_tail); -} +// Console input is copied out of TinyUSB's fifo on the task that runs tud_task(). +// On espressif that is its own FreeRTOS task, and a read from the VM task can +// re-arm the OUT endpoint while the previous packet is still being copied. +// The ringbuf is shared by the two tasks, so guard it with interrupts disabled. +static uint8_t _cdc_rx_buf[256]; +static ringbuf_t _cdc_rx_ringbuf = { .buf = _cdc_rx_buf, .size = sizeof(_cdc_rx_buf) }; +static volatile bool _cdc_rx_pending; void usb_cdc_rx_drain(void) { + uint8_t chunk[CFG_TUD_CDC_RX_EPSIZE]; while (tud_cdc_available() > 0) { - if (_cdc_rx_count() >= CDC_RX_RING_SIZE) { + common_hal_mcu_disable_interrupts(); + size_t room = ringbuf_num_empty(&_cdc_rx_ringbuf); + common_hal_mcu_enable_interrupts(); + if (room == 0) { _cdc_rx_pending = true; return; } - int c = tud_cdc_read_char(); - if (c < 0) { + // tud_cdc_read() takes TinyUSB's fifo mutex, so it runs outside the critical section. + uint32_t count = tud_cdc_read(chunk, MIN(room, sizeof(chunk))); + if (count == 0) { break; } - _cdc_rx_buf[_cdc_rx_head % CDC_RX_RING_SIZE] = (uint8_t)c; - _cdc_rx_head++; + common_hal_mcu_disable_interrupts(); + ringbuf_put_n(&_cdc_rx_ringbuf, chunk, count); + common_hal_mcu_enable_interrupts(); } _cdc_rx_pending = false; } @@ -228,39 +226,31 @@ void usb_cdc_rx_background(void) { } } -// Called by the consumer only, so the tail keeps a single writer. -static inline void _cdc_rx_apply_flush(void) { - if (_cdc_rx_flush) { - _cdc_rx_flush = false; - _cdc_rx_tail = _cdc_rx_head; - } -} - int usb_cdc_rx_get(void) { - _cdc_rx_apply_flush(); - if (_cdc_rx_count() == 0) { - return -1; - } - uint8_t c = _cdc_rx_buf[_cdc_rx_tail % CDC_RX_RING_SIZE]; - _cdc_rx_tail++; + common_hal_mcu_disable_interrupts(); + int c = ringbuf_get(&_cdc_rx_ringbuf); + common_hal_mcu_enable_interrupts(); return c; } size_t usb_cdc_rx_available(void) { - _cdc_rx_apply_flush(); - return _cdc_rx_count(); + common_hal_mcu_disable_interrupts(); + size_t count = ringbuf_num_filled(&_cdc_rx_ringbuf); + common_hal_mcu_enable_interrupts(); + return count; } void usb_cdc_rx_clear(void) { - _cdc_rx_flush = true; + common_hal_mcu_disable_interrupts(); + ringbuf_clear(&_cdc_rx_ringbuf); + common_hal_mcu_enable_interrupts(); } void tud_cdc_rx_cb(uint8_t itf) { if (itf == 0) { usb_cdc_rx_drain(); } - // Wake the main task so it sees the input right away. On espressif it is a - // different FreeRTOS task from the one running TinyUSB. + // Wake the main task so it sees the input right away. port_wake_main_task(); } #endif // CIRCUITPY_USB_CDC From 3324f0f11c18fc500088cbf2d30beb3110317b7b Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Sat, 12 Sep 2026 07:34:40 -0700 Subject: [PATCH 4/6] supervisor/usb: limit the CDC console ringbuf to espressif Only espressif runs tud_task() in its own task, so the ringbuf is now built only when CFG_TUSB_OS is OPT_OS_FREERTOS; other ports read the TinyUSB fifo directly again, as on main. The espressif usbd loop calls the drain on every pass, which replaces the pending flag. The drain does nothing when the console is disabled, so a data serial at index 0 keeps its input, and usb_cdc.console reads, in_waiting and reset_input_buffer() use the ringbuf. The copy chunk is a fixed 64 bytes so a high-speed endpoint size does not put 512 bytes on the usbd task stack. Co-Authored-By: Claude Opus 5 (1M context) --- ports/espressif/supervisor/usb.c | 4 +- shared-module/usb_cdc/Serial.c | 32 +++++++- supervisor/shared/serial.c | 15 +++- supervisor/shared/usb/usb.c | 3 - supervisor/shared/usb/usb_device.c | 128 ++++++++++++++--------------- supervisor/usb.h | 10 +-- 6 files changed, 111 insertions(+), 81 deletions(-) diff --git a/ports/espressif/supervisor/usb.c b/ports/espressif/supervisor/usb.c index 4210475d804..64cc297e37a 100644 --- a/ports/espressif/supervisor/usb.c +++ b/ports/espressif/supervisor/usb.c @@ -51,11 +51,11 @@ static void usb_device_task(void *param) { while (1) { // tinyusb device task if (tusb_inited()) { - // Time out so the ring buffer is drained even when no USB event arrives. + // Time out so console input left in the fifo while the ringbuf was full is moved in. tud_task_ext(10, false); tud_cdc_write_flush(); #if CIRCUITPY_USB_CDC - usb_cdc_rx_background(); + usb_cdc_rx_drain(); #endif } // Yield with zero delay to switch to any other tasks at same priority. diff --git a/shared-module/usb_cdc/Serial.c b/shared-module/usb_cdc/Serial.c index 348729365c2..b4c86935f71 100644 --- a/shared-module/usb_cdc/Serial.c +++ b/shared-module/usb_cdc/Serial.c @@ -6,11 +6,29 @@ #include "shared/runtime/interrupt_char.h" #include "shared-bindings/usb_cdc/Serial.h" +#include "shared-module/usb_cdc/__init__.h" #include "shared-module/usb_cdc/Serial.h" #include "supervisor/shared/tick.h" +#include "supervisor/usb.h" #include "tusb.h" +#if CFG_TUSB_OS == OPT_OS_FREERTOS +// On espressif, console input is read from the ringbuf in supervisor/shared/usb/usb_device.c. +static bool _console_uses_ringbuf(usb_cdc_serial_obj_t *self) { + return self->idx == 0 && usb_cdc_console_enabled(); +} +#endif + +static uint32_t _read(usb_cdc_serial_obj_t *self, uint8_t *data, size_t len) { + #if CFG_TUSB_OS == OPT_OS_FREERTOS + if (_console_uses_ringbuf(self)) { + return usb_cdc_rx_read(data, len); + } + #endif + return tud_cdc_n_read(self->idx, data, len); +} + size_t common_hal_usb_cdc_serial_read(usb_cdc_serial_obj_t *self, uint8_t *data, size_t len, int *errcode) { const bool wait_forever = self->timeout < 0.0f; @@ -19,7 +37,7 @@ size_t common_hal_usb_cdc_serial_read(usb_cdc_serial_obj_t *self, uint8_t *data, // Read up to len bytes immediately. // The number of bytes read will not be larger than what is already in the TinyUSB FIFO. uint32_t total_num_read = 0; - total_num_read = tud_cdc_n_read(self->idx, data, len); + total_num_read = _read(self, data, len); if (wait_forever || wait_for_timeout) { // Continue filling the buffer past what we already read. @@ -46,7 +64,7 @@ size_t common_hal_usb_cdc_serial_read(usb_cdc_serial_obj_t *self, uint8_t *data, data += num_read; // Try to read another batch of bytes. - num_read = tud_cdc_n_read(self->idx, data, len); + num_read = _read(self, data, len); total_num_read += num_read; } } @@ -98,6 +116,11 @@ size_t common_hal_usb_cdc_serial_write(usb_cdc_serial_obj_t *self, const uint8_t } uint32_t common_hal_usb_cdc_serial_get_in_waiting(usb_cdc_serial_obj_t *self) { + #if CFG_TUSB_OS == OPT_OS_FREERTOS + if (_console_uses_ringbuf(self)) { + return usb_cdc_rx_available(); + } + #endif return tud_cdc_n_available(self->idx); } @@ -108,6 +131,11 @@ uint32_t common_hal_usb_cdc_serial_get_out_waiting(usb_cdc_serial_obj_t *self) { void common_hal_usb_cdc_serial_reset_input_buffer(usb_cdc_serial_obj_t *self) { tud_cdc_n_read_flush(self->idx); + #if CFG_TUSB_OS == OPT_OS_FREERTOS + if (_console_uses_ringbuf(self)) { + usb_cdc_rx_clear(); + } + #endif } uint32_t common_hal_usb_cdc_serial_reset_output_buffer(usb_cdc_serial_obj_t *self) { diff --git a/supervisor/shared/serial.c b/supervisor/shared/serial.c index dc88c65217f..0d7b07b63a7 100644 --- a/supervisor/shared/serial.c +++ b/supervisor/shared/serial.c @@ -327,8 +327,15 @@ char serial_read(void) { return -1; } #endif - #if CIRCUITPY_TINYUSB && CIRCUITPY_USB_DEVICE && CIRCUITPY_USB_CDC - return (char)usb_cdc_rx_get(); + #if CIRCUITPY_TINYUSB && CIRCUITPY_USB_DEVICE + #if CIRCUITPY_USB_CDC && CFG_TUSB_OS == OPT_OS_FREERTOS + uint8_t c; + if (usb_cdc_rx_read(&c, 1) == 1) { + return c; + } + #else + return (char)tud_cdc_read_char(); + #endif #endif return -1; @@ -362,7 +369,11 @@ uint32_t serial_bytes_available(void) { #if CIRCUITPY_TINYUSB && CIRCUITPY_USB_DEVICE && CIRCUITPY_USB_CDC if (usb_cdc_console_enabled()) { + #if CFG_TUSB_OS == OPT_OS_FREERTOS count += usb_cdc_rx_available(); + #else + count += tud_cdc_available(); + #endif } #endif diff --git a/supervisor/shared/usb/usb.c b/supervisor/shared/usb/usb.c index 9877f882ad6..51222be0e59 100644 --- a/supervisor/shared/usb/usb.c +++ b/supervisor/shared/usb/usb.c @@ -161,9 +161,6 @@ void usb_background(void) { if (usb_enabled()) { #if CFG_TUSB_OS == OPT_OS_NONE || CFG_TUSB_OS == OPT_OS_PICO tud_task(); - #if CIRCUITPY_USB_DEVICE && CIRCUITPY_USB_CDC - usb_cdc_rx_background(); - #endif #if CIRCUITPY_USB_HOST || CIRCUITPY_MAX3421E tuh_task(); #endif diff --git a/supervisor/shared/usb/usb_device.c b/supervisor/shared/usb/usb_device.c index 1ddb9780101..ddc8a7ae3e3 100644 --- a/supervisor/shared/usb/usb_device.c +++ b/supervisor/shared/usb/usb_device.c @@ -14,8 +14,6 @@ #endif #if CIRCUITPY_USB_CDC -#include "py/ringbuf.h" -#include "shared-bindings/microcontroller/__init__.h" #include "shared-module/usb_cdc/__init__.h" #include "lib/tinyusb/src/class/cdc/cdc_device.h" #endif @@ -26,6 +24,11 @@ #include "tusb.h" +#if CIRCUITPY_USB_CDC && CFG_TUSB_OS == OPT_OS_FREERTOS +#include "py/ringbuf.h" +#include "shared-bindings/microcontroller/__init__.h" +#endif + #if CIRCUITPY_USB_VENDOR #include "usb_vendor_descriptors.h" @@ -151,6 +154,54 @@ bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_requ #endif // CIRCUITPY_USB_VENDOR +#if CIRCUITPY_USB_CDC && CFG_TUSB_OS == OPT_OS_FREERTOS +// Console input is moved from the TinyUSB fifo into this ringbuf on the task that +// runs tud_task(), and read from the ringbuf on the VM task. Both tasks use the +// ringbuf, so every access is made with interrupts disabled. +static uint8_t _cdc_rx_buf[256]; +static ringbuf_t _cdc_rx_ringbuf = { .buf = _cdc_rx_buf, .size = sizeof(_cdc_rx_buf) }; + +void usb_cdc_rx_drain(void) { + if (!usb_cdc_console_enabled()) { + return; + } + uint8_t chunk[64]; + while (tud_cdc_available() > 0) { + common_hal_mcu_disable_interrupts(); + size_t room = ringbuf_num_empty(&_cdc_rx_ringbuf); + common_hal_mcu_enable_interrupts(); + if (room == 0) { + return; + } + // tud_cdc_read() can wait on the TinyUSB fifo mutex, so it runs with interrupts enabled. + uint32_t count = tud_cdc_read(chunk, MIN(room, sizeof(chunk))); + common_hal_mcu_disable_interrupts(); + ringbuf_put_n(&_cdc_rx_ringbuf, chunk, count); + common_hal_mcu_enable_interrupts(); + } +} + +size_t usb_cdc_rx_read(uint8_t *data, size_t len) { + common_hal_mcu_disable_interrupts(); + size_t count = ringbuf_get_n(&_cdc_rx_ringbuf, data, len); + common_hal_mcu_enable_interrupts(); + return count; +} + +size_t usb_cdc_rx_available(void) { + common_hal_mcu_disable_interrupts(); + size_t count = ringbuf_num_filled(&_cdc_rx_ringbuf); + common_hal_mcu_enable_interrupts(); + return count; +} + +void usb_cdc_rx_clear(void) { + common_hal_mcu_disable_interrupts(); + ringbuf_clear(&_cdc_rx_ringbuf); + common_hal_mcu_enable_interrupts(); +} +#endif + #if MICROPY_KBD_EXCEPTION && CIRCUITPY_USB_CDC // The CDC RX buffer impacts monitoring for ctrl-c. TinyUSB will only ask for @@ -175,7 +226,9 @@ void tud_cdc_rx_wanted_cb(uint8_t itf, char wanted_char) { // Compare mp_interrupt_char with wanted_char and ignore if not matched if (mp_interrupt_char == wanted_char) { tud_cdc_n_read_flush(itf); // flush read fifo + #if CFG_TUSB_OS == OPT_OS_FREERTOS usb_cdc_rx_clear(); + #endif mp_sched_keyboard_interrupt(); } } @@ -187,70 +240,13 @@ void tud_cdc_send_break_cb(uint8_t itf, uint16_t duration_ms) { } } -#endif // MICROPY_KBD_EXCEPTION && CIRCUITPY_USB_CDC - -#if CIRCUITPY_USB_CDC -// Console input is copied out of TinyUSB's fifo on the task that runs tud_task(). -// On espressif that is its own FreeRTOS task, and a read from the VM task can -// re-arm the OUT endpoint while the previous packet is still being copied. -// The ringbuf is shared by the two tasks, so guard it with interrupts disabled. -static uint8_t _cdc_rx_buf[256]; -static ringbuf_t _cdc_rx_ringbuf = { .buf = _cdc_rx_buf, .size = sizeof(_cdc_rx_buf) }; -static volatile bool _cdc_rx_pending; - -void usb_cdc_rx_drain(void) { - uint8_t chunk[CFG_TUD_CDC_RX_EPSIZE]; - while (tud_cdc_available() > 0) { - common_hal_mcu_disable_interrupts(); - size_t room = ringbuf_num_empty(&_cdc_rx_ringbuf); - common_hal_mcu_enable_interrupts(); - if (room == 0) { - _cdc_rx_pending = true; - return; - } - // tud_cdc_read() takes TinyUSB's fifo mutex, so it runs outside the critical section. - uint32_t count = tud_cdc_read(chunk, MIN(room, sizeof(chunk))); - if (count == 0) { - break; - } - common_hal_mcu_disable_interrupts(); - ringbuf_put_n(&_cdc_rx_ringbuf, chunk, count); - common_hal_mcu_enable_interrupts(); - } - _cdc_rx_pending = false; -} - -void usb_cdc_rx_background(void) { - if (_cdc_rx_pending) { - usb_cdc_rx_drain(); - } -} - -int usb_cdc_rx_get(void) { - common_hal_mcu_disable_interrupts(); - int c = ringbuf_get(&_cdc_rx_ringbuf); - common_hal_mcu_enable_interrupts(); - return c; -} - -size_t usb_cdc_rx_available(void) { - common_hal_mcu_disable_interrupts(); - size_t count = ringbuf_num_filled(&_cdc_rx_ringbuf); - common_hal_mcu_enable_interrupts(); - return count; -} - -void usb_cdc_rx_clear(void) { - common_hal_mcu_disable_interrupts(); - ringbuf_clear(&_cdc_rx_ringbuf); - common_hal_mcu_enable_interrupts(); -} - void tud_cdc_rx_cb(uint8_t itf) { - if (itf == 0) { - usb_cdc_rx_drain(); - } - // Wake the main task so it sees the input right away. + (void)itf; + #if CFG_TUSB_OS == OPT_OS_FREERTOS + usb_cdc_rx_drain(); + #endif + // Workaround for "press any key to enter REPL" response being delayed on espressif. + // Wake main task when any key is pressed. port_wake_main_task(); } -#endif // CIRCUITPY_USB_CDC +#endif diff --git a/supervisor/usb.h b/supervisor/usb.h index e0634692149..2a476ec7db4 100644 --- a/supervisor/usb.h +++ b/supervisor/usb.h @@ -19,15 +19,13 @@ void usb_background(void); // Schedule usb background void usb_background_schedule(void); -#if CIRCUITPY_USB_CDC -// Console (CDC itf 0) input ring buffer, filled from tud_cdc_rx_cb on the task -// that runs tud_task() and read by the VM. See usb_device.c. +// Console input ringbuf, used where tud_task() runs in its own task (espressif). +// usb_cdc_rx_drain() runs on that task, the others on the VM task. +// usb_cdc_rx_read() and usb_cdc_rx_available() return a byte count, 0 when empty. void usb_cdc_rx_drain(void); -void usb_cdc_rx_background(void); -int usb_cdc_rx_get(void); +size_t usb_cdc_rx_read(uint8_t *data, size_t len); size_t usb_cdc_rx_available(void); void usb_cdc_rx_clear(void); -#endif // Ports must call this from their particular USB IRQ handler void usb_irq_handler(int instance); From 6a8a2d8a6e6e843fdfcfceb66180a8511017717a Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Sun, 13 Sep 2026 10:18:35 -0700 Subject: [PATCH 5/6] supervisor/usb: default usb_cdc_rx_* implementations, espressif override Shared code now calls usb_cdc_rx_read/available/clear/drain unconditionally. The default implementations in supervisor/shared/usb/usb_device.c are weak pass-throughs to the TinyUSB fifo, the same behavior as before on ports that run tud_task() on the VM task. ports/espressif/supervisor/usb.c overrides them with the console ringbuf. No CFG_TUSB_OS conditionals remain in supervisor/shared or shared-module. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01KwTBwHF9YvQvN18ctYzJ7U --- ports/espressif/supervisor/usb.c | 55 ++++++++++++++++++++++++++++ shared-module/usb_cdc/Serial.c | 19 ++++------ supervisor/shared/serial.c | 10 +----- supervisor/shared/usb/usb_device.c | 57 +++++------------------------- supervisor/usb.h | 12 +++++-- 5 files changed, 80 insertions(+), 73 deletions(-) diff --git a/ports/espressif/supervisor/usb.c b/ports/espressif/supervisor/usb.c index 64cc297e37a..a347762ab4f 100644 --- a/ports/espressif/supervisor/usb.c +++ b/ports/espressif/supervisor/usb.c @@ -30,6 +30,61 @@ #include "hal/usb_serial_jtag_ll.h" #endif +#if CIRCUITPY_USB_DEVICE && CIRCUITPY_USB_CDC +#include "py/ringbuf.h" +#include "shared-bindings/microcontroller/__init__.h" +#include "shared-module/usb_cdc/__init__.h" + +// tud_task() runs in the usbd task below, not on the VM task. If the VM read the +// TinyUSB fifo directly it could re-arm the CDC OUT endpoint while tud_task() is +// still copying the previous packet out of the endpoint buffer (tinyusb#1292). +// So console input is moved from the fifo into this ringbuf on the usbd task and +// read from the ringbuf on the VM task. Both tasks use the ringbuf, so every +// access is made with interrupts disabled. +static uint8_t _cdc_rx_buf[256]; +static ringbuf_t _cdc_rx_ringbuf = { .buf = _cdc_rx_buf, .size = sizeof(_cdc_rx_buf) }; + +void usb_cdc_rx_drain(void) { + if (!usb_cdc_console_enabled()) { + return; + } + uint8_t chunk[64]; + while (tud_cdc_available() > 0) { + common_hal_mcu_disable_interrupts(); + size_t room = ringbuf_num_empty(&_cdc_rx_ringbuf); + common_hal_mcu_enable_interrupts(); + if (room == 0) { + return; + } + // tud_cdc_read() can wait on the TinyUSB fifo mutex, so it runs with interrupts enabled. + uint32_t count = tud_cdc_read(chunk, MIN(room, sizeof(chunk))); + common_hal_mcu_disable_interrupts(); + ringbuf_put_n(&_cdc_rx_ringbuf, chunk, count); + common_hal_mcu_enable_interrupts(); + } +} + +size_t usb_cdc_rx_read(uint8_t *data, size_t len) { + common_hal_mcu_disable_interrupts(); + size_t count = ringbuf_get_n(&_cdc_rx_ringbuf, data, len); + common_hal_mcu_enable_interrupts(); + return count; +} + +size_t usb_cdc_rx_available(void) { + common_hal_mcu_disable_interrupts(); + size_t count = ringbuf_num_filled(&_cdc_rx_ringbuf); + common_hal_mcu_enable_interrupts(); + return count; +} + +void usb_cdc_rx_clear(void) { + common_hal_mcu_disable_interrupts(); + ringbuf_clear(&_cdc_rx_ringbuf); + common_hal_mcu_enable_interrupts(); +} +#endif // CIRCUITPY_USB_DEVICE && CIRCUITPY_USB_CDC + #if CIRCUITPY_USB_DEVICE #ifdef CFG_TUSB_DEBUG #define USBD_STACK_SIZE (3 * configMINIMAL_STACK_SIZE) diff --git a/shared-module/usb_cdc/Serial.c b/shared-module/usb_cdc/Serial.c index b4c86935f71..45db6d51de4 100644 --- a/shared-module/usb_cdc/Serial.c +++ b/shared-module/usb_cdc/Serial.c @@ -13,19 +13,16 @@ #include "tusb.h" -#if CFG_TUSB_OS == OPT_OS_FREERTOS -// On espressif, console input is read from the ringbuf in supervisor/shared/usb/usb_device.c. -static bool _console_uses_ringbuf(usb_cdc_serial_obj_t *self) { +// Console input goes through the supervisor's usb_cdc_rx_* path (see supervisor/usb.h), +// which a port may stage in its own buffer. +static bool _is_console(usb_cdc_serial_obj_t *self) { return self->idx == 0 && usb_cdc_console_enabled(); } -#endif static uint32_t _read(usb_cdc_serial_obj_t *self, uint8_t *data, size_t len) { - #if CFG_TUSB_OS == OPT_OS_FREERTOS - if (_console_uses_ringbuf(self)) { + if (_is_console(self)) { return usb_cdc_rx_read(data, len); } - #endif return tud_cdc_n_read(self->idx, data, len); } @@ -116,11 +113,9 @@ size_t common_hal_usb_cdc_serial_write(usb_cdc_serial_obj_t *self, const uint8_t } uint32_t common_hal_usb_cdc_serial_get_in_waiting(usb_cdc_serial_obj_t *self) { - #if CFG_TUSB_OS == OPT_OS_FREERTOS - if (_console_uses_ringbuf(self)) { + if (_is_console(self)) { return usb_cdc_rx_available(); } - #endif return tud_cdc_n_available(self->idx); } @@ -131,11 +126,9 @@ uint32_t common_hal_usb_cdc_serial_get_out_waiting(usb_cdc_serial_obj_t *self) { void common_hal_usb_cdc_serial_reset_input_buffer(usb_cdc_serial_obj_t *self) { tud_cdc_n_read_flush(self->idx); - #if CFG_TUSB_OS == OPT_OS_FREERTOS - if (_console_uses_ringbuf(self)) { + if (_is_console(self)) { usb_cdc_rx_clear(); } - #endif } uint32_t common_hal_usb_cdc_serial_reset_output_buffer(usb_cdc_serial_obj_t *self) { diff --git a/supervisor/shared/serial.c b/supervisor/shared/serial.c index 0d7b07b63a7..5728ce646be 100644 --- a/supervisor/shared/serial.c +++ b/supervisor/shared/serial.c @@ -327,15 +327,11 @@ char serial_read(void) { return -1; } #endif - #if CIRCUITPY_TINYUSB && CIRCUITPY_USB_DEVICE - #if CIRCUITPY_USB_CDC && CFG_TUSB_OS == OPT_OS_FREERTOS + #if CIRCUITPY_TINYUSB && CIRCUITPY_USB_DEVICE && CIRCUITPY_USB_CDC uint8_t c; if (usb_cdc_rx_read(&c, 1) == 1) { return c; } - #else - return (char)tud_cdc_read_char(); - #endif #endif return -1; @@ -369,11 +365,7 @@ uint32_t serial_bytes_available(void) { #if CIRCUITPY_TINYUSB && CIRCUITPY_USB_DEVICE && CIRCUITPY_USB_CDC if (usb_cdc_console_enabled()) { - #if CFG_TUSB_OS == OPT_OS_FREERTOS count += usb_cdc_rx_available(); - #else - count += tud_cdc_available(); - #endif } #endif diff --git a/supervisor/shared/usb/usb_device.c b/supervisor/shared/usb/usb_device.c index ddc8a7ae3e3..e6276e8e4b5 100644 --- a/supervisor/shared/usb/usb_device.c +++ b/supervisor/shared/usb/usb_device.c @@ -24,11 +24,6 @@ #include "tusb.h" -#if CIRCUITPY_USB_CDC && CFG_TUSB_OS == OPT_OS_FREERTOS -#include "py/ringbuf.h" -#include "shared-bindings/microcontroller/__init__.h" -#endif - #if CIRCUITPY_USB_VENDOR #include "usb_vendor_descriptors.h" @@ -154,51 +149,21 @@ bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_requ #endif // CIRCUITPY_USB_VENDOR -#if CIRCUITPY_USB_CDC && CFG_TUSB_OS == OPT_OS_FREERTOS -// Console input is moved from the TinyUSB fifo into this ringbuf on the task that -// runs tud_task(), and read from the ringbuf on the VM task. Both tasks use the -// ringbuf, so every access is made with interrupts disabled. -static uint8_t _cdc_rx_buf[256]; -static ringbuf_t _cdc_rx_ringbuf = { .buf = _cdc_rx_buf, .size = sizeof(_cdc_rx_buf) }; - -void usb_cdc_rx_drain(void) { - if (!usb_cdc_console_enabled()) { - return; - } - uint8_t chunk[64]; - while (tud_cdc_available() > 0) { - common_hal_mcu_disable_interrupts(); - size_t room = ringbuf_num_empty(&_cdc_rx_ringbuf); - common_hal_mcu_enable_interrupts(); - if (room == 0) { - return; - } - // tud_cdc_read() can wait on the TinyUSB fifo mutex, so it runs with interrupts enabled. - uint32_t count = tud_cdc_read(chunk, MIN(room, sizeof(chunk))); - common_hal_mcu_disable_interrupts(); - ringbuf_put_n(&_cdc_rx_ringbuf, chunk, count); - common_hal_mcu_enable_interrupts(); - } +#if CIRCUITPY_USB_CDC +// Default console input path: tud_task() runs on the VM's task on these ports, so +// the TinyUSB fifo can be read directly. See supervisor/usb.h. +MP_WEAK void usb_cdc_rx_drain(void) { } -size_t usb_cdc_rx_read(uint8_t *data, size_t len) { - common_hal_mcu_disable_interrupts(); - size_t count = ringbuf_get_n(&_cdc_rx_ringbuf, data, len); - common_hal_mcu_enable_interrupts(); - return count; +MP_WEAK size_t usb_cdc_rx_read(uint8_t *data, size_t len) { + return tud_cdc_read(data, len); } -size_t usb_cdc_rx_available(void) { - common_hal_mcu_disable_interrupts(); - size_t count = ringbuf_num_filled(&_cdc_rx_ringbuf); - common_hal_mcu_enable_interrupts(); - return count; +MP_WEAK size_t usb_cdc_rx_available(void) { + return tud_cdc_available(); } -void usb_cdc_rx_clear(void) { - common_hal_mcu_disable_interrupts(); - ringbuf_clear(&_cdc_rx_ringbuf); - common_hal_mcu_enable_interrupts(); +MP_WEAK void usb_cdc_rx_clear(void) { } #endif @@ -226,9 +191,7 @@ void tud_cdc_rx_wanted_cb(uint8_t itf, char wanted_char) { // Compare mp_interrupt_char with wanted_char and ignore if not matched if (mp_interrupt_char == wanted_char) { tud_cdc_n_read_flush(itf); // flush read fifo - #if CFG_TUSB_OS == OPT_OS_FREERTOS usb_cdc_rx_clear(); - #endif mp_sched_keyboard_interrupt(); } } @@ -242,9 +205,7 @@ void tud_cdc_send_break_cb(uint8_t itf, uint16_t duration_ms) { void tud_cdc_rx_cb(uint8_t itf) { (void)itf; - #if CFG_TUSB_OS == OPT_OS_FREERTOS usb_cdc_rx_drain(); - #endif // Workaround for "press any key to enter REPL" response being delayed on espressif. // Wake main task when any key is pressed. port_wake_main_task(); diff --git a/supervisor/usb.h b/supervisor/usb.h index 2a476ec7db4..28ae399e625 100644 --- a/supervisor/usb.h +++ b/supervisor/usb.h @@ -19,9 +19,15 @@ void usb_background(void); // Schedule usb background void usb_background_schedule(void); -// Console input ringbuf, used where tud_task() runs in its own task (espressif). -// usb_cdc_rx_drain() runs on that task, the others on the VM task. -// usb_cdc_rx_read() and usb_cdc_rx_available() return a byte count, 0 when empty. +// Console (CDC interface 0) input. The default implementations read TinyUSB's +// fifo directly. A port that runs tud_task() in its own task (espressif) overrides +// them to stage input in a ringbuf, so the fifo is only touched on that task: +// usb_cdc_rx_drain() runs there, the other three on the VM task. +// +// usb_cdc_rx_read() copies up to len bytes into *data and returns the count, 0 when +// nothing is pending. usb_cdc_rx_available() returns the pending byte count. +// usb_cdc_rx_clear() discards staged input; callers flush the TinyUSB fifo themselves. +// usb_cdc_rx_drain() moves fifo bytes into the staging buffer; a no-op by default. void usb_cdc_rx_drain(void); size_t usb_cdc_rx_read(uint8_t *data, size_t len); size_t usb_cdc_rx_available(void); From 3e0a43eaf7760ffa293dbb935026854e215ee794 Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Sun, 13 Sep 2026 11:40:46 -0700 Subject: [PATCH 6/6] supervisor/usb: shorten the usb_cdc_rx_* comments Co-Authored-By: Claude Fable 5.1 --- ports/espressif/supervisor/usb.c | 10 ++++------ shared-module/usb_cdc/Serial.c | 3 +-- supervisor/shared/usb/usb_device.c | 3 +-- supervisor/usb.h | 14 +++++--------- 4 files changed, 11 insertions(+), 19 deletions(-) diff --git a/ports/espressif/supervisor/usb.c b/ports/espressif/supervisor/usb.c index a347762ab4f..ccf18ef96c0 100644 --- a/ports/espressif/supervisor/usb.c +++ b/ports/espressif/supervisor/usb.c @@ -35,12 +35,10 @@ #include "shared-bindings/microcontroller/__init__.h" #include "shared-module/usb_cdc/__init__.h" -// tud_task() runs in the usbd task below, not on the VM task. If the VM read the -// TinyUSB fifo directly it could re-arm the CDC OUT endpoint while tud_task() is -// still copying the previous packet out of the endpoint buffer (tinyusb#1292). -// So console input is moved from the fifo into this ringbuf on the usbd task and -// read from the ringbuf on the VM task. Both tasks use the ringbuf, so every -// access is made with interrupts disabled. +// tud_task() runs in the usbd task below. Reading the TinyUSB fifo from the VM +// task could re-arm the CDC OUT endpoint while tud_task() is still copying the +// previous packet out of it, so console input is staged in this ringbuf on the +// usbd task. Both tasks use it, so every access runs with interrupts disabled. static uint8_t _cdc_rx_buf[256]; static ringbuf_t _cdc_rx_ringbuf = { .buf = _cdc_rx_buf, .size = sizeof(_cdc_rx_buf) }; diff --git a/shared-module/usb_cdc/Serial.c b/shared-module/usb_cdc/Serial.c index 45db6d51de4..58445514f65 100644 --- a/shared-module/usb_cdc/Serial.c +++ b/shared-module/usb_cdc/Serial.c @@ -13,8 +13,7 @@ #include "tusb.h" -// Console input goes through the supervisor's usb_cdc_rx_* path (see supervisor/usb.h), -// which a port may stage in its own buffer. +// Console input goes through usb_cdc_rx_*, which a port may stage elsewhere. static bool _is_console(usb_cdc_serial_obj_t *self) { return self->idx == 0 && usb_cdc_console_enabled(); } diff --git a/supervisor/shared/usb/usb_device.c b/supervisor/shared/usb/usb_device.c index e6276e8e4b5..34def8d75a3 100644 --- a/supervisor/shared/usb/usb_device.c +++ b/supervisor/shared/usb/usb_device.c @@ -150,8 +150,7 @@ bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_requ #if CIRCUITPY_USB_CDC -// Default console input path: tud_task() runs on the VM's task on these ports, so -// the TinyUSB fifo can be read directly. See supervisor/usb.h. +// Defaults: tud_task() runs on the VM task here, so the fifo is read directly. MP_WEAK void usb_cdc_rx_drain(void) { } diff --git a/supervisor/usb.h b/supervisor/usb.h index 28ae399e625..d53e516064f 100644 --- a/supervisor/usb.h +++ b/supervisor/usb.h @@ -19,15 +19,11 @@ void usb_background(void); // Schedule usb background void usb_background_schedule(void); -// Console (CDC interface 0) input. The default implementations read TinyUSB's -// fifo directly. A port that runs tud_task() in its own task (espressif) overrides -// them to stage input in a ringbuf, so the fifo is only touched on that task: -// usb_cdc_rx_drain() runs there, the other three on the VM task. -// -// usb_cdc_rx_read() copies up to len bytes into *data and returns the count, 0 when -// nothing is pending. usb_cdc_rx_available() returns the pending byte count. -// usb_cdc_rx_clear() discards staged input; callers flush the TinyUSB fifo themselves. -// usb_cdc_rx_drain() moves fifo bytes into the staging buffer; a no-op by default. +// Console (CDC interface 0) input. Defaults read the TinyUSB fifo directly; a port +// whose tud_task() runs on another task overrides them and stages input there. +// usb_cdc_rx_drain() runs on that task, the other three on the VM task. +// read() fills *data with up to len bytes and returns the count, available() +// returns the pending count, both 0 when empty. clear() drops staged input only. void usb_cdc_rx_drain(void); size_t usb_cdc_rx_read(uint8_t *data, size_t len); size_t usb_cdc_rx_available(void);