Merge pull request #9409 from dhalbert/serial-jtag-atomic

Add more atomicity in usb_serial_jtag.c; handle C6 in more places

Author: tannewt

ports/espressif/common-hal/microcontroller/__init__.c

@@ -81,7 +81,7 @@ void common_hal_mcu_enable_interrupts(void) {
 void common_hal_mcu_on_next_reset(mcu_runmode_t runmode) {
     switch (runmode) {
         case RUNMODE_UF2:
-            #if defined(CONFIG_IDF_TARGET_ESP32) || defined(CONFIG_IDF_TARGET_ESP32C3)
+            #if defined(CONFIG_IDF_TARGET_ESP32) || defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32C6)
             mp_arg_error_invalid(MP_QSTR_run_mode);
             #else
             // 0x11F2 is APP_REQUEST_UF2_RESET_HINT & is defined by TinyUF2

ports/espressif/supervisor/port.c

@@ -240,9 +240,12 @@ safe_mode_t port_init(void) {
 
     #if DEBUG
     // debug UART
-    #ifdef CONFIG_IDF_TARGET_ESP32C3
+    #if defined(CONFIG_IDF_TARGET_ESP32C3)
     common_hal_never_reset_pin(&pin_GPIO20);
     common_hal_never_reset_pin(&pin_GPIO21);
+    #elif defined(CONFIG_IDF_TARGET_ESP32C6)
+    common_hal_never_reset_pin(&pin_GPIO16);
+    common_hal_never_reset_pin(&pin_GPIO17);
     #elif defined(CONFIG_IDF_TARGET_ESP32S2) || defined(CONFIG_IDF_TARGET_ESP32S3)
     common_hal_never_reset_pin(&pin_GPIO43);
     common_hal_never_reset_pin(&pin_GPIO44);
@@ -256,7 +259,7 @@ safe_mode_t port_init(void) {
     #if ENABLE_JTAG
     ESP_LOGI(TAG, "Marking JTAG pins never_reset");
     // JTAG
-    #ifdef CONFIG_IDF_TARGET_ESP32C3
+    #if defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32C6)
     common_hal_never_reset_pin(&pin_GPIO4);
     common_hal_never_reset_pin(&pin_GPIO5);
     common_hal_never_reset_pin(&pin_GPIO6);

ports/espressif/supervisor/usb.c

@@ -20,8 +20,10 @@
 #include "driver/gpio.h"
 #include "esp_private/periph_ctrl.h"
 
-#ifdef CONFIG_IDF_TARGET_ESP32C3
+#if defined(CONFIG_IDF_TARGET_ESP32C3)
 #include "components/esp_rom/include/esp32c3/rom/gpio.h"
+#elif defined(CONFIG_IDF_TARGET_ESP32C6)
+#include "components/esp_rom/include/esp32c6/rom/gpio.h"
 #elif defined(CONFIG_IDF_TARGET_ESP32S2)
 #include "components/esp_rom/include/esp32s2/rom/gpio.h"
 #elif defined(CONFIG_IDF_TARGET_ESP32S3)

ports/espressif/supervisor/usb_serial_jtag.c

@@ -39,7 +39,10 @@ static void _copy_out_of_fifo(void) {
         req_len = USB_SERIAL_JTAG_BUF_SIZE;
     }
     uint8_t rx_buf[USB_SERIAL_JTAG_BUF_SIZE];
+
+    // Read up to req_len bytes. Does not block.
     size_t len = usb_serial_jtag_ll_read_rxfifo(rx_buf, req_len);
+
     for (size_t i = 0; i < len; ++i) {
         if (rx_buf[i] == mp_interrupt_char) {
             mp_sched_keyboard_interrupt();
@@ -62,7 +65,9 @@ static void usb_serial_jtag_isr_handler(void *arg) {
     }
 
     if (flags & USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT) {
+        // New bytes are in the FIFO. Read them and check for keyboard interrupt.
         usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT);
+        // This is executed at interrupt level, so we don't explicitly need to make it atomic.
         _copy_out_of_fifo();
         port_wake_main_task_from_isr();
     }
@@ -81,28 +86,41 @@ bool usb_serial_jtag_connected(void) {
 }
 
 char usb_serial_jtag_read_char(void) {
-    if (ringbuf_num_filled(&ringbuf) == 0 && !usb_serial_jtag_ll_rxfifo_data_available()) {
+    uint32_t num_filled = ringbuf_num_filled(&ringbuf);
+
+    if (num_filled == 0 && !usb_serial_jtag_ll_rxfifo_data_available()) {
         return -1;
     }
     char c = -1;
-    if (ringbuf_num_filled(&ringbuf) > 0) {
+
+    if (num_filled > 0) {
+        common_hal_mcu_disable_interrupts();
         c = ringbuf_get(&ringbuf);
+        common_hal_mcu_enable_interrupts();
+
+        num_filled--;
     }
+
     // Maybe re-enable the recv interrupt if we've emptied the ringbuf.
-    if (ringbuf_num_filled(&ringbuf) == 0) {
+    if (num_filled == 0) {
         usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT);
         _copy_out_of_fifo();
-        usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT);
+
         // May have only been ctrl-c.
         if (c == -1 && ringbuf_num_filled(&ringbuf) > 0) {
             c = ringbuf_get(&ringbuf);
         }
+        usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT);
     }
     return c;
 }
 
 uint32_t usb_serial_jtag_bytes_available(void) {
-    return ringbuf_num_filled(&ringbuf) + usb_serial_jtag_ll_rxfifo_data_available();
+    // Atomically get the number of bytes in the ringbuf plus what is not yet in the ringbuf.
+    common_hal_mcu_disable_interrupts();
+    const uint32_t count = ringbuf_num_filled(&ringbuf) + usb_serial_jtag_ll_rxfifo_data_available();
+    common_hal_mcu_enable_interrupts();
+    return count;
 }
 
 void usb_serial_jtag_write(const char *text, uint32_t length) {