From c39be8bc76bf082701c688fffc322385fe6a7d2b Mon Sep 17 00:00:00 2001 From: Chip Weinberger Date: Thu, 8 Dec 2022 00:12:20 -0800 Subject: [PATCH 1/2] [USB Serial/JTAG Driver] use time-limited blocking for TX --- .../include/driver/usb_serial_jtag.h | 6 ++-- .../driver/usb_serial_jtag/usb_serial_jtag.c | 33 +++++++++++++++---- components/vfs/vfs_usb_serial_jtag.c | 3 +- .../en/api-guides/usb-serial-jtag-console.rst | 20 ++++++----- 4 files changed, 44 insertions(+), 18 deletions(-) diff --git a/components/driver/usb_serial_jtag/include/driver/usb_serial_jtag.h b/components/driver/usb_serial_jtag/include/driver/usb_serial_jtag.h index f9d0d3f7f476..c1e97bb4d801 100644 --- a/components/driver/usb_serial_jtag/include/driver/usb_serial_jtag.h +++ b/components/driver/usb_serial_jtag/include/driver/usb_serial_jtag.h @@ -35,7 +35,9 @@ typedef struct { * USB-SERIAL-JTAG driver's ISR will be attached to the same CPU core that calls this function. Thus, users * should ensure that the same core is used when calling `usb_serial_jtag_driver_uninstall()`. * - * @note Blocking mode will result in usb_serial_jtag_write_bytes() blocking until all bytes have been written to the TX FIFO. + * @note Blocking mode will result in usb_serial_jtag_write_bytes() blocking for a + * short period if the TX FIFO if full. It will not block again until the buffer + * has some space available again. * * @param usb_serial_jtag_driver_config_t Configuration for usb_serial_jtag driver. * @@ -65,7 +67,7 @@ int usb_serial_jtag_read_bytes(void* buf, uint32_t length, TickType_t ticks_to_w * * @param src data buffer address * @param size data length to send - * @param ticks_to_wait Timeout in RTOS ticks + * @param ticks_to_wait Maximum timeout in RTOS ticks * * @return * - The number of bytes pushed to the TX FIFO diff --git a/components/driver/usb_serial_jtag/usb_serial_jtag.c b/components/driver/usb_serial_jtag/usb_serial_jtag.c index 94879aa117f8..b113beca1f90 100644 --- a/components/driver/usb_serial_jtag/usb_serial_jtag.c +++ b/components/driver/usb_serial_jtag/usb_serial_jtag.c @@ -32,6 +32,7 @@ typedef struct{ // TX parameters uint32_t tx_buf_size; /*!< TX buffer size */ RingbufHandle_t tx_ring_buf; /*!< TX ring buffer handler */ + bool tx_has_tried_blocking; /*!< TX have we tried a blocking send already? */ } usb_serial_jtag_obj_t; static usb_serial_jtag_obj_t *p_usb_serial_jtag_obj = NULL; @@ -161,12 +162,32 @@ 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(p_usb_serial_jtag_obj != NULL, ESP_ERR_INVALID_ARG, USB_SERIAL_JTAG_TAG, "The driver hasn't been initialized"); - const uint8_t *buff = (const uint8_t *)src; - // Blocking method, Sending data to ringbuffer, and handle the data in ISR. - xRingbufferSend(p_usb_serial_jtag_obj->tx_ring_buf, (void*) (buff), size, ticks_to_wait); - // Now trigger the ISR to read data from the ring buffer. - usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); - return size; + usb_serial_jtag_obj_t* sjtag = p_usb_serial_jtag_obj; + + // try to send without blocking + if (xRingbufferSend(sjtag->tx_ring_buf, (void*) (src), size, 0)) { + goto success; + } + + // If the ring buffer is full, try to send again with a short + // blocking delay, hoping for the buffer to become available soon. + // If we still fail, dont ever block again until the buffer has space again. + if (sjtag->tx_has_tried_blocking == false) { + if (xRingbufferSend(sjtag->tx_ring_buf, (void*) (src), size, ticks_to_wait)) { + goto success; + } else { + sjtag->tx_has_tried_blocking = true; + } + } + + // tx failure + return 0; + +success: + // Now trigger the ISR to read more data from the ring buffer. + usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); + sjtag->tx_has_tried_blocking = false; // clear + return size; } esp_err_t usb_serial_jtag_driver_uninstall(void) diff --git a/components/vfs/vfs_usb_serial_jtag.c b/components/vfs/vfs_usb_serial_jtag.c index a0d0771c6c5c..b5427d394be3 100644 --- a/components/vfs/vfs_usb_serial_jtag.c +++ b/components/vfs/vfs_usb_serial_jtag.c @@ -404,7 +404,8 @@ static int usbjtag_rx_char_via_driver(int fd) static void usbjtag_tx_char_via_driver(int fd, int c) { char ch = (char) c; - usb_serial_jtag_write_bytes(&ch, 1, portMAX_DELAY); + TickType_t ticks = (TX_FLUSH_TIMEOUT_US / 1000) / portTICK_PERIOD_MS; + usb_serial_jtag_write_bytes(&ch, 1, ticks); } void esp_vfs_usb_serial_jtag_use_nonblocking(void) diff --git a/docs/en/api-guides/usb-serial-jtag-console.rst b/docs/en/api-guides/usb-serial-jtag-console.rst index c2d548379e3b..8c7eb1620f38 100644 --- a/docs/en/api-guides/usb-serial-jtag-console.rst +++ b/docs/en/api-guides/usb-serial-jtag-console.rst @@ -53,26 +53,28 @@ The USB Serial/JTAG Controller is able to put the {IDF_TARGET_NAME} into downloa Limitations =========== -There are several limitations to the USB console feature. These may or may not be significant, depending on the type of application being developed, and the development workflow. +There are several limitations to the USB Serial/JTAG console feature. These may or may not be significant, depending on the type of application being developed, and the development workflow. {IDF_TARGET_BOOT_PIN:default = "Not Updated!", esp32c3 = "GPIO9", esp32s3 = "GPIO0"} 1. If the application accidentally reconfigures the USB peripheral pins, or disables the USB Serial/JTAG Controller, the device will disappear from the system. After fixing the issue in the application, you will need to manually put the {IDF_TARGET_NAME} into download mode by pulling low {IDF_TARGET_BOOT_PIN} and resetting the chip. -2. If the application enters deep sleep mode, USB CDC device will disappear from the system. +2. If the application enters deep sleep mode, the USB Serial/JTAG device will disappear from the system. -3. The behavior between an actual USB-to-serial bridge chip and the USB Serial/JTAG Controller is slightly different if the ESP-IDF application does not listen for incoming bytes. An USB-to-serial bridge chip will just send the bytes to a (not listening) chip, while the USB Serial/JTAG Controller will block until the application reads the bytes. This can lead to a non-responsive looking terminal program. +3. For data sent in the direction of ESP32 to PC Terminal (e.g. stdout, logs), the ESP32 first writes to a small internal buffer. If this buffer becomes full (for example, if no PC Terminal is connected), the ESP32 will do a one-time wait of 50ms hoping for the PC Terminal to request the data. This can appear as a very brief 'pause' in your application. -4. The USB CDC device won't work in sleep modes as normal due to the lack of APB clock in sleep modes. This includes deep-sleep, light-sleep (automataic light-sleep as well). +4. For data sent in the PC Terminal to ESP32 direction (e.g. console commands), many PC Terminals will wait for the ESP32 to ingest the bytes before allowing you to sending more data. This is in contrast to using a USB-to-Serial (UART) bridge chip, which will always ingest the bytes and send them to a (possibly not listening) ESP32. -5. The power consumption in sleep modes will be higher if the USB CDC device is in use. +5. The USB Serial/JTAG device won't work in sleep modes as normal due to the lack of APB clock in sleep modes. This includes deep-sleep, light-sleep (automataic light-sleep as well). - This is because we want to keep the USB CDC device alive during software reset by default. +6. The power consumption in sleep modes will be higher if the USB Serial/JTAG device is in use. - However there is an issue that this might also increase the power consumption in sleep modes. This is because the software keeps a clock source on during the reset to keep the USB CDC device alive. As a side-effect, the clock is also kept on during sleep modes. There is one exception: the clock will only be kept on when your USB CDC port is really in use (like data transaction), therefore, if your USB CDC is connected to power bank or battery, etc., instead of a valid USB host (for example, a PC), the power consumption will not increase. + This is because we want to keep the USB Serial/JTAG device alive during software reset by default. + + However there is an issue that this might also increase the power consumption in sleep modes. This is because the software keeps a clock source on during the reset to keep the USB Serial/JTAG device alive. As a side-effect, the clock is also kept on during sleep modes. There is one exception: the clock will only be kept on when your USB Serial/JTAG port is really in use (like data transaction), therefore, if your USB Serial/JTAG is connected to power bank or battery, etc., instead of a valid USB host (for example, a PC), the power consumption will not increase. If you still want to keep low power consumption in sleep modes: - 1. If you are not using the USB CDC port, you don't need to do anything. Software will detect if the CDC device is connected to a valid host before going to sleep, and keep the clocks only when the host is connected. Otherwise the clocks will be turned off as normal. + 1. If you are not using the USB Serial/JTAG port, you don't need to do anything. Software will detect if the USB Serial/JTAG is connected to a valid host before going to sleep, and keep the clocks only when the host is connected. Otherwise the clocks will be turned off as normal. - 2. If you are using the USB CDC port, please disable the menuconfig option ``CONFIG_RTC_CLOCK_BBPLL_POWER_ON_WITH_USB``. The clock will be switched off as normal during software reset and in sleep modes. In these cases, the USB CDC device may be unplugged from the host. + 2. If you are using the USB Serial/JTAG port, please disable the menuconfig option ``CONFIG_RTC_CLOCK_BBPLL_POWER_ON_WITH_USB``. The clock will be switched off as normal during software reset and in sleep modes. In these cases, the USB Serial/JTAG device may be unplugged from the host. \ No newline at end of file From 720b8d9c1a741784cdcf73ca3cef0afb3ef92208 Mon Sep 17 00:00:00 2001 From: Cao Sen Miao Date: Wed, 22 Feb 2023 14:39:06 +0800 Subject: [PATCH 2/2] usb_serial_jtag: Fix bug of blocking TX xfer when using driver, Merges https://github.com/espressif/esp-idf/pull/10208 --- .../driver/usb_serial_jtag/usb_serial_jtag.c | 29 +++---------------- components/vfs/vfs_usb_serial_jtag.c | 15 +++++++++- .../en/api-guides/usb-serial-jtag-console.rst | 4 +-- 3 files changed, 20 insertions(+), 28 deletions(-) diff --git a/components/driver/usb_serial_jtag/usb_serial_jtag.c b/components/driver/usb_serial_jtag/usb_serial_jtag.c index b113beca1f90..e1f1354c2aba 100644 --- a/components/driver/usb_serial_jtag/usb_serial_jtag.c +++ b/components/driver/usb_serial_jtag/usb_serial_jtag.c @@ -32,7 +32,6 @@ typedef struct{ // TX parameters uint32_t tx_buf_size; /*!< TX buffer size */ RingbufHandle_t tx_ring_buf; /*!< TX ring buffer handler */ - bool tx_has_tried_blocking; /*!< TX have we tried a blocking send already? */ } usb_serial_jtag_obj_t; static usb_serial_jtag_obj_t *p_usb_serial_jtag_obj = NULL; @@ -162,31 +161,11 @@ 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(p_usb_serial_jtag_obj != NULL, ESP_ERR_INVALID_ARG, USB_SERIAL_JTAG_TAG, "The driver hasn't been initialized"); - usb_serial_jtag_obj_t* sjtag = p_usb_serial_jtag_obj; - - // try to send without blocking - if (xRingbufferSend(sjtag->tx_ring_buf, (void*) (src), size, 0)) { - goto success; - } - - // If the ring buffer is full, try to send again with a short - // blocking delay, hoping for the buffer to become available soon. - // If we still fail, dont ever block again until the buffer has space again. - if (sjtag->tx_has_tried_blocking == false) { - if (xRingbufferSend(sjtag->tx_ring_buf, (void*) (src), size, ticks_to_wait)) { - goto success; - } else { - sjtag->tx_has_tried_blocking = true; - } - } - - // tx failure - return 0; - -success: - // Now trigger the ISR to read more data from the ring buffer. + const uint8_t *buff = (const uint8_t *)src; + // Blocking method, Sending data to ringbuffer, and handle the data in ISR. + xRingbufferSend(p_usb_serial_jtag_obj->tx_ring_buf, (void*) (buff), size, ticks_to_wait); + // Now trigger the ISR to read data from the ring buffer. usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); - sjtag->tx_has_tried_blocking = false; // clear return size; } diff --git a/components/vfs/vfs_usb_serial_jtag.c b/components/vfs/vfs_usb_serial_jtag.c index b5427d394be3..d17a2a4d6c76 100644 --- a/components/vfs/vfs_usb_serial_jtag.c +++ b/components/vfs/vfs_usb_serial_jtag.c @@ -77,6 +77,8 @@ typedef struct { // When the driver is used (via esp_vfs_usb_serial_jtag_use_driver), // reads are either blocking or non-blocking depending on this flag. bool non_blocking; + // TX has already tried a blocking send. + bool tx_tried_blocking; // Newline conversion mode when transmitting esp_line_endings_t tx_mode; // Newline conversion mode when receiving @@ -405,7 +407,18 @@ static void usbjtag_tx_char_via_driver(int fd, int c) { char ch = (char) c; TickType_t ticks = (TX_FLUSH_TIMEOUT_US / 1000) / portTICK_PERIOD_MS; - usb_serial_jtag_write_bytes(&ch, 1, ticks); + if (usb_serial_jtag_write_bytes(&ch, 1, 0) != 0) { + s_ctx.tx_tried_blocking = false; + return; + } + + if (s_ctx.tx_tried_blocking == false) { + if (usb_serial_jtag_write_bytes(&ch, 1, ticks) != 0) { + return; + } else { + s_ctx.tx_tried_blocking = true; + } + } } void esp_vfs_usb_serial_jtag_use_nonblocking(void) diff --git a/docs/en/api-guides/usb-serial-jtag-console.rst b/docs/en/api-guides/usb-serial-jtag-console.rst index 8c7eb1620f38..fcb9a3e7fbd1 100644 --- a/docs/en/api-guides/usb-serial-jtag-console.rst +++ b/docs/en/api-guides/usb-serial-jtag-console.rst @@ -61,9 +61,9 @@ There are several limitations to the USB Serial/JTAG console feature. These may 2. If the application enters deep sleep mode, the USB Serial/JTAG device will disappear from the system. -3. For data sent in the direction of ESP32 to PC Terminal (e.g. stdout, logs), the ESP32 first writes to a small internal buffer. If this buffer becomes full (for example, if no PC Terminal is connected), the ESP32 will do a one-time wait of 50ms hoping for the PC Terminal to request the data. This can appear as a very brief 'pause' in your application. +3. For data sent in the direction of {IDF_TARGET_NAME} to PC Terminal (e.g. stdout, logs), the {IDF_TARGET_NAME} first writes to a small internal buffer. If this buffer becomes full (for example, if no PC Terminal is connected), the {IDF_TARGET_NAME} will do a one-time wait of 50ms hoping for the PC Terminal to request the data. This can appear as a very brief 'pause' in your application. -4. For data sent in the PC Terminal to ESP32 direction (e.g. console commands), many PC Terminals will wait for the ESP32 to ingest the bytes before allowing you to sending more data. This is in contrast to using a USB-to-Serial (UART) bridge chip, which will always ingest the bytes and send them to a (possibly not listening) ESP32. +4. For data sent in the PC Terminal to {IDF_TARGET_NAME} direction (e.g. console commands), many PC Terminals will wait for the {IDF_TARGET_NAME} to ingest the bytes before allowing you to sending more data. This is in contrast to using a USB-to-Serial (UART) bridge chip, which will always ingest the bytes and send them to a (possibly not listening) {IDF_TARGET_NAME}. 5. The USB Serial/JTAG device won't work in sleep modes as normal due to the lack of APB clock in sleep modes. This includes deep-sleep, light-sleep (automataic light-sleep as well).