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

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

See merge request espressif/esp-idf!36250

Author: suda-morris

components/esp_driver_uart/include/driver/uart.h

@@ -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
  */
@@ -398,8 +398,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.

components/esp_driver_uart/src/uart.c

@@ -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
  */
@@ -27,6 +27,7 @@
 #include "driver/uart_select.h"
 #include "esp_private/esp_clk_tree_common.h"
 #include "esp_private/gpio.h"
+#include "esp_private/esp_gpio_reserve.h"
 #include "esp_private/uart_share_hw_ctrl.h"
 #include "esp_clk_tree.h"
 #include "sdkconfig.h"
@@ -739,8 +740,19 @@ esp_err_t uart_set_pin(uart_port_t uart_num, int tx_io_num, int rx_io_num, int r
     }
 #endif
 
+    // Potential IO reserved mask
+    uint64_t io_reserve_mask = 0;
+    io_reserve_mask |= (tx_io_num > 0 ? BIT64(tx_io_num) : 0);
+    io_reserve_mask |= (rx_io_num > 0 ? BIT64(rx_io_num) : 0);
+    io_reserve_mask |= (rts_io_num > 0 ? BIT64(rts_io_num) : 0);
+    io_reserve_mask |= (cts_io_num > 0 ? BIT64(cts_io_num) : 0);
+
+    // 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_func_sel(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,27 +761,27 @@ 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, RTCIO_LL_PIN_FUNC);
-
+            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
         }
 #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))) {
+        io_reserve_mask &= ~BIT64(rx_io_num); // input IO via GPIO matrix does not need to be reserved
         if (uart_num < SOC_UART_HP_NUM) {
             gpio_func_sel(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);
+            gpio_input_enable(rx_io_num);
             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, RTCIO_LL_PIN_FUNC);
+            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,31 +796,39 @@ 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, RTCIO_LL_PIN_FUNC);
+            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
         }
 #endif
     }
 
     if (cts_io_num >= 0  && !uart_try_set_iomux_pin(uart_num, cts_io_num, SOC_UART_CTS_PIN_IDX)) {
+        io_reserve_mask &= ~BIT64(cts_io_num); // input IO via GPIO matrix does not need to be reserved
         if (uart_num < SOC_UART_HP_NUM) {
             gpio_func_sel(cts_io_num, PIN_FUNC_GPIO);
-            gpio_set_pull_mode(cts_io_num, GPIO_PULLUP_ONLY);
-            gpio_set_direction(cts_io_num, GPIO_MODE_INPUT);
+            gpio_pullup_en(cts_io_num);
+            gpio_input_enable(cts_io_num);
             esp_rom_gpio_connect_in_signal(cts_io_num, UART_PERIPH_SIGNAL(uart_num, SOC_UART_CTS_PIN_IDX), 0);
         }
 #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, RTCIO_LL_PIN_FUNC);
-
+            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);
         }
 #endif
     }
+
+    // IO reserve
+    uint64_t old_busy_mask = esp_gpio_reserve(io_reserve_mask);
+    uint64_t conflict_mask = old_busy_mask & io_reserve_mask;
+    while (conflict_mask > 0) {
+        uint8_t pos = __builtin_ctzll(conflict_mask);
+        conflict_mask &= ~(1ULL << pos);
+        ESP_LOGW(UART_TAG, "GPIO %d is not usable, maybe used by others", pos);
+    }
+
     return ESP_OK;
 }
 
@@ -1133,12 +1153,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 +1201,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);

components/esp_driver_uart/test_apps/uart/main/test_uart.c

@@ -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
  */
@@ -306,9 +306,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);
     }
@@ -330,9 +328,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,
@@ -399,10 +395,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,
@@ -536,3 +531,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));
+}

docs/en/api-reference/peripherals/gpio.rst

@@ -81,7 +81,7 @@ In addition, if you would like to dump the configurations of all IOs, you can us
 
 ::
 
-    gpio_dump_all_io_configuration(stdout, SOC_GPIO_VALID_GPIO_MASK);
+    gpio_dump_io_configuration(stdout, SOC_GPIO_VALID_GPIO_MASK);
 
 If an IO pin is routed to a peripheral signal through the GPIO matrix, the signal ID printed in the dump information is defined in the :component_file:`soc/{IDF_TARGET_PATH_NAME}/include/soc/gpio_sig_map.h` header file. The word ``**RESERVED**`` indicates the IO is occupied by either SPI flash or PSRAM. It is strongly not recommended to reconfigure them for other application purposes.
 

docs/zh_CN/api-reference/peripherals/gpio.rst

@@ -81,7 +81,7 @@ GPIO 驱动提供了一个函数 :cpp:func:`gpio_dump_io_configuration` 用来
 
 ::
 
-    gpio_dump_all_io_configuration(stdout, SOC_GPIO_VALID_GPIO_MASK);
+    gpio_dump_io_configuration(stdout, SOC_GPIO_VALID_GPIO_MASK);
 
 如果 IO 管脚通过 GPIO 交换矩阵连接到内部外设信号，输出信息打印中的外设信号 ID 定义可以在 :component_file:`soc/{IDF_TARGET_PATH_NAME}/include/soc/gpio_sig_map.h` 头文件中查看。``**RESERVED**`` 字样则表示此 IO 用于连接 SPI flash 或 PSRAM，强烈建议不要重新配置这些管脚用于其他功能。