From 131330034a8ea529c9fdeed7c61d6aa475218f50 Mon Sep 17 00:00:00 2001 From: Song Ruo Jing Date: Wed, 8 Jan 2025 20:38:11 +0800 Subject: [PATCH] fix(uart): allow same pin for tx and rx in uart_set_pin Closes https://github.com/espressif/esp-idf/issues/14787 --- .../esp_driver_uart/include/driver/uart.h | 8 ++- components/esp_driver_uart/src/uart.c | 56 ++++++++++----- .../test_apps/uart/main/test_uart.c | 68 ++++++++++++++++--- 3 files changed, 101 insertions(+), 31 deletions(-) diff --git a/components/esp_driver_uart/include/driver/uart.h b/components/esp_driver_uart/include/driver/uart.h index 2d090ba10a..55f9e2623a 100644 --- a/components/esp_driver_uart/include/driver/uart.h +++ b/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. diff --git a/components/esp_driver_uart/src/uart.c b/components/esp_driver_uart/src/uart.c index 2ffc48cb10..c51cfa303d 100644 --- a/components/esp_driver_uart/src/uart.c +++ b/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,8 +796,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, 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 } @@ -793,22 +804,31 @@ esp_err_t uart_set_pin(uart_port_t uart_num, int tx_io_num, int rx_io_num, int r } 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; } diff --git a/components/esp_driver_uart/test_apps/uart/main/test_uart.c b/components/esp_driver_uart/test_apps/uart/main/test_uart.c index 2840e833d9..2add03dbf2 100644 --- a/components/esp_driver_uart/test_apps/uart/main/test_uart.c +++ b/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)); +}