Merge branch 'bugfix/uart_single_wire_mode_v5.2' into 'release/v5.2'

fix(uart): allow same pin for tx and rx in uart_set_pin; UART_SELECT_READ_NOTIF race conditon fix (v5.2)

See merge request espressif/esp-idf!36261
This commit is contained in:
morris
2025-01-10 11:42:19 +08:00
3 changed files with 87 additions and 32 deletions

View File

@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2021-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2021-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -302,9 +302,7 @@ static void uart_write_task(void *param)
{
uart_port_t uart_num = (uart_port_t)param;
uint8_t *tx_buf = (uint8_t *)malloc(1024);
if(tx_buf == NULL) {
TEST_FAIL_MESSAGE("tx buffer malloc fail");
}
TEST_ASSERT_NOT_NULL(tx_buf);
for(int i = 1; i < 1023; i++) {
tx_buf[i] = (i & 0xff);
}
@@ -326,9 +324,7 @@ TEST_CASE("uart read write test", "[uart]")
uart_port_t uart_num = port_param.port_num;
uint8_t *rd_data = (uint8_t *)malloc(1024);
if(rd_data == NULL) {
TEST_FAIL_MESSAGE("rx buffer malloc fail");
}
TEST_ASSERT_NOT_NULL(rd_data);
uart_config_t uart_config = {
.baud_rate = 2000000,
.data_bits = UART_DATA_8_BITS,
@@ -395,10 +391,9 @@ TEST_CASE("uart tx with ringbuffer test", "[uart]")
uart_port_t uart_num = port_param.port_num;
uint8_t *rd_data = (uint8_t *)malloc(1024);
TEST_ASSERT_NOT_NULL(rd_data);
uint8_t *wr_data = (uint8_t *)malloc(1024);
if(rd_data == NULL || wr_data == NULL) {
TEST_FAIL_MESSAGE("buffer malloc fail");
}
TEST_ASSERT_NOT_NULL(wr_data);
uart_config_t uart_config = {
.baud_rate = 2000000,
.data_bits = UART_DATA_8_BITS,
@@ -525,3 +520,56 @@ TEST_CASE("uart int state restored after flush", "[uart]")
TEST_ESP_OK(uart_driver_delete(uart_num));
free(data);
}
TEST_CASE("uart in one-wire mode", "[uart]")
{
uart_port_param_t port_param = {};
TEST_ASSERT(port_select(&port_param));
port_param.tx_pin_num = port_param.rx_pin_num; // let tx and rx use the same pin
uart_port_t uart_num = port_param.port_num;
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 = port_param.default_src_clk,
};
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));
esp_err_t err = uart_set_pin(uart_num, port_param.tx_pin_num, port_param.rx_pin_num, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE);
if (uart_num < SOC_UART_HP_NUM) {
TEST_ESP_OK(err);
#if SOC_UART_LP_NUM > 0
} else {
#if !SOC_LP_GPIO_MATRIX_SUPPORTED
TEST_ESP_ERR(ESP_FAIL, err); // For LP UART port, if no LP GPIO Matrix, unable to be used in one-wire mode
#else
TEST_ESP_OK(err);
#endif
#endif // SOC_UART_LP_NUM > 0
}
// If configured successfully in one-wire mode
if (err == ESP_OK) {
TEST_ESP_OK(uart_wait_tx_done(uart_num, portMAX_DELAY));
vTaskDelay(pdMS_TO_TICKS(20)); // make sure last byte has flushed from TX FIFO
TEST_ESP_OK(uart_flush_input(uart_num));
const char *wr_data = "ECHO!";
const int len = strlen(wr_data);
uint8_t *rd_data = (uint8_t *)calloc(1, 1024);
TEST_ASSERT_NOT_NULL(rd_data);
uart_write_bytes(uart_num, wr_data, len);
int bytes_received = uart_read_bytes(uart_num, rd_data, BUF_SIZE, pdMS_TO_TICKS(20));
TEST_ASSERT_EQUAL(len, bytes_received);
TEST_ASSERT_EQUAL_STRING_LEN(wr_data, rd_data, bytes_received);
free(rd_data);
}
TEST_ESP_OK(uart_driver_delete(uart_num));
}

