From 7e2465995f45777548948d425d274b2025bffe16 Mon Sep 17 00:00:00 2001 From: kooho <2229179028@qq.com> Date: Wed, 12 Jun 2019 16:20:19 +0800 Subject: [PATCH] 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. --- components/driver/uart.c | 71 ++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 36 deletions(-) diff --git a/components/driver/uart.c b/components/driver/uart.c index afead15a5..597993dbb 100644 --- a/components/driver/uart.c +++ b/components/driver/uart.c @@ -676,9 +676,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 { @@ -719,14 +719,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); @@ -738,15 +743,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. @@ -767,9 +769,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; @@ -802,9 +801,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. @@ -847,8 +843,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; @@ -908,9 +904,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(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); @@ -966,9 +959,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(uart_num, UART_TX_BRK_IDLE_DONE_INT_ENA_M); @@ -999,9 +989,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; @@ -1011,11 +998,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(); } } @@ -1025,20 +1011,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) { @@ -1528,7 +1521,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;