From 0da4b0867b732ff6baba3b5c3f956792ebe426d7 Mon Sep 17 00:00:00 2001 From: Song Ruo Jing Date: Tue, 21 Nov 2023 14:52:55 +0800 Subject: [PATCH] change(uart): improved the internal logic of uart_read_bytes Ringbuffer usage becomes more efficient with the use of xRingbufferReceiveUpTo Closes https://github.com/espressif/esp-idf/issues/12386 --- components/driver/uart/uart.c | 85 ++++++++++------------------------- 1 file changed, 24 insertions(+), 61 deletions(-) diff --git a/components/driver/uart/uart.c b/components/driver/uart/uart.c index be1f6e1aca..690bb4dd17 100644 --- a/components/driver/uart/uart.c +++ b/components/driver/uart/uart.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -109,9 +109,6 @@ typedef struct { int rx_buffered_len; /*!< UART cached data length */ int rx_buf_size; /*!< RX ring buffer size */ bool rx_buffer_full_flg; /*!< RX ring buffer full flag. */ - uint32_t rx_cur_remain; /*!< Data number that waiting to be read out in ring buffer item*/ - uint8_t *rx_ptr; /*!< pointer to the current data in ring buffer*/ - uint8_t *rx_head_ptr; /*!< pointer to the head of RX item*/ uint8_t rx_data_buf[SOC_UART_FIFO_LEN]; /*!< Data buffer to stash FIFO data*/ uint8_t rx_stash_len; /*!< stashed data length.(When using flow control, after reading out FIFO data, if we fail to push to buffer, we can just stash them.) */ uint32_t rx_int_usr_mask; /*!< RX interrupt status. Valid at any time, regardless of RX buffer status. */ @@ -1268,53 +1265,35 @@ int uart_read_bytes(uart_port_t uart_num, void *buf, uint32_t length, TickType_t ESP_RETURN_ON_FALSE((buf), (-1), UART_TAG, "uart data null"); ESP_RETURN_ON_FALSE((p_uart_obj[uart_num]), (-1), UART_TAG, "uart driver error"); uint8_t *data = NULL; - size_t size; + size_t size = 0; size_t copy_len = 0; - int len_tmp; if (xSemaphoreTake(p_uart_obj[uart_num]->rx_mux, (TickType_t)ticks_to_wait) != pdTRUE) { return -1; } while (length) { - if (p_uart_obj[uart_num]->rx_cur_remain == 0) { - data = (uint8_t *) xRingbufferReceive(p_uart_obj[uart_num]->rx_ring_buf, &size, (TickType_t) ticks_to_wait); - if (data) { - p_uart_obj[uart_num]->rx_head_ptr = data; - p_uart_obj[uart_num]->rx_ptr = data; - p_uart_obj[uart_num]->rx_cur_remain = size; + data = (uint8_t *) xRingbufferReceiveUpTo(p_uart_obj[uart_num]->rx_ring_buf, &size, (TickType_t) ticks_to_wait, length); + if (!data) { + // When using dual cores, `rx_buffer_full_flg` may read and write on different cores at same time, + // which may lose synchronization. So we also need to call `uart_check_buf_full` once when ringbuffer is empty + // to solve the possible asynchronous issues. + if (uart_check_buf_full(uart_num)) { + // This condition will never be true if `uart_read_bytes` + // and `uart_rx_intr_handler_default` are scheduled on the same core. + continue; } else { - //When using dual cores, `rx_buffer_full_flg` may read and write on different cores at same time, - //which may lose synchronization. So we also need to call `uart_check_buf_full` once when ringbuffer is empty - //to solve the possible asynchronous issues. - if (uart_check_buf_full(uart_num)) { - //This condition will never be true if `uart_read_bytes` - //and `uart_rx_intr_handler_default` are scheduled on the same core. - continue; - } else { - xSemaphoreGive(p_uart_obj[uart_num]->rx_mux); - return copy_len; - } + // Timeout while not fetched all requested length + break; } } - if (p_uart_obj[uart_num]->rx_cur_remain > length) { - len_tmp = length; - } else { - len_tmp = p_uart_obj[uart_num]->rx_cur_remain; - } - memcpy((uint8_t *)buf + copy_len, p_uart_obj[uart_num]->rx_ptr, len_tmp); + memcpy((uint8_t *)buf + copy_len, data, size); UART_ENTER_CRITICAL(&(uart_context[uart_num].spinlock)); - p_uart_obj[uart_num]->rx_buffered_len -= len_tmp; - uart_pattern_queue_update(uart_num, len_tmp); - p_uart_obj[uart_num]->rx_ptr += len_tmp; + p_uart_obj[uart_num]->rx_buffered_len -= size; + uart_pattern_queue_update(uart_num, size); UART_EXIT_CRITICAL(&(uart_context[uart_num].spinlock)); - p_uart_obj[uart_num]->rx_cur_remain -= len_tmp; - copy_len += len_tmp; - length -= len_tmp; - if (p_uart_obj[uart_num]->rx_cur_remain == 0) { - vRingbufferReturnItem(p_uart_obj[uart_num]->rx_ring_buf, p_uart_obj[uart_num]->rx_head_ptr); - p_uart_obj[uart_num]->rx_head_ptr = NULL; - p_uart_obj[uart_num]->rx_ptr = NULL; - uart_check_buf_full(uart_num); - } + copy_len += size; + length -= size; + vRingbufferReturnItem(p_uart_obj[uart_num]->rx_ring_buf, data); + uart_check_buf_full(uart_num); } xSemaphoreGive(p_uart_obj[uart_num]->rx_mux); @@ -1356,25 +1335,15 @@ esp_err_t uart_flush_input(uart_port_t uart_num) uart_hal_disable_intr_mask(&(uart_context[uart_num].hal), UART_INTR_RXFIFO_FULL | UART_INTR_RXFIFO_TOUT); UART_EXIT_CRITICAL(&(uart_context[uart_num].spinlock)); while (true) { - if (p_uart->rx_head_ptr) { - vRingbufferReturnItem(p_uart->rx_ring_buf, p_uart->rx_head_ptr); - UART_ENTER_CRITICAL(&(uart_context[uart_num].spinlock)); - p_uart_obj[uart_num]->rx_buffered_len -= p_uart->rx_cur_remain; - uart_pattern_queue_update(uart_num, p_uart->rx_cur_remain); - UART_EXIT_CRITICAL(&(uart_context[uart_num].spinlock)); - p_uart->rx_ptr = NULL; - p_uart->rx_cur_remain = 0; - p_uart->rx_head_ptr = NULL; - } - data = (uint8_t*) xRingbufferReceive(p_uart->rx_ring_buf, &size, (TickType_t) 0); - if(data == NULL) { + data = (uint8_t *) xRingbufferReceive(p_uart->rx_ring_buf, &size, (TickType_t) 0); + if (data == NULL) { bool error = false; UART_ENTER_CRITICAL(&(uart_context[uart_num].spinlock)); - if( p_uart_obj[uart_num]->rx_buffered_len != 0 ) { + if (p_uart_obj[uart_num]->rx_buffered_len != 0) { p_uart_obj[uart_num]->rx_buffered_len = 0; error = true; } - //We also need to clear the `rx_buffer_full_flg` here. + // We also need to clear the `rx_buffer_full_flg` here. p_uart_obj[uart_num]->rx_buffer_full_flg = false; UART_EXIT_CRITICAL(&(uart_context[uart_num].spinlock)); if (error) { @@ -1398,9 +1367,6 @@ esp_err_t uart_flush_input(uart_port_t uart_num) } } } - p_uart->rx_ptr = NULL; - p_uart->rx_cur_remain = 0; - p_uart->rx_head_ptr = NULL; uart_hal_rxfifo_rst(&(uart_context[uart_num].hal)); /* Only re-enable UART_INTR_RXFIFO_TOUT or UART_INTR_RXFIFO_FULL if they * were explicitly enabled by the user. */ @@ -1581,10 +1547,7 @@ esp_err_t uart_driver_install(uart_port_t uart_num, int rx_buffer_size, int tx_b p_uart_obj[uart_num]->rx_buffered_len = 0; p_uart_obj[uart_num]->rx_buffer_full_flg = false; p_uart_obj[uart_num]->tx_waiting_fifo = false; - p_uart_obj[uart_num]->rx_ptr = NULL; - p_uart_obj[uart_num]->rx_cur_remain = 0; p_uart_obj[uart_num]->rx_int_usr_mask = UART_INTR_RXFIFO_FULL | UART_INTR_RXFIFO_TOUT; - p_uart_obj[uart_num]->rx_head_ptr = NULL; p_uart_obj[uart_num]->tx_buf_size = tx_buffer_size; p_uart_obj[uart_num]->uart_select_notif_callback = NULL; xSemaphoreGive(p_uart_obj[uart_num]->tx_fifo_sem);