View File

@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -401,8 +401,10 @@ esp_err_t uart_enable_tx_intr(uart_port_t uart_num, int enable, int thresh);
* RX pin binded to a GPIO through the GPIO matrix, whereas TX is binded
* to its GPIO through the IOMUX.
*
* @note Internal signal can be output to multiple GPIO pads.
* Only one GPIO pad can connect with input signal.
* @note It is possible to configure TX and RX to share the same IO (single wire mode),
* but please be aware of output conflict, which could damage the pad.
* Apply open-drain and pull-up to the pad ahead of time as a protection,
* or the upper layer protocol must guarantee no output from two ends at the same time.
*
* @param uart_num UART port number, the max port number is (UART_NUM_MAX -1).
* @param tx_io_num UART TX pin GPIO number.

View File

@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -739,8 +739,12 @@ esp_err_t uart_set_pin(uart_port_t uart_num, int tx_io_num, int rx_io_num, int r
}
#endif
// Since an IO cannot route peripheral signals via IOMUX and GPIO matrix at the same time,
// if tx and rx share the same IO, both signals need to be route to IOs through GPIO matrix
bool tx_rx_same_io = (tx_io_num == rx_io_num);
/* In the following statements, if the io_num is negative, no need to configure anything. */
if (tx_io_num >= 0 && !uart_try_set_iomux_pin(uart_num, tx_io_num, SOC_UART_TX_PIN_IDX)) {
if (tx_io_num >= 0 && (tx_rx_same_io || !uart_try_set_iomux_pin(uart_num, tx_io_num, SOC_UART_TX_PIN_IDX))) {
if (uart_num < SOC_UART_HP_NUM) {
gpio_hal_iomux_func_sel(GPIO_PIN_MUX_REG[tx_io_num], PIN_FUNC_GPIO);
esp_rom_gpio_connect_out_signal(tx_io_num, UART_PERIPH_SIGNAL(uart_num, SOC_UART_TX_PIN_IDX), 0, 0);
@@ -749,8 +753,7 @@ esp_err_t uart_set_pin(uart_port_t uart_num, int tx_io_num, int rx_io_num, int r
}
#if SOC_LP_GPIO_MATRIX_SUPPORTED
else {
rtc_gpio_init(tx_io_num);
rtc_gpio_iomux_func_sel(tx_io_num, 1);
rtc_gpio_init(tx_io_num); // set as a LP_GPIO pin
lp_gpio_connect_out_signal(tx_io_num, UART_PERIPH_SIGNAL(uart_num, SOC_UART_TX_PIN_IDX), 0, 0);
// output enable is set inside lp_gpio_connect_out_signal func after the signal is connected
@@ -758,18 +761,19 @@ esp_err_t uart_set_pin(uart_port_t uart_num, int tx_io_num, int rx_io_num, int r
#endif
}
if (rx_io_num >= 0 && !uart_try_set_iomux_pin(uart_num, rx_io_num, SOC_UART_RX_PIN_IDX)) {
if (rx_io_num >= 0 && (tx_rx_same_io || !uart_try_set_iomux_pin(uart_num, rx_io_num, SOC_UART_RX_PIN_IDX))) {
if (uart_num < SOC_UART_HP_NUM) {
gpio_hal_iomux_func_sel(GPIO_PIN_MUX_REG[rx_io_num], PIN_FUNC_GPIO);
gpio_set_pull_mode(rx_io_num, GPIO_PULLUP_ONLY); // This does not consider that RX signal can be read inverted by configuring the hardware (i.e. idle is at low level). However, it is only a weak pullup, the TX at the other end can always drive the line.
gpio_set_direction(rx_io_num, GPIO_MODE_INPUT);
esp_rom_gpio_connect_in_signal(rx_io_num, UART_PERIPH_SIGNAL(uart_num, SOC_UART_RX_PIN_IDX), 0);
}
#if SOC_LP_GPIO_MATRIX_SUPPORTED
else {
rtc_gpio_set_direction(rx_io_num, RTC_GPIO_MODE_INPUT_ONLY);
rtc_gpio_init(rx_io_num);
rtc_gpio_iomux_func_sel(rx_io_num, 1);
rtc_gpio_mode_t mode = (tx_rx_same_io ? RTC_GPIO_MODE_INPUT_OUTPUT : RTC_GPIO_MODE_INPUT_ONLY);
rtc_gpio_set_direction(rx_io_num, mode);
if (!tx_rx_same_io) { // set the same pin again as a LP_GPIO will overwrite connected out_signal, not desired, so skip
rtc_gpio_init(rx_io_num); // set as a LP_GPIO pin
}
lp_gpio_connect_in_signal(rx_io_num, UART_PERIPH_SIGNAL(uart_num, SOC_UART_RX_PIN_IDX), 0);
}
@@ -784,8 +788,7 @@ esp_err_t uart_set_pin(uart_port_t uart_num, int tx_io_num, int rx_io_num, int r
}
#if SOC_LP_GPIO_MATRIX_SUPPORTED
else {
rtc_gpio_init(rts_io_num);
rtc_gpio_iomux_func_sel(rts_io_num, 1);
rtc_gpio_init(rts_io_num); // set as a LP_GPIO pin
lp_gpio_connect_out_signal(rts_io_num, UART_PERIPH_SIGNAL(uart_num, SOC_UART_RTS_PIN_IDX), 0, 0);
// output enable is set inside lp_gpio_connect_out_signal func after the signal is connected
}
@@ -802,8 +805,7 @@ esp_err_t uart_set_pin(uart_port_t uart_num, int tx_io_num, int rx_io_num, int r
#if SOC_LP_GPIO_MATRIX_SUPPORTED
else {
rtc_gpio_set_direction(cts_io_num, RTC_GPIO_MODE_INPUT_ONLY);
rtc_gpio_init(cts_io_num);
rtc_gpio_iomux_func_sel(cts_io_num, 1);
rtc_gpio_init(cts_io_num); // set as a LP_GPIO pin
lp_gpio_connect_in_signal(cts_io_num, UART_PERIPH_SIGNAL(uart_num, SOC_UART_CTS_PIN_IDX), 0);
}
@@ -1133,12 +1135,6 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
uart_event.type = UART_DATA;
uart_event.size = rx_fifo_len;
uart_event.timeout_flag = (uart_intr_status & UART_INTR_RXFIFO_TOUT) ? true : false;
UART_ENTER_CRITICAL_ISR(&uart_selectlock);
if (p_uart->uart_select_notif_callback) {
p_uart->uart_select_notif_callback(uart_num, UART_SELECT_READ_NOTIF, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
}
UART_EXIT_CRITICAL_ISR(&uart_selectlock);
}
p_uart->rx_stash_len = rx_fifo_len;
//If we fail to push data to ring buffer, we will have to stash the data, and send next time.
@@ -1187,6 +1183,15 @@ static void UART_ISR_ATTR uart_rx_intr_handler_default(void *param)
p_uart->rx_buffered_len += p_uart->rx_stash_len;
UART_EXIT_CRITICAL_ISR(&(uart_context[uart_num].spinlock));
}
if (uart_event.type == UART_DATA) {
UART_ENTER_CRITICAL_ISR(&uart_selectlock);
if (p_uart->uart_select_notif_callback) {
p_uart->uart_select_notif_callback(uart_num, UART_SELECT_READ_NOTIF, &HPTaskAwoken);
need_yield |= (HPTaskAwoken == pdTRUE);
}
UART_EXIT_CRITICAL_ISR(&uart_selectlock);
}
} else {
UART_ENTER_CRITICAL_ISR(&(uart_context[uart_num].spinlock));
uart_hal_disable_intr_mask(&(uart_context[uart_num].hal), UART_INTR_RXFIFO_FULL | UART_INTR_RXFIFO_TOUT);