From ab4157b6cf45af10afe2ceba792af99afe0740c5 Mon Sep 17 00:00:00 2001 From: Song Ruo Jing Date: Mon, 12 Aug 2024 20:50:40 +0800 Subject: [PATCH 1/2] fix(gpio): patched esp_rom_gpio_connect_out_signal for esp32 and esp32s2 The original ROM function enabled output for the pad first, and then connected the signal This could result in an undesired level change at the pad Closes https://github.com/espressif/esp-idf/issues/12826 --- components/esp_rom/CMakeLists.txt | 3 ++- components/esp_rom/include/esp_rom_gpio.h | 1 + components/esp_rom/patches/esp_rom_gpio.c | 28 +++++++++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 components/esp_rom/patches/esp_rom_gpio.c diff --git a/components/esp_rom/CMakeLists.txt b/components/esp_rom/CMakeLists.txt index a216bad4c2..da886376c3 100644 --- a/components/esp_rom/CMakeLists.txt +++ b/components/esp_rom/CMakeLists.txt @@ -21,7 +21,8 @@ else() list(APPEND sources "patches/esp_rom_crc.c" "patches/esp_rom_uart.c" "patches/esp_rom_spiflash.c" - "patches/esp_rom_efuse.c") + "patches/esp_rom_efuse.c" + "patches/esp_rom_gpio.c") # Override regi2c implementation in ROM diff --git a/components/esp_rom/include/esp_rom_gpio.h b/components/esp_rom/include/esp_rom_gpio.h index 4788d7b2de..5405fa3f0a 100644 --- a/components/esp_rom/include/esp_rom_gpio.h +++ b/components/esp_rom/include/esp_rom_gpio.h @@ -66,6 +66,7 @@ void esp_rom_gpio_connect_in_signal(uint32_t gpio_num, uint32_t signal_idx, bool * @brief Combine a peripheral signal which tagged as output attribute with a GPIO. * * @note There's no limitation on the number of signals that a GPIO can combine with. + * @note Internally, the signal will be connected first, then output will be enabled on the pad. * * @param gpio_num GPIO number * @param signal_idx Peripheral signal index (tagged as output attribute). Particularly, `SIG_GPIO_OUT_IDX` means disconnect GPIO and other peripherals. Only the GPIO driver can control the output level. diff --git a/components/esp_rom/patches/esp_rom_gpio.c b/components/esp_rom/patches/esp_rom_gpio.c new file mode 100644 index 0000000000..bf8bef623c --- /dev/null +++ b/components/esp_rom/patches/esp_rom_gpio.c @@ -0,0 +1,28 @@ +/* + * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "sdkconfig.h" +#include "esp_attr.h" +#include "esp_rom_gpio.h" +#include "soc/gpio_reg.h" + +#if CONFIG_IDF_TARGET_ESP32 || CONFIG_IDF_TARGET_ESP32S2 +// On such targets, the ROM code for this function enabled output for the pad first, and then connected the signal +// This could result in an undesired glitch at the pad +IRAM_ATTR void esp_rom_gpio_connect_out_signal(uint32_t gpio_num, uint32_t signal_idx, bool out_inv, bool oen_inv) +{ + uint32_t value = signal_idx << GPIO_FUNC0_OUT_SEL_S; + if (out_inv) { + value |= GPIO_FUNC0_OUT_INV_SEL_M; + } + if (oen_inv) { + value |= GPIO_FUNC0_OEN_INV_SEL_M; + } + REG_WRITE(GPIO_FUNC0_OUT_SEL_CFG_REG + (gpio_num * 4), value); + + REG_WRITE(GPIO_ENABLE_W1TS_REG, (1 << gpio_num)); +} +#endif From 2f92d863d89ca46448c4257d09d2550d20cd9977 Mon Sep 17 00:00:00 2001 From: Song Ruo Jing Date: Mon, 12 Aug 2024 21:50:18 +0800 Subject: [PATCH 2/2] fix(uart): eliminated potential glitch on TX at setup if TX signal is inversed Closes https://github.com/espressif/esp-idf/issues/14285 --- components/esp_driver_uart/src/uart.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/components/esp_driver_uart/src/uart.c b/components/esp_driver_uart/src/uart.c index 55e93166d3..03f9c1bffd 100644 --- a/components/esp_driver_uart/src/uart.c +++ b/components/esp_driver_uart/src/uart.c @@ -740,16 +740,17 @@ esp_err_t uart_set_pin(uart_port_t uart_num, int tx_io_num, int rx_io_num, int r if (tx_io_num >= 0 && !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); - gpio_set_level(tx_io_num, 1); esp_rom_gpio_connect_out_signal(tx_io_num, UART_PERIPH_SIGNAL(uart_num, SOC_UART_TX_PIN_IDX), 0, 0); + // output enable is set inside esp_rom_gpio_connect_out_signal func after the signal is connected + // (output enabled too early may cause unnecessary level change at the pad) } #if SOC_LP_GPIO_MATRIX_SUPPORTED else { - rtc_gpio_set_direction(tx_io_num, RTC_GPIO_MODE_OUTPUT_ONLY); rtc_gpio_init(tx_io_num); rtc_gpio_iomux_func_sel(tx_io_num, RTCIO_LL_PIN_FUNC); 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 } @@ -757,7 +758,7 @@ esp_err_t uart_set_pin(uart_port_t uart_num, int tx_io_num, int rx_io_num, int r if (rx_io_num >= 0 && !uart_try_set_iomux_pin(uart_num, rx_io_num, SOC_UART_RX_PIN_IDX)) { 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); + 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); } @@ -765,7 +766,7 @@ esp_err_t uart_set_pin(uart_port_t uart_num, int tx_io_num, int rx_io_num, int r 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_iomux_func_sel(rx_io_num, RTCIO_LL_PIN_FUNC); lp_gpio_connect_in_signal(rx_io_num, UART_PERIPH_SIGNAL(uart_num, SOC_UART_RX_PIN_IDX), 0); } @@ -775,15 +776,15 @@ esp_err_t uart_set_pin(uart_port_t uart_num, int tx_io_num, int rx_io_num, int r if (rts_io_num >= 0 && !uart_try_set_iomux_pin(uart_num, rts_io_num, SOC_UART_RTS_PIN_IDX)) { if (uart_num < SOC_UART_HP_NUM) { gpio_func_sel(rts_io_num, PIN_FUNC_GPIO); - gpio_set_direction(rts_io_num, GPIO_MODE_OUTPUT); esp_rom_gpio_connect_out_signal(rts_io_num, UART_PERIPH_SIGNAL(uart_num, SOC_UART_RTS_PIN_IDX), 0, 0); + // output enable is set inside esp_rom_gpio_connect_out_signal func after the signal is connected } #if SOC_LP_GPIO_MATRIX_SUPPORTED else { - rtc_gpio_set_direction(rts_io_num, RTC_GPIO_MODE_OUTPUT_ONLY); rtc_gpio_init(rts_io_num); - rtc_gpio_iomux_func_sel(rts_io_num, 1); + rtc_gpio_iomux_func_sel(rts_io_num, RTCIO_LL_PIN_FUNC); 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 } @@ -799,7 +800,7 @@ esp_err_t uart_set_pin(uart_port_t uart_num, int tx_io_num, int rx_io_num, int r 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_iomux_func_sel(cts_io_num, RTCIO_LL_PIN_FUNC); lp_gpio_connect_in_signal(cts_io_num, UART_PERIPH_SIGNAL(uart_num, SOC_UART_CTS_PIN_IDX), 0); }