Crash_Override/OVMS3-idf
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

bugfix(UART): fix uart driver spinlock misused bug

Browse source 
1. fix uart driver spinlock misused bug
    2. add uart driver ut test case
    3. undo the change in light_sleep_example_main.c
This commit is contained in:
houwenxiang 2019-11-27 21:32:52 +08:00 committed by kooho
parent 7c8139734d
commit e4230d11ca
6 changed files with 223 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

197
components/driver/test/test_uart.c
View file

@ -38,7 +38,7 @@ static void uart_config(uint32_t baud_rate, bool use_ref_tick)
uart_config.source_clk = use_ref_tick ? UART_SCLK_REF_TICK : UART_SCLK_APB;
uart_driver_install(UART_NUM1, BUF_SIZE * 2, BUF_SIZE * 2, 20, NULL, 0);
uart_param_config(UART_NUM1, &uart_config);
uart_set_pin(UART_NUM1, UART1_TX_PIN, UART1_RX_PIN, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE);
TEST_ESP_OK(uart_set_loop_back(UART_NUM1, true));
}
static volatile bool exit_flag;
@ -126,3 +126,198 @@ TEST_CASE("test uart tx data with break", "[uart]")
free(psend);
uart_driver_delete(UART_NUM1);
}
static void uart_word_len_set_get_test(int uart_num)
{
printf("uart word len set and get test\n");
uart_word_length_t word_length_set = 0;
uart_word_length_t word_length_get = 0;
for (int i = 0; i < UART_DATA_BITS_MAX; i++) {
word_length_set = UART_DATA_5_BITS + i;
TEST_ESP_OK(uart_set_word_length(uart_num, word_length_set));
TEST_ESP_OK(uart_get_word_length(uart_num, &word_length_get));
TEST_ASSERT(word_length_set == word_length_get);
}
}
static void uart_stop_bit_set_get_test(int uart_num)
{
printf("uart stop bit set and get test\n");
uart_stop_bits_t stop_bit_set = 0;
uart_stop_bits_t stop_bit_get = 0;
for (int i = UART_STOP_BITS_1; i < UART_STOP_BITS_MAX; i++) {
stop_bit_set = i;
TEST_ESP_OK(uart_set_stop_bits(uart_num, stop_bit_set));
TEST_ESP_OK(uart_get_stop_bits(uart_num, &stop_bit_get));
TEST_ASSERT(stop_bit_set == stop_bit_get);
}
}
static void uart_parity_set_get_test(int uart_num)
{
printf("uart parity set and get test\n");
uart_parity_t parity_set[3] = {
UART_PARITY_DISABLE,
UART_PARITY_EVEN,
UART_PARITY_ODD,
};
uart_parity_t parity_get = 0;
for (int i = 0; i < 3; i++) {
TEST_ESP_OK(uart_set_parity(uart_num, parity_set[i]));
TEST_ESP_OK(uart_get_parity(uart_num, &parity_get));
TEST_ASSERT(parity_set[i] == parity_get);
}
}
static void uart_hw_flow_set_get_test(int uart_num)
{
printf("uart hw flow control set and get test\n");
uart_hw_flowcontrol_t flowcontro_set = 0;
uart_hw_flowcontrol_t flowcontro_get = 0;
for (int i = 0; i < UART_HW_FLOWCTRL_DISABLE; i++) {
TEST_ESP_OK(uart_set_hw_flow_ctrl(uart_num, flowcontro_set, 20));
TEST_ESP_OK(uart_get_hw_flow_ctrl(uart_num, &flowcontro_get));
TEST_ASSERT(flowcontro_set == flowcontro_get);
}
}
static void uart_wakeup_set_get_test(int uart_num)
{
printf("uart wake up set and get test\n");
int wake_up_set = 0;
int wake_up_get = 0;
for (int i = 3; i < 0x3ff; i++) {
wake_up_set = i;
TEST_ESP_OK(uart_set_wakeup_threshold(uart_num, wake_up_set));
TEST_ESP_OK(uart_get_wakeup_threshold(uart_num, &wake_up_get));
TEST_ASSERT(wake_up_set == wake_up_get);
}
}
TEST_CASE("uart general API test", "[uart]")
{
const int uart_num = UART_NUM1;
uart_config_t uart_config = {
.baud_rate = 115200,
.data_bits = UART_DATA_8_BITS,
.parity = UART_PARITY_DISABLE,
.stop_bits = UART_STOP_BITS_1,
.flow_ctrl = UART_HW_FLOWCTRL_DISABLE,
.source_clk = UART_SCLK_APB,
};
uart_param_config(uart_num, &uart_config);
uart_word_len_set_get_test(uart_num);
uart_stop_bit_set_get_test(uart_num);
uart_parity_set_get_test(uart_num);
uart_hw_flow_set_get_test(uart_num);
uart_wakeup_set_get_test(uart_num);
}
static void uart_write_task(void *param)
{
int uart_num = (int)param;
uint8_t *tx_buf = (uint8_t *)malloc(1024);
if(tx_buf == NULL) {
printf("tx buffer malloc fail\n");
TEST_ASSERT(0);
}
for(int i = 1; i < 1023; i++) {
tx_buf[i] = (i & 0xff);
}
for(int i = 0; i < 1024; i++) {
//d[0] and d[1023] are header
tx_buf[0] = (i & 0xff);
tx_buf[1023] = ((~i) & 0xff);
uart_write_bytes(uart_num, (const char*)tx_buf, 1024);
uart_wait_tx_done(uart_num, (TickType_t)portMAX_DELAY);
}
free(tx_buf);
vTaskDelete(NULL);
}
static void uart_read_write_test(void)
{
const int uart_num = UART_NUM1;
uint8_t *rd_data = (uint8_t *)malloc(1024);
if(rd_data == NULL) {
printf("rx buffer malloc fail\n");
TEST_ASSERT(0);
}
uart_config_t uart_config = {
.baud_rate = 2000000,
.data_bits = UART_DATA_8_BITS,
.parity = UART_PARITY_DISABLE,
.stop_bits = UART_STOP_BITS_1,
.flow_ctrl = UART_HW_FLOWCTRL_DISABLE,
.source_clk = UART_SCLK_APB,
};
TEST_ESP_OK(uart_driver_install(uart_num, BUF_SIZE * 2, 0, 20, NULL, 0));
TEST_ESP_OK(uart_param_config(uart_num, &uart_config));
TEST_ESP_OK(uart_set_loop_back(uart_num, true));
xTaskCreate(uart_write_task, "uart_write_task", 2048 * 4, (void *)uart_num, 5, NULL);
int len_tmp = 0;
int rd_len = 1024;
for (int i = 0; i < 1024; i++) {
rd_len = 1024;
memset(rd_data, 0, 1024);
while (rd_len) {
len_tmp = uart_read_bytes(uart_num, rd_data + 1024 - rd_len, rd_len, (TickType_t)1000);
if (len_tmp < 0) {
printf("read timeout, uart read write test fail\n");
TEST_ASSERT(0);
}
rd_len -= len_tmp;
}
if (rd_data[0] != (i & 0xff) || rd_data[1023] != ((~i) & 0xff)) {
printf("uart data header check error\n");
TEST_ASSERT(0);
}
for (int j = 1; j < 1023; j++) {
if (rd_data[j] != (j & 0xff)) {
printf("uart data check error\n");
TEST_ASSERT(0);
}
}
}
uart_wait_tx_done(uart_num, (TickType_t)portMAX_DELAY);
uart_driver_delete(uart_num);
free(rd_data);
}
TEST_CASE("uart read write test", "[uart]")
{
uart_read_write_test();
}
TEST_CASE("uart tx with ringbuffer test", "[uart]")
{
const int uart_num = UART_NUM1;
uint8_t *rd_data = (uint8_t *)malloc(1024);
uint8_t *wr_data = (uint8_t *)malloc(1024);
if(rd_data == NULL || wr_data == NULL) {
printf("buffer malloc fail\n");
TEST_ASSERT(0);
}
uart_config_t uart_config = {
.baud_rate = 2000000,
.data_bits = UART_DATA_8_BITS,
.parity = UART_PARITY_DISABLE,
.stop_bits = UART_STOP_BITS_1,
.flow_ctrl = UART_HW_FLOWCTRL_DISABLE,
.source_clk = UART_SCLK_APB,
};
TEST_ESP_OK(uart_param_config(uart_num, &uart_config));
TEST_ESP_OK(uart_driver_install(uart_num, 1024 * 2, 1024 *2, 20, NULL, 0));
TEST_ESP_OK(uart_set_loop_back(uart_num, true));
for (int i = 0; i < 1024; i++) {
wr_data[i] = i;
rd_data[i] = 0;
}
uart_write_bytes(uart_num, (const char*)wr_data, 1024);
uart_wait_tx_done(uart_num, (TickType_t)portMAX_DELAY);
uart_read_bytes(uart_num, rd_data, 1024, (TickType_t)1000);
TEST_ASSERT(memcmp(wr_data, rd_data, 1024) == 0);
TEST_ESP_OK(uart_driver_delete(uart_num));
free(rd_data);
free(wr_data);
}

