refactor(usb-serial-jtag): usb-serial-jtag driver simplification to fix rom print coexistence

This commit is contained in:
Jeroen Domburg 2024-06-13 11:34:13 +08:00
parent 0a12dbee45
commit 7982ece4af
2 changed files with 75 additions and 99 deletions

View File

@ -19,35 +19,31 @@
#include "esp_check.h" #include "esp_check.h"
#include "esp_private/periph_ctrl.h" #include "esp_private/periph_ctrl.h"
/*
Note: Before you add a workaround for an issue in this driver, please please try
to figure out the actual root cause first. The USB-serial-JTAG is a simple device,
it shouldn't need anything more than a simple, straightforward driver.
*/
#if !SOC_RCC_IS_INDEPENDENT #if !SOC_RCC_IS_INDEPENDENT
#define USJ_RCC_ATOMIC() PERIPH_RCC_ATOMIC() #define USJ_RCC_ATOMIC() PERIPH_RCC_ATOMIC()
#else #else
#define USJ_RCC_ATOMIC() #define USJ_RCC_ATOMIC()
#endif #endif
typedef enum { // The hardware buffer max size is 64, both for RX and TX.
FIFO_IDLE = 0, /*!< Indicates the fifo is in idle state */
FIFO_BUSY = 1, /*!< Indicates the fifo is in busy state */
} fifo_status_t;
// The hardware buffer max size is 64
#define USB_SER_JTAG_ENDP_SIZE (64) #define USB_SER_JTAG_ENDP_SIZE (64)
#define USB_SER_JTAG_RX_MAX_SIZE (64) #define USB_SER_JTAG_RX_MAX_SIZE (USB_SER_JTAG_ENDP_SIZE)
typedef struct { typedef struct {
intr_handle_t intr_handle; /*!< USB-SERIAL-JTAG interrupt handler */ intr_handle_t intr_handle; /*!< USB-SERIAL-JTAG interrupt handler */
portMUX_TYPE spinlock; /*!< Spinlock for usb_serial_jtag */
_Atomic fifo_status_t fifo_status; /*!< Record the status of fifo */
// RX parameters // RX parameters
RingbufHandle_t rx_ring_buf; /*!< RX ring buffer handler */ RingbufHandle_t rx_ring_buf; /*!< RX ring buffer handler */
uint32_t rx_buf_size; /*!< TX buffer size */
uint8_t rx_data_buf[USB_SER_JTAG_ENDP_SIZE]; /*!< Data buffer to stash FIFO data */
// TX parameters // TX parameters
uint32_t tx_buf_size; /*!< TX buffer size */
RingbufHandle_t tx_ring_buf; /*!< TX ring buffer handler */ RingbufHandle_t tx_ring_buf; /*!< TX ring buffer handler */
uint8_t tx_data_buf[USB_SER_JTAG_ENDP_SIZE]; /*!< Data buffer to stash TX FIFO data */ uint8_t tx_stash_buf[USB_SER_JTAG_ENDP_SIZE]; /*!< Data buffer to stash TX FIFO data */
size_t tx_stash_cnt; /*!< Number of stashed TX FIFO bytes */ size_t tx_stash_cnt; /*!< Number of stashed TX FIFO bytes */
} usb_serial_jtag_obj_t; } usb_serial_jtag_obj_t;
@ -55,13 +51,6 @@ static usb_serial_jtag_obj_t *p_usb_serial_jtag_obj = NULL;
static const char* USB_SERIAL_JTAG_TAG = "usb_serial_jtag"; static const char* USB_SERIAL_JTAG_TAG = "usb_serial_jtag";
static size_t usb_serial_jtag_write_and_flush(const uint8_t *buf, uint32_t wr_len)
{
size_t size = usb_serial_jtag_ll_write_txfifo(buf, wr_len);
usb_serial_jtag_ll_txfifo_flush();
return size;
}
static void usb_serial_jtag_isr_handler_default(void *arg) static void usb_serial_jtag_isr_handler_default(void *arg)
{ {
BaseType_t xTaskWoken = 0; BaseType_t xTaskWoken = 0;
@ -69,77 +58,70 @@ static void usb_serial_jtag_isr_handler_default(void *arg)
usbjtag_intr_status = usb_serial_jtag_ll_get_intsts_mask(); usbjtag_intr_status = usb_serial_jtag_ll_get_intsts_mask();
if (usbjtag_intr_status & USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY) { if (usbjtag_intr_status & USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY) {
//Clear interrupt so we won't be called until the next transfer finishes.
usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
// Interrupt tells us the host picked up the data we sent. // Interrupt tells us the host picked up the data we sent.
// If we have more data, we can put it in the buffer and the host will pick that up next. // If we have more data, we can put it in the buffer and the host will pick that up next.
// Send data in isr. // We expect the TX FIFO to be writable for this. If it's not, somehow someone else
// If the hardware fifo is available, write in it. Otherwise, do nothing. // (ROM print routines?) have snuck in a full buffer before we got here. In that case,
// we simply ignore the interrupt, a new one will come if the buffer is empty again.
if (usb_serial_jtag_ll_txfifo_writable() == 1) { if (usb_serial_jtag_ll_txfifo_writable() == 1) {
// We disable the interrupt here so that the interrupt won't be triggered if there is no data to send. // Retrieve data from either the stash buffer or, if that's empty, from the ring buffer.
size_t queued_size; size_t queued_size;
uint8_t *queued_buff = NULL; uint8_t *queued_buf = NULL;
bool is_stashed_data = false;
if (p_usb_serial_jtag_obj->tx_stash_cnt != 0) { if (p_usb_serial_jtag_obj->tx_stash_cnt != 0) {
// Send stashed tx bytes before reading bytes from ring buffer // Send stashed tx bytes before reading bytes from ring buffer
queued_buff = p_usb_serial_jtag_obj->tx_data_buf; queued_buf = p_usb_serial_jtag_obj->tx_stash_buf;
queued_size = p_usb_serial_jtag_obj->tx_stash_cnt; queued_size = p_usb_serial_jtag_obj->tx_stash_cnt;
is_stashed_data = true;
} else { } else {
// Max 64 data payload size in a single EndPoint // Max 64 data payload size in a single EndPoint
queued_buff = (uint8_t *)xRingbufferReceiveUpToFromISR(p_usb_serial_jtag_obj->tx_ring_buf, &queued_size, USB_SER_JTAG_ENDP_SIZE); queued_buf = (uint8_t *)xRingbufferReceiveUpToFromISR(p_usb_serial_jtag_obj->tx_ring_buf, &queued_size, USB_SER_JTAG_ENDP_SIZE);
} }
usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); if (queued_buf != NULL && queued_size > 0) {
// We have some data to send. Send it.
uint32_t sent_size = usb_serial_jtag_ll_write_txfifo(queued_buf, queued_size);
usb_serial_jtag_ll_txfifo_flush();
if (queued_buff != NULL) { // Check if we were able to send everything.
if (sent_size < queued_size) {
// Although tx_queued_bytes may be larger than 0, we may have // Not all bytes could be sent at once; stash the unwritten bytes in a buffer
// interrupted before xRingbufferSend() was called. // This will happen if e.g. the rom output functions manage to sneak a few bytes into the
// Copy the queued buffer into the TX FIFO // TX FIFO before this interrupt triggers. Note stash_size will not larger than
// USB_SER_JTAG_ENDP_SIZE because queued_size is obtained from xRingbufferReceiveUpToFromISR.
// On ringbuffer wrap-around the size can be 0 even though the buffer returned is not NULL size_t stash_size = queued_size - sent_size;
if (queued_size > 0) { memcpy(p_usb_serial_jtag_obj->tx_stash_buf, &queued_buf[sent_size], stash_size);
portENTER_CRITICAL_ISR(&p_usb_serial_jtag_obj->spinlock); p_usb_serial_jtag_obj->tx_stash_cnt = stash_size;
atomic_store(&p_usb_serial_jtag_obj->fifo_status, FIFO_BUSY); } else {
uint32_t sent_size = usb_serial_jtag_write_and_flush(queued_buff, queued_size); p_usb_serial_jtag_obj->tx_stash_cnt = 0;
portEXIT_CRITICAL_ISR(&p_usb_serial_jtag_obj->spinlock);
if (sent_size < queued_size) {
// Not all bytes could be sent at once; stash the unwritten bytes in a tx buffer
// stash_size will not larger than USB_SER_JTAG_ENDP_SIZE because queued_size is got from xRingbufferReceiveUpToFromISR
size_t stash_size = queued_size - sent_size;
memcpy(p_usb_serial_jtag_obj->tx_data_buf, &queued_buff[sent_size], stash_size);
p_usb_serial_jtag_obj->tx_stash_cnt = stash_size;
} else {
p_usb_serial_jtag_obj->tx_stash_cnt = 0;
// assert if sent_size is larger than queued_size.
assert(sent_size <= queued_size);
}
} }
if (is_stashed_data == false) { // Return the buffer if we got it from the ring buffer.
vRingbufferReturnItemFromISR(p_usb_serial_jtag_obj->tx_ring_buf, queued_buff, &xTaskWoken); if (queued_buf != p_usb_serial_jtag_obj->tx_stash_buf) {
vRingbufferReturnItemFromISR(p_usb_serial_jtag_obj->tx_ring_buf, queued_buf, &xTaskWoken);
} }
} else { } else {
// No data to send.
// The last transmit may have sent a full EP worth of data. The host will interpret // The last transmit may have sent a full EP worth of data. The host will interpret
// this as a transaction that hasn't finished yet and keep the data in its internal // this as a transaction that hasn't finished yet and keep the data in its internal
// buffers rather than releasing it to the program listening on the CDC serial port. // buffers rather than releasing it to the program listening on the CDC serial port.
// We need to flush again in order to send a 0-byte packet that ends the transaction. // We need to flush again in order to send a 0-byte packet that ends the transaction.
usb_serial_jtag_ll_txfifo_flush(); usb_serial_jtag_ll_txfifo_flush();
// Note that since this doesn't re-enable USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY, the
// flush will not by itself cause this ISR to be called again. // We will also disable the interrupt as for now there's no need to handle the
// TX interrupt again. We'll re-enable this externally if we need data sent.
usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
} }
} else {
atomic_store(&p_usb_serial_jtag_obj->fifo_status, FIFO_IDLE);
usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
} }
} }
if (usbjtag_intr_status & USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT) { if (usbjtag_intr_status & USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT) {
// read rx buffer(max length is 64), and send available data to ringbuffer. // Acknowledge interrupt
// Ensure the rx buffer size is larger than RX_MAX_SIZE.
usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT); usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT);
uint32_t rx_fifo_len = usb_serial_jtag_ll_read_rxfifo(p_usb_serial_jtag_obj->rx_data_buf, USB_SER_JTAG_RX_MAX_SIZE); // Read RX FIFO and send available data to ringbuffer.
xRingbufferSendFromISR(p_usb_serial_jtag_obj->rx_ring_buf, p_usb_serial_jtag_obj->rx_data_buf, rx_fifo_len, &xTaskWoken); uint8_t buf[USB_SER_JTAG_RX_MAX_SIZE];
uint32_t rx_fifo_len = usb_serial_jtag_ll_read_rxfifo(buf, USB_SER_JTAG_RX_MAX_SIZE);
xRingbufferSendFromISR(p_usb_serial_jtag_obj->rx_ring_buf, buf, rx_fifo_len, &xTaskWoken);
} }
if (xTaskWoken == pdTRUE) { if (xTaskWoken == pdTRUE) {
@ -157,15 +139,13 @@ esp_err_t usb_serial_jtag_driver_install(usb_serial_jtag_driver_config_t *usb_se
p_usb_serial_jtag_obj = (usb_serial_jtag_obj_t*) heap_caps_calloc(1, sizeof(usb_serial_jtag_obj_t), MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT); p_usb_serial_jtag_obj = (usb_serial_jtag_obj_t*) heap_caps_calloc(1, sizeof(usb_serial_jtag_obj_t), MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT);
if (p_usb_serial_jtag_obj == NULL) { if (p_usb_serial_jtag_obj == NULL) {
ESP_LOGE(USB_SERIAL_JTAG_TAG, "memory allocate error"); ESP_LOGE(USB_SERIAL_JTAG_TAG, "memory allocate error");
err = ESP_ERR_NO_MEM; // no `goto _exit` here as there's nothing to clean up and that would make the uninstall
goto _exit; // routine unhappy.
return ESP_ERR_NO_MEM;
} }
p_usb_serial_jtag_obj->rx_buf_size = usb_serial_jtag_config->rx_buffer_size;
p_usb_serial_jtag_obj->tx_buf_size = usb_serial_jtag_config->tx_buffer_size;
p_usb_serial_jtag_obj->tx_stash_cnt = 0; p_usb_serial_jtag_obj->tx_stash_cnt = 0;
p_usb_serial_jtag_obj->spinlock = (portMUX_TYPE)portMUX_INITIALIZER_UNLOCKED;
p_usb_serial_jtag_obj->rx_ring_buf = xRingbufferCreate(p_usb_serial_jtag_obj->rx_buf_size, RINGBUF_TYPE_BYTEBUF); p_usb_serial_jtag_obj->rx_ring_buf = xRingbufferCreate(usb_serial_jtag_config->rx_buffer_size, RINGBUF_TYPE_BYTEBUF);
if (p_usb_serial_jtag_obj->rx_ring_buf == NULL) { if (p_usb_serial_jtag_obj->rx_ring_buf == NULL) {
ESP_LOGE(USB_SERIAL_JTAG_TAG, "ringbuffer create error"); ESP_LOGE(USB_SERIAL_JTAG_TAG, "ringbuffer create error");
err = ESP_ERR_NO_MEM; err = ESP_ERR_NO_MEM;
@ -183,7 +163,6 @@ esp_err_t usb_serial_jtag_driver_install(usb_serial_jtag_driver_config_t *usb_se
USJ_RCC_ATOMIC() { USJ_RCC_ATOMIC() {
usb_serial_jtag_ll_enable_bus_clock(true); usb_serial_jtag_ll_enable_bus_clock(true);
} }
atomic_store(&p_usb_serial_jtag_obj->fifo_status, FIFO_IDLE);
// Configure PHY // Configure PHY
#if USB_SERIAL_JTAG_LL_EXT_PHY_SUPPORTED #if USB_SERIAL_JTAG_LL_EXT_PHY_SUPPORTED
@ -193,10 +172,14 @@ esp_err_t usb_serial_jtag_driver_install(usb_serial_jtag_driver_config_t *usb_se
usb_serial_jtag_ll_phy_set_defaults(); // External PHY not supported. Set default values. usb_serial_jtag_ll_phy_set_defaults(); // External PHY not supported. Set default values.
#endif // USB_WRAP_LL_EXT_PHY_SUPPORTED #endif // USB_WRAP_LL_EXT_PHY_SUPPORTED
usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY | // Note: DO NOT clear the interrupt status bits here. The output routine needs
USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT); // USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY set because it needs the ISR to trigger
usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY | // as soon as data is sent; the input routine needs the status to retrieve any
USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT); // data that is still in the FIFOs.
// We only enable the RX interrupt; we'll enable the TX one when we actually
// have anything to send.
usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT);
err = esp_intr_alloc(ETS_USB_SERIAL_JTAG_INTR_SOURCE, 0, usb_serial_jtag_isr_handler_default, NULL, &p_usb_serial_jtag_obj->intr_handle); err = esp_intr_alloc(ETS_USB_SERIAL_JTAG_INTR_SOURCE, 0, usb_serial_jtag_isr_handler_default, NULL, &p_usb_serial_jtag_obj->intr_handle);
if (err != ESP_OK) { if (err != ESP_OK) {
@ -238,37 +221,29 @@ int usb_serial_jtag_write_bytes(const void* src, size_t size, TickType_t ticks_t
ESP_RETURN_ON_FALSE(src != NULL, ESP_ERR_INVALID_ARG, USB_SERIAL_JTAG_TAG, "Invalid buffer pointer."); ESP_RETURN_ON_FALSE(src != NULL, ESP_ERR_INVALID_ARG, USB_SERIAL_JTAG_TAG, "Invalid buffer pointer.");
ESP_RETURN_ON_FALSE(p_usb_serial_jtag_obj != NULL, ESP_ERR_INVALID_ARG, USB_SERIAL_JTAG_TAG, "The driver hasn't been initialized"); ESP_RETURN_ON_FALSE(p_usb_serial_jtag_obj != NULL, ESP_ERR_INVALID_ARG, USB_SERIAL_JTAG_TAG, "The driver hasn't been initialized");
size_t sent_data = 0;
BaseType_t result = pdTRUE; BaseType_t result = pdTRUE;
const uint8_t *buff = (const uint8_t *)src; result = xRingbufferSend(p_usb_serial_jtag_obj->tx_ring_buf, (void*)src, size, ticks_to_wait);
if (p_usb_serial_jtag_obj->fifo_status == FIFO_IDLE) { // Re-enable the TX interrupt. If this was disabled, this will immediately trigger the ISR
portENTER_CRITICAL(&p_usb_serial_jtag_obj->spinlock); // and send the things we just put in the ringbuffer.
atomic_store(&p_usb_serial_jtag_obj->fifo_status, FIFO_BUSY);
sent_data = usb_serial_jtag_write_and_flush(src, size);
portEXIT_CRITICAL(&p_usb_serial_jtag_obj->spinlock);
}
// Blocking method, Sending data to ringbuffer, and handle the data in ISR.
if (size - sent_data > 0) {
result = xRingbufferSend(p_usb_serial_jtag_obj->tx_ring_buf, (void*)(buff + sent_data), size - sent_data, ticks_to_wait);
} else {
atomic_store(&p_usb_serial_jtag_obj->fifo_status, FIFO_IDLE);
}
usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
return (result == pdFALSE) ? 0 : size; return (result == pdFALSE) ? 0 : size;
} }
//Note that this is also called when usb_serial_jtag_driver_install errors out and as such should
//work on a half-initialized driver as well.
esp_err_t usb_serial_jtag_driver_uninstall(void) esp_err_t usb_serial_jtag_driver_uninstall(void)
{ {
if (p_usb_serial_jtag_obj == NULL) { if (p_usb_serial_jtag_obj == NULL) {
ESP_LOGI(USB_SERIAL_JTAG_TAG, "ALREADY NULL"); ESP_LOGE(USB_SERIAL_JTAG_TAG, "uninstall without install called");
return ESP_OK; return ESP_OK;
} }
/* Not disable the module clock and usb_pad_enable here since the USJ stdout might still depends on it. */ /* Don't disable the module clock and usb_pad_enable here since the USJ stdout might still depends on it. */
//Disable tx/rx interrupt.
usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY | USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT); usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY | USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT);
esp_intr_free(p_usb_serial_jtag_obj->intr_handle); if (p_usb_serial_jtag_obj->intr_handle) {
esp_intr_free(p_usb_serial_jtag_obj->intr_handle);
}
if (p_usb_serial_jtag_obj->rx_ring_buf) { if (p_usb_serial_jtag_obj->rx_ring_buf) {
vRingbufferDelete(p_usb_serial_jtag_obj->rx_ring_buf); vRingbufferDelete(p_usb_serial_jtag_obj->rx_ring_buf);

View File

@ -121,6 +121,11 @@ static void usb_serial_jtag_tx_char(int fd, int c)
do { do {
if (usb_serial_jtag_ll_txfifo_writable()) { if (usb_serial_jtag_ll_txfifo_writable()) {
usb_serial_jtag_ll_write_txfifo(&cc, 1); usb_serial_jtag_ll_write_txfifo(&cc, 1);
if (c == '\n') {
//Make sure line doesn't linger in fifo
usb_serial_jtag_ll_txfifo_flush();
}
//update time of last successful tx to now.
s_ctx.last_tx_ts = esp_timer_get_time(); s_ctx.last_tx_ts = esp_timer_get_time();
break; break;
} }
@ -155,10 +160,6 @@ static ssize_t usb_serial_jtag_write(int fd, const void * data, size_t size)
} }
} }
s_ctx.tx_func(fd, c); s_ctx.tx_func(fd, c);
if (c == '\n') {
//Make sure line doesn't linger in fifo
usb_serial_jtag_ll_txfifo_flush();
}
} }
_lock_release_recursive(&s_ctx.write_lock); _lock_release_recursive(&s_ctx.write_lock);
return size; return size;