From a9c11247632b9eff1ae5f4eb6a390570d48101a3 Mon Sep 17 00:00:00 2001
From: kooho <2229179028@qq.com>
Date: Fri, 19 Oct 2018 14:51:28 +0800
Subject: [PATCH] driver(uart): fixed uart read error bug when using dual core.

closes https://github.com/espressif/esp-idf/issues/2204
---
 components/driver/uart.c | 42 +++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/components/driver/uart.c b/components/driver/uart.c
index 0063007df..afead15a5 100644
--- a/components/driver/uart.c
+++ b/components/driver/uart.c
@@ -875,6 +875,7 @@ static void uart_rx_intr_handler_default(void *param)
                 //If we fail to push data to ring buffer, we will have to stash the data, and send next time.
                 //Mainly for applications that uses flow control or small ring buffer.
                 if(pdFALSE == xRingbufferSendFromISR(p_uart->rx_ring_buf, p_uart->rx_data_buf, p_uart->rx_stash_len, &HPTaskAwoken)) {
+                    p_uart->rx_buffer_full_flg = true;
                     uart_disable_intr_mask(uart_num, UART_RXFIFO_TOUT_INT_ENA_M | UART_RXFIFO_FULL_INT_ENA_M);
                     if (uart_event.type == UART_PATTERN_DET) {
                         if (rx_fifo_len < pat_num) {
@@ -893,7 +894,6 @@ static void uart_rx_intr_handler_default(void *param)
                         }
                     }
                     uart_event.type = UART_BUFFER_FULL;
-                    p_uart->rx_buffer_full_flg = true;
                 } else {
                     UART_ENTER_CRITICAL_ISR(&uart_spinlock[uart_num]);
                     if (uart_intr_status & UART_AT_CMD_CHAR_DET_INT_ST_M) {
@@ -1164,6 +1164,22 @@ int uart_write_bytes_with_break(uart_port_t uart_num, const char* src, size_t si
     return uart_tx_all(uart_num, src, size, 1, brk_len);
 }
 
+static bool uart_check_buf_full(uart_port_t uart_num)
+{
+    if(p_uart_obj[uart_num]->rx_buffer_full_flg) {
+        BaseType_t res = xRingbufferSend(p_uart_obj[uart_num]->rx_ring_buf, p_uart_obj[uart_num]->rx_data_buf, p_uart_obj[uart_num]->rx_stash_len, 1);
+        if(res == pdTRUE) {
+            UART_ENTER_CRITICAL(&uart_spinlock[uart_num]);
+            p_uart_obj[uart_num]->rx_buffered_len += p_uart_obj[uart_num]->rx_stash_len;
+            p_uart_obj[uart_num]->rx_buffer_full_flg = false;
+            UART_EXIT_CRITICAL(&uart_spinlock[uart_num]);
+            uart_enable_rx_intr(p_uart_obj[uart_num]->uart_num);
+            return true;
+        }
+    }
+    return false;
+}
+
 int uart_read_bytes(uart_port_t uart_num, uint8_t* buf, uint32_t length, TickType_t ticks_to_wait)
 {
     UART_CHECK((uart_num < UART_NUM_MAX), "uart_num error", (-1));
@@ -1184,8 +1200,17 @@ int uart_read_bytes(uart_port_t uart_num, uint8_t* buf, uint32_t length, TickTyp
                 p_uart_obj[uart_num]->rx_ptr = data;
                 p_uart_obj[uart_num]->rx_cur_remain = size;
             } else {
-                xSemaphoreGive(p_uart_obj[uart_num]->rx_mux);
-                return copy_len;
+                //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;
+                }
             }
         }
         if(p_uart_obj[uart_num]->rx_cur_remain > length) {
@@ -1206,16 +1231,7 @@ int uart_read_bytes(uart_port_t uart_num, uint8_t* buf, uint32_t length, TickTyp
             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;
-            if(p_uart_obj[uart_num]->rx_buffer_full_flg) {
-                BaseType_t res = xRingbufferSend(p_uart_obj[uart_num]->rx_ring_buf, p_uart_obj[uart_num]->rx_data_buf, p_uart_obj[uart_num]->rx_stash_len, 1);
-                if(res == pdTRUE) {
-                    UART_ENTER_CRITICAL(&uart_spinlock[uart_num]);
-                    p_uart_obj[uart_num]->rx_buffered_len += p_uart_obj[uart_num]->rx_stash_len;
-                    p_uart_obj[uart_num]->rx_buffer_full_flg = false;
-                    UART_EXIT_CRITICAL(&uart_spinlock[uart_num]);
-                    uart_enable_rx_intr(p_uart_obj[uart_num]->uart_num);
-                }
-            }
+            uart_check_buf_full(uart_num);
         }
     }