3
components/driver/uart.c
View file

@ -879,6 +879,8 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
// When fifo overflows, we reset the fifo.
UART_ENTER_CRITICAL_ISR(&(uart_context[uart_num].spinlock));
uart_hal_rxfifo_rst(&(uart_context[uart_num].hal));
UART_EXIT_CRITICAL_ISR(&(uart_context[uart_num].spinlock));
UART_ENTER_CRITICAL_ISR(&uart_selectlock);
if (p_uart->uart_select_notif_callback) {
p_uart->uart_select_notif_callback(uart_num, UART_SELECT_ERROR_NOTIF, &HPTaskAwoken);
}
@ -1472,7 +1474,6 @@ esp_err_t uart_get_collision_flag(uart_port_t uart_num, bool* collision_flag)
esp_err_t uart_set_wakeup_threshold(uart_port_t uart_num, int wakeup_threshold)
{
UART_CHECK((uart_num < UART_NUM_MAX), "uart_num error", ESP_ERR_INVALID_ARG);
UART_CHECK((p_uart_obj[uart_num]), "uart driver error", ESP_FAIL);
UART_CHECK((wakeup_threshold <= UART_ACTIVE_THRESHOLD_V &&
wakeup_threshold > UART_MIN_WAKEUP_THRESH),
"wakeup_threshold out of bounds", ESP_ERR_INVALID_ARG);

22
components/soc/esp32/include/hal/uart_ll.h
View file

@ -462,12 +462,12 @@ static inline void uart_ll_get_hw_flow_ctrl(uart_dev_t *hw, uart_hw_flowcontrol_
static inline void uart_ll_set_sw_flow_ctrl(uart_dev_t *hw, uart_sw_flowctrl_t *flow_ctrl, bool sw_flow_ctrl_en)
{
if(sw_flow_ctrl_en) {
hw->flow_conf.xonoff_del = 1;
hw->flow_conf.sw_flow_con_en = 1;
hw->swfc_conf.xon_threshold = flow_ctrl->xon_thrd;
hw->swfc_conf.xoff_threshold = flow_ctrl->xoff_thrd;
hw->swfc_conf.xon_char = flow_ctrl->xon_char;
hw->swfc_conf.xoff_char = flow_ctrl->xoff_char;
hw->flow_conf.sw_flow_con_en = 1;
hw->flow_conf.xonoff_del = 1;
} else {
hw->flow_conf.sw_flow_con_en = 0;
hw->flow_conf.xonoff_del = 0;
@ -701,7 +701,7 @@ static inline void uart_ll_get_at_cmd_char(uart_dev_t *hw, uint8_t *cmd_char, ui
*/
static inline uint32_t uart_ll_get_wakeup_thrd(uart_dev_t *hw)
{
return hw->sleep_conf.active_threshold;
return hw->sleep_conf.active_threshold + SOC_UART_MIN_WAKEUP_THRESH;
}
/**
@ -779,14 +779,14 @@ static inline void uart_ll_set_loop_back(uart_dev_t *hw, bool loop_back_en)
static inline void uart_ll_inverse_signal(uart_dev_t *hw, uint32_t inv_mask)
{
typeof(hw->conf0) conf0_reg = hw->conf0;
conf0_reg.irda_tx_inv |= (inv_mask & UART_SIGNAL_IRDA_TX_INV);
conf0_reg.irda_rx_inv |= (inv_mask & UART_SIGNAL_IRDA_RX_INV);
conf0_reg.rxd_inv |= (inv_mask & UART_SIGNAL_RXD_INV);
conf0_reg.cts_inv |= (inv_mask & UART_SIGNAL_CTS_INV);
conf0_reg.dsr_inv |= (inv_mask & UART_SIGNAL_DSR_INV);
conf0_reg.txd_inv |= (inv_mask & UART_SIGNAL_TXD_INV);
conf0_reg.rts_inv |= (inv_mask & UART_SIGNAL_RTS_INV);
conf0_reg.dtr_inv |= (inv_mask & UART_SIGNAL_DTR_INV);
conf0_reg.irda_tx_inv |= (inv_mask & UART_SIGNAL_IRDA_TX_INV) ? 1 : 0;
conf0_reg.irda_rx_inv |= (inv_mask & UART_SIGNAL_IRDA_RX_INV) ? 1 : 0;
conf0_reg.rxd_inv |= (inv_mask & UART_SIGNAL_RXD_INV) ? 1 : 0;
conf0_reg.cts_inv |= (inv_mask & UART_SIGNAL_CTS_INV) ? 1 : 0;
conf0_reg.dsr_inv |= (inv_mask & UART_SIGNAL_DSR_INV) ? 1 : 0;
conf0_reg.txd_inv |= (inv_mask & UART_SIGNAL_TXD_INV) ? 1 : 0;
conf0_reg.rts_inv |= (inv_mask & UART_SIGNAL_RTS_INV) ? 1 : 0;
conf0_reg.dtr_inv |= (inv_mask & UART_SIGNAL_DTR_INV) ? 1 : 0;
hw->conf0.val = conf0_reg.val;
}

22
components/soc/esp32s2beta/include/hal/uart_ll.h
View file

@ -429,12 +429,12 @@ static inline void uart_ll_get_hw_flow_ctrl(uart_dev_t *hw, uart_hw_flowcontrol_
static inline void uart_ll_set_sw_flow_ctrl(uart_dev_t *hw, uart_sw_flowctrl_t *flow_ctrl, bool sw_flow_ctrl_en)
{
if(sw_flow_ctrl_en) {
hw->flow_conf.xonoff_del = 1;
hw->flow_conf.sw_flow_con_en = 1;
hw->swfc_conf1.xon_threshold = flow_ctrl->xon_thrd;
hw->swfc_conf0.xoff_threshold = flow_ctrl->xoff_thrd;
hw->swfc_conf1.xon_char = flow_ctrl->xon_char;
hw->swfc_conf0.xoff_char = flow_ctrl->xoff_char;
hw->flow_conf.sw_flow_con_en = 1;
hw->flow_conf.xonoff_del = 1;
} else {
hw->flow_conf.sw_flow_con_en = 0;
hw->flow_conf.xonoff_del = 0;
@ -668,7 +668,7 @@ static inline void uart_ll_get_at_cmd_char(uart_dev_t *hw, uint8_t *cmd_char, ui
*/
static inline uint32_t uart_ll_get_wakeup_thrd(uart_dev_t *hw)
{
return hw->sleep_conf.active_threshold;
return hw->sleep_conf.active_threshold + SOC_UART_MIN_WAKEUP_THRESH;
}
/**
@ -745,13 +745,13 @@ static inline void uart_ll_set_loop_back(uart_dev_t *hw, bool loop_back_en)
static inline void uart_ll_inverse_signal(uart_dev_t *hw, uint32_t inv_mask)
{
typeof(hw->conf0) conf0_reg = hw->conf0;
conf0_reg.irda_tx_inv |= (inv_mask & UART_SIGNAL_IRDA_TX_INV);
conf0_reg.irda_rx_inv |= (inv_mask & UART_SIGNAL_IRDA_RX_INV);
conf0_reg.rxd_inv |= (inv_mask & UART_SIGNAL_RXD_INV);
conf0_reg.cts_inv |= (inv_mask & UART_SIGNAL_CTS_INV);
conf0_reg.dsr_inv |= (inv_mask & UART_SIGNAL_DSR_INV);
conf0_reg.txd_inv |= (inv_mask & UART_SIGNAL_TXD_INV);
conf0_reg.rts_inv |= (inv_mask & UART_SIGNAL_RTS_INV);
conf0_reg.dtr_inv |= (inv_mask & UART_SIGNAL_DTR_INV);
conf0_reg.irda_tx_inv |= (inv_mask & UART_SIGNAL_IRDA_TX_INV) ? 1 : 0;
conf0_reg.irda_rx_inv |= (inv_mask & UART_SIGNAL_IRDA_RX_INV) ? 1 : 0;
conf0_reg.rxd_inv |= (inv_mask & UART_SIGNAL_RXD_INV) ? 1 : 0;
conf0_reg.cts_inv |= (inv_mask & UART_SIGNAL_CTS_INV) ? 1 : 0;
conf0_reg.dsr_inv |= (inv_mask & UART_SIGNAL_DSR_INV) ? 1 : 0;
conf0_reg.txd_inv |= (inv_mask & UART_SIGNAL_TXD_INV) ? 1 : 0;
conf0_reg.rts_inv |= (inv_mask & UART_SIGNAL_RTS_INV) ? 1 : 0;
conf0_reg.dtr_inv |= (inv_mask & UART_SIGNAL_DTR_INV) ? 1 : 0;
hw->conf0.val = conf0_reg.val;
}

6
examples/system/console/components/cmd_system/cmd_system.c
View file

@ -17,7 +17,6 @@
#include "esp_spi_flash.h"
#include "driver/rtc_io.h"
#include "driver/uart.h"
#include "hal/uart_ll.h"
#include "argtable3/argtable3.h"
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
@ -288,12 +287,11 @@ static int light_sleep(int argc, char **argv)
}
if (CONFIG_ESP_CONSOLE_UART_NUM <= UART_NUM_1) {
ESP_LOGI(TAG, "Enabling UART wakeup (press ENTER to exit light sleep)");
uart_ll_set_wakeup_thrd(UART_LL_GET_HW(CONFIG_ESP_CONSOLE_UART_NUM), 3);
ESP_ERROR_CHECK( uart_set_wakeup_threshold(CONFIG_ESP_CONSOLE_UART_NUM, 3) );
ESP_ERROR_CHECK( esp_sleep_enable_uart_wakeup(CONFIG_ESP_CONSOLE_UART_NUM) );
}
fflush(stdout);
//Wait UART TX idle
while(!uart_ll_is_tx_idle(UART_LL_GET_HW(CONFIG_ESP_CONSOLE_UART_NUM)));
uart_wait_tx_idle_polling(CONFIG_ESP_CONSOLE_UART_NUM);
esp_light_sleep_start();
esp_sleep_wakeup_cause_t cause = esp_sleep_get_wakeup_cause();
const char *cause_str;

3
examples/system/light_sleep/main/light_sleep_example_main.c
View file

@ -17,7 +17,6 @@
#include "esp_sleep.h"
#include "esp_log.h"
#include "driver/uart.h"
#include "hal/uart_ll.h"
#include "driver/rtc_io.h"
/* Most development boards have "boot" button attached to GPIO0.
@ -58,7 +57,7 @@ void app_main(void)
/* To make sure the complete line is printed before entering sleep mode,
* need to wait until UART TX FIFO is empty:
*/
while(!uart_ll_is_tx_idle(UART_LL_GET_HW(CONFIG_ESP_CONSOLE_UART_NUM)));
uart_wait_tx_idle_polling(CONFIG_ESP_CONSOLE_UART_NUM);
/* Get timestamp before entering sleep */
int64_t t_before_us = esp_timer_get_time();