Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ports/espressif/common-hal/microcontroller/__init__.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions ports/espressif/supervisor/port.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion ports/espressif/supervisor/usb.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
28 changes: 23 additions & 5 deletions ports/espressif/supervisor/usb_serial_jtag.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
}
Expand All @@ -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) {
Expand Down