/ports/esp32/usb_serial_jtag.c usb_serial_jtag_isr_handler bug
`static void usb_serial_jtag_isr_handler(void *arg) {
uint32_t flags = usb_serial_jtag_ll_get_intsts_mask();
if (flags & USB_SERIAL_JTAG_INTR_SOF) {
usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SOF);
}
if (flags & USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT) {
usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT);
/*
The commented part has bugs.
Assume the PC sends data very fast, such as 500 bytes (can be simulated by pasting),
After multiple interrupts, stdin_ringbuf will be full, that is, req_len = 0 below.
Since the interrupt has been cleared, the USB buffer cannot be read out,
interrupts will never happen again, causing the USB to fail and the link to eventually break.
Simply changing to
usb_serial_jtag_ll_read_rxfifo(rx_buf, USB_SERIAL_JTAG_BUF_SIZE)
can ensure normal operation. Of course, poll and other methods can also be used to fix this bug.
*/
//size_t req_len = ringbuf_free(&stdin_ringbuf);
//if (req_len > USB_SERIAL_JTAG_BUF_SIZE) {
// req_len = USB_SERIAL_JTAG_BUF_SIZE;
//}
//size_t len = usb_serial_jtag_ll_read_rxfifo(rx_buf, req_len);
size_t len = usb_serial_jtag_ll_read_rxfifo(rx_buf, USB_SERIAL_JTAG_BUF_SIZE);
for (size_t i = 0; i < len; ++i) {
if (rx_buf[i] == mp_interrupt_char) {
mp_sched_keyboard_interrupt();
} else {
ringbuf_put(&stdin_ringbuf, rx_buf[i]);
}
}
mp_hal_wake_main_task_from_isr();
}
}
`
esp32: Fix ESP32-C3 usb serial/jtag on IDF v5.0.4, clean up native USB-CDC
Summary
- Closes #15701 which is a regression introduced with #15523 when using IDF v5.0.x.
- Fixes a lower impact regression from the same PR where on ESP-IDF >= 5.1 the ESP32-S3 would come up using the USB Serial/JTAG interface instead of native USB. The obvious fixes for the first regression all made this second regression appear on all ESP-IDF versions.
- Overall the logic for how the USB function is assigned were a bit hard to follow, and depended on some default ESP-IDF config selections.
- There are now two new MicroPython config macros, fully independent of ESP-IDF config:
MICROPY_HW_USB_CDCfor native USB-CDC which defaults to 1 on supported SoCs (currently ESP32-S2,S3).MICROPY_HW_ESP_USB_SERIAL_JTAGto use the serial/JTAG peripheral which defaults to 1 on supported SoCs that don't have native USB-CDC enabled.
- Combined with the existing
MICROPY_HW_ENABLE_UART_REPLthis gives full control in the MicroPython board header over which serial options are enabled. This somewhat helps with #14217 although it's not a fix for it. - Support in the code for setting
CONFIG_ESP_CONSOLE_USB_CDCon S3 has been removed. This already didn't work on master, and no boards used it. Getting both ESP-IDF stdio and MicroPython stdio on the USB-CDC is actually now possible with this PR (didn't work without it), but requires setting both the sdkconfig option and the MicroPython build option independently.
Testing
Build and verified REPL works with ESP-IDF v5.0.4 and v5.2.2 on:
- ESP32-C3 DevKitC (external USB/serial)
- Seeed XIAO C3 (internal USB/serial)
- ESP32-S3 DevKitC (both "UART" and "native USB" ports)
Trade-offs and Alternatives
We could drop ESP-IDF v5.0.4 support now, but nightly builds have only just switched over so good to wait a little while and see if any other issues crop up. There is still the S3 issue to resolve.
There is a weird inconsistency where ESP-IDF stdout (and logs) go to different places than MicroPython stdout. However that's already the case on master and this PR makes it a little easier to reason about (because the ESP-IDF stdio config and the MicroPython stdio config are now fully independent of each other). If necessary then we could add a hook that routes ESP-IDF stdout into MicroPython stdout as part of app_main(), but I think safe to leave for now.
Future Work
- Disabling both USB config macros doesn't disable the USB port after MicroPython starts (currently, or after this change). PR to follow for this.
This work was funded through GitHub Sponsors.