bugfix(UART): fixed two UART issues:

1. uart_wait_tx_done works incorrect when sending a byte of data.
2. uart_set_rx_timeout sets an incorrect rx timeout value when ref_tick is enabled

diff --git a/components/driver/uart.c b/components/driver/uart.c
index de872d881d56183bca44d70f6a4850ebc6dd1773..188be1418dc0de670728db612991e9c3865b7342 100644 (file)
--- a/components/driver/uart.c
+++ b/components/driver/uart.c
@@ -691,9 +691,9 @@ esp_err_t uart_intr_config(uart_port_t uart_num, const uart_intr_config_t *intr_
         //Hardware issue workaround: when using ref_tick, the rx timeout threshold needs increase to 10 times.
         //T_ref = T_apb * APB_CLK/(REF_TICK << CLKDIV_FRAG_BIT_WIDTH)
         if(UART[uart_num]->conf0.tick_ref_always_on == 0) {
-            UART[uart_num]->conf1.rx_tout_thrhd = ((intr_conf->rx_timeout_thresh * UART_TOUT_REF_FACTOR_DEFAULT) & UART_RX_TOUT_THRHD_V);
+            UART[uart_num]->conf1.rx_tout_thrhd = (intr_conf->rx_timeout_thresh * UART_TOUT_REF_FACTOR_DEFAULT);
         } else {
-            UART[uart_num]->conf1.rx_tout_thrhd = ((intr_conf->rx_timeout_thresh) & UART_RX_TOUT_THRHD_V);
+            UART[uart_num]->conf1.rx_tout_thrhd = intr_conf->rx_timeout_thresh;
         }
         UART[uart_num]->conf1.rx_tout_en = 1;
     } else {
@@ -734,14 +734,19 @@ static void uart_rx_intr_handler_default(void *param)
     uart_obj_t *p_uart = (uart_obj_t*) param;
     uint8_t uart_num = p_uart->uart_num;
     uart_dev_t* uart_reg = UART[uart_num];
-    int rx_fifo_len = uart_reg->status.rxfifo_cnt;
+    int rx_fifo_len = 0;
     uint8_t buf_idx = 0;
-    uint32_t uart_intr_status = UART[uart_num]->int_st.val;
+    uint32_t uart_intr_status = 0;
     uart_event_t uart_event;
     portBASE_TYPE HPTaskAwoken = 0;
     static uint8_t pat_flg = 0;
-    while(uart_intr_status != 0x0) {
-        buf_idx = 0;
+    while(1) {
+        uart_intr_status = uart_reg->int_st.val;
+        // The `continue statement` may cause the interrupt to loop infinitely
+        // we exit the interrupt here
+        if(uart_intr_status == 0) {
+            break;
+        }
         uart_event.type = UART_EVENT_MAX;
         if(uart_intr_status & UART_TXFIFO_EMPTY_INT_ST_M) {
             uart_clear_intr_status(uart_num, UART_TXFIFO_EMPTY_INT_CLR_M);
@@ -753,15 +758,12 @@ static void uart_rx_intr_handler_default(void *param)
             if(p_uart->tx_waiting_fifo == true && p_uart->tx_buf_size == 0) {
                 p_uart->tx_waiting_fifo = false;
                 xSemaphoreGiveFromISR(p_uart->tx_fifo_sem, &HPTaskAwoken);
-                if(HPTaskAwoken == pdTRUE) {
-                    portYIELD_FROM_ISR();
-                }
             } else {
                 //We don't use TX ring buffer, because the size is zero.
                 if(p_uart->tx_buf_size == 0) {
                     continue;
                 }
-                int tx_fifo_rem = UART_FIFO_LEN - UART[uart_num]->status.txfifo_cnt;
+                int tx_fifo_rem = UART_FIFO_LEN - uart_reg->status.txfifo_cnt;
                 bool en_tx_flg = false;
                 //We need to put a loop here, in case all the buffer items are very short.
                 //That would cause a watch_dog reset because empty interrupt happens so often.
@@ -782,9 +784,6 @@ static void uart_rx_intr_handler_default(void *param)
                                 }
                                 //We have saved the data description from the 1st item, return buffer.
                                 vRingbufferReturnItemFromISR(p_uart->tx_ring_buf, p_uart->tx_head, &HPTaskAwoken);
-                                if(HPTaskAwoken == pdTRUE) {
-                                    portYIELD_FROM_ISR();
-                                }
                             }else if(p_uart->tx_ptr == NULL) {
                                 //Update the TX item pointer, we will need this to return item to buffer.
                                 p_uart->tx_ptr =  (uint8_t*) p_uart->tx_head;
@@ -817,9 +816,6 @@ static void uart_rx_intr_handler_default(void *param)
                         if (p_uart->tx_len_cur == 0) {
                             //Return item to ring buffer.
                             vRingbufferReturnItemFromISR(p_uart->tx_ring_buf, p_uart->tx_head, &HPTaskAwoken);
-                            if(HPTaskAwoken == pdTRUE) {
-                                portYIELD_FROM_ISR();
-                            }
                             p_uart->tx_head = NULL;
                             p_uart->tx_ptr = NULL;
                             //Sending item done, now we need to send break if there is a record.
@@ -862,8 +858,8 @@ static void uart_rx_intr_handler_default(void *param)
             }
             if (p_uart->rx_buffer_full_flg == false) {
                 //We have to read out all data in RX FIFO to clear the interrupt signal
-                while (buf_idx < rx_fifo_len) {
-                    p_uart->rx_data_buf[buf_idx++] = uart_reg->fifo.rw_byte;
+                for(buf_idx = 0; buf_idx < rx_fifo_len; buf_idx++) {
+                    p_uart->rx_data_buf[buf_idx] = uart_reg->fifo.rw_byte;
                 }
                 uint8_t pat_chr = uart_reg->at_cmd_char.data;
                 int pat_num = uart_reg->at_cmd_char.char_num;
@@ -923,9 +919,6 @@ static void uart_rx_intr_handler_default(void *param)
                     p_uart->rx_buffered_len += p_uart->rx_stash_len;
                     UART_EXIT_CRITICAL_ISR(&uart_spinlock[uart_num]);
                 }
-                if(HPTaskAwoken == pdTRUE) {
-                    portYIELD_FROM_ISR();
-                }
             } else {
                 uart_disable_intr_mask_from_isr(uart_num, UART_RXFIFO_FULL_INT_ENA_M | UART_RXFIFO_TOUT_INT_ENA_M);
                 uart_clear_intr_status(uart_num, UART_RXFIFO_FULL_INT_CLR_M | UART_RXFIFO_TOUT_INT_CLR_M);
@@ -981,9 +974,6 @@ static void uart_rx_intr_handler_default(void *param)
                 p_uart->tx_waiting_brk = 0;
             } else {
                 xSemaphoreGiveFromISR(p_uart->tx_brk_sem, &HPTaskAwoken);
-                if(HPTaskAwoken == pdTRUE) {
-                    portYIELD_FROM_ISR();
-                }
             }
         } else if(uart_intr_status & UART_TX_BRK_IDLE_DONE_INT_ST_M) {
             uart_disable_intr_mask_from_isr(uart_num, UART_TX_BRK_IDLE_DONE_INT_ENA_M);
@@ -1014,9 +1004,6 @@ static void uart_rx_intr_handler_default(void *param)
                 UART_EXIT_CRITICAL_ISR(&uart_spinlock[uart_num]);
             }
             xSemaphoreGiveFromISR(p_uart_obj[uart_num]->tx_done_sem, &HPTaskAwoken);
-            if (HPTaskAwoken == pdTRUE) {
-                portYIELD_FROM_ISR();
-            }
         } else {
             uart_reg->int_clr.val = uart_intr_status; /*simply clear all other intr status*/
             uart_event.type = UART_EVENT_MAX;
@@ -1026,11 +1013,10 @@ static void uart_rx_intr_handler_default(void *param)
             if (pdFALSE == xQueueSendFromISR(p_uart->xQueueUart, (void * )&uart_event, &HPTaskAwoken)) {
                 ESP_EARLY_LOGV(UART_TAG, "UART event queue full");
             }
-            if(HPTaskAwoken == pdTRUE) {
-                portYIELD_FROM_ISR();
-            }
         }
-        uart_intr_status = uart_reg->int_st.val;
+    }
+    if(HPTaskAwoken == pdTRUE) {
+        portYIELD_FROM_ISR();
     }
 }
 
