From b600a2ef8b5d852217b93861c31a8c621eff8721 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 affbe3d3a8..d01c6f8221 100644 --- a/components/esp_rom/CMakeLists.txt +++ b/components/esp_rom/CMakeLists.txt @@ -18,7 +18,8 @@ else() "patches/esp_rom_sys.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 5dd2f38eb478cb199c32be89b2a27c726036b53c 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/driver/uart/uart.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/components/driver/uart/uart.c b/components/driver/uart/uart.c index 94ca84a9a2..a6629955bf 100644 --- a/components/driver/uart/uart.c +++ b/components/driver/uart/uart.c @@ -743,16 +743,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_hal_iomux_func_sel(GPIO_PIN_MUX_REG[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, 1); 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 } @@ -760,7 +761,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_hal_iomux_func_sel(GPIO_PIN_MUX_REG[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); } @@ -778,15 +779,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_hal_iomux_func_sel(GPIO_PIN_MUX_REG[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); 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 }