From 6c363a4075eabc02642ce53ba5cf538923ded008 Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Mon, 13 Sep 2021 23:18:25 +0800 Subject: [PATCH] uart: Add missing critical section wrappers around rx_buffered_len The missing barriers caused uart_get_buffered_data_len() to (very rarely) return a garbage value. When used in MicroPython, though, this caused select() to return and a subsequent read() to stall indefinitely until a char was actually available. Signed-off-by: Chen Yi Qun Closes https://github.com/espressif/esp-idf/issues/6397 Merges https://github.com/espressif/esp-idf/pull/6396 --- components/driver/uart.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/components/driver/uart.c b/components/driver/uart.c index e9ece99175..8001e9acaf 100644 --- a/components/driver/uart.c +++ b/components/driver/uart.c @@ -257,7 +257,9 @@ esp_err_t uart_set_stop_bits(uart_port_t uart_num, uart_stop_bits_t stop_bit) esp_err_t uart_get_stop_bits(uart_port_t uart_num, uart_stop_bits_t* stop_bit) { UART_CHECK((uart_num < UART_NUM_MAX), "uart_num error", ESP_FAIL); + UART_ENTER_CRITICAL(&(uart_context[uart_num].spinlock)); uart_hal_get_stop_bits(&(uart_context[uart_num].hal), stop_bit); + UART_EXIT_CRITICAL(&(uart_context[uart_num].spinlock)); return ESP_OK; } @@ -273,7 +275,9 @@ esp_err_t uart_set_parity(uart_port_t uart_num, uart_parity_t parity_mode) esp_err_t uart_get_parity(uart_port_t uart_num, uart_parity_t* parity_mode) { UART_CHECK((uart_num < UART_NUM_MAX), "uart_num error", ESP_FAIL); + UART_ENTER_CRITICAL(&(uart_context[uart_num].spinlock)); uart_hal_get_parity(&(uart_context[uart_num].hal), parity_mode); + UART_EXIT_CRITICAL(&(uart_context[uart_num].spinlock)); return ESP_OK; } @@ -1262,7 +1266,9 @@ esp_err_t uart_get_buffered_data_len(uart_port_t uart_num, size_t* size) { UART_CHECK((uart_num < UART_NUM_MAX), "uart_num error", ESP_FAIL); UART_CHECK((p_uart_obj[uart_num]), "uart driver error", ESP_FAIL); + UART_ENTER_CRITICAL(&(uart_context[uart_num].spinlock)); *size = p_uart_obj[uart_num]->rx_buffered_len; + UART_EXIT_CRITICAL(&(uart_context[uart_num].spinlock)); return ESP_OK; } @@ -1303,14 +1309,19 @@ esp_err_t uart_flush_input(uart_port_t uart_num) } data = (uint8_t*) xRingbufferReceive(p_uart->rx_ring_buf, &size, (portTickType) 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 ) { - ESP_LOGE(UART_TAG, "rx_buffered_len error"); p_uart_obj[uart_num]->rx_buffered_len = 0; + error = true; } //We also need to clear the `rx_buffer_full_flg` here. - UART_ENTER_CRITICAL(&(uart_context[uart_num].spinlock)); p_uart_obj[uart_num]->rx_buffer_full_flg = false; UART_EXIT_CRITICAL(&(uart_context[uart_num].spinlock)); + if (error) { + // this must be called outside the critical section + ESP_LOGE(UART_TAG, "rx_buffered_len error"); + } break; } UART_ENTER_CRITICAL(&(uart_context[uart_num].spinlock));