@@ -1040,20 +1026,27 @@ esp_err_t uart_wait_tx_done(uart_port_t uart_num, TickType_t ticks_to_wait)
     UART_CHECK((uart_num < UART_NUM_MAX), "uart_num error", ESP_FAIL);
     UART_CHECK((p_uart_obj[uart_num]), "uart driver error", ESP_FAIL);
     BaseType_t res;
-    portTickType ticks_end = xTaskGetTickCount() + ticks_to_wait;
+    portTickType ticks_start = xTaskGetTickCount();
     //Take tx_mux
     res = xSemaphoreTake(p_uart_obj[uart_num]->tx_mux, (portTickType)ticks_to_wait);
     if(res == pdFALSE) {
         return ESP_ERR_TIMEOUT;
     }
-    ticks_to_wait = ticks_end - xTaskGetTickCount();
     xSemaphoreTake(p_uart_obj[uart_num]->tx_done_sem, 0);
-    ticks_to_wait = ticks_end - xTaskGetTickCount();
-    if(UART[uart_num]->status.txfifo_cnt == 0) {
+    typeof(UART0.status) status = UART[uart_num]->status;
+    //Wait txfifo_cnt = 0, and the transmitter state machine is in idle state.
+    if(status.txfifo_cnt == 0 && status.st_utx_out == 0) {
         xSemaphoreGive(p_uart_obj[uart_num]->tx_mux);
         return ESP_OK;
     }
     uart_enable_intr_mask(uart_num, UART_TX_DONE_INT_ENA_M);
+
+    TickType_t ticks_end = xTaskGetTickCount();
+    if (ticks_end - ticks_start > ticks_to_wait) {
+        ticks_to_wait = 0;
+    } else {
+        ticks_to_wait = ticks_to_wait - (ticks_end - ticks_start);
+    }
     //take 2nd tx_done_sem, wait given from ISR
     res = xSemaphoreTake(p_uart_obj[uart_num]->tx_done_sem, (portTickType)ticks_to_wait);
     if(res == pdFALSE) {
@@ -1543,7 +1536,13 @@ esp_err_t uart_set_rx_timeout(uart_port_t uart_num, const uint8_t tout_thresh)
     // The tout_thresh = 1, defines TOUT interrupt timeout equal to  
     // transmission time of one symbol (~11 bit) on current baudrate  
     if (tout_thresh > 0) {
-        UART[uart_num]->conf1.rx_tout_thrhd = (tout_thresh & UART_RX_TOUT_THRHD_V);
+        //Hardware issue workaround: when using ref_tick, the rx timeout threshold needs increase to 10 times.
+        //T_ref = T_apb * APB_CLK/(REF_TICK << CLKDIV_FRAG_BIT_WIDTH)
+        if(UART[uart_num]->conf0.tick_ref_always_on == 0) {
+            UART[uart_num]->conf1.rx_tout_thrhd = tout_thresh * UART_TOUT_REF_FACTOR_DEFAULT;
+        } else {
+            UART[uart_num]->conf1.rx_tout_thrhd = tout_thresh;
+        }
         UART[uart_num]->conf1.rx_tout_en = 1;
     } else {
         UART[uart_num]->conf1.rx_tout_en = 0;