Skip to content

cpu/esp32: fix and improve UART initialization #19146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bors merged 5 commits into from
Jan 14, 2023
Merged

cpu/esp32: fix and improve UART initialization #19146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d5a28ec
cpu/esp32: reorder the syscalls_init
gschorcht Jan 14, 2023
9004867
cpu/esp32/bootloader: fix the UART pin configuration
gschorcht Jan 14, 2023
fe21e82
cpu/esp32: improve initialization of UART pins
gschorcht Jan 14, 2023
3085b92
cpu/esp32: improve UART initialization
gschorcht Jan 14, 2023
f933fde
cpu/esp32: don't initialize the UART pins if already initialized
gschorcht Jan 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cpu/esp32/bootloader/sdkconfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ extern "C" {
* If custom TX and RX are defined, use custom UART configuration for 2nd stage
* bootloader.
*/
#if defined(CONFIG_CONSOLE_UART_RX) && defined(CONFIG_CONSOLE_UART_RX)
#if defined(CONFIG_CONSOLE_UART_TX) && defined(CONFIG_CONSOLE_UART_RX)
#define CONFIG_ESP_CONSOLE_UART_CUSTOM 1
#define CONFIG_ESP_CONSOLE_UART_TX_GPIO CONFIG_CONSOLE_UART_TX
#define CONFIG_ESP_CONSOLE_UART_RX_GPIO CONFIG_CONSOLE_UART_RX
Expand Down
14 changes: 7 additions & 7 deletions cpu/esp32/startup.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,13 @@ static NORETURN void IRAM system_startup_cpu0(void)
puf_sram_init((uint8_t *)&_sheap, SEED_RAM_LEN);
#endif

/* initialize system call tables of ESP32x rom and newlib */
syscalls_init();

/* systemwide UART initialization */
extern void uart_system_init (void);
uart_system_init();

/* initialize stdio */
esp_rom_uart_tx_wait_idle(CONFIG_ESP_CONSOLE_UART_NUM);
early_init();
Expand Down Expand Up @@ -228,16 +235,9 @@ static NORETURN void IRAM system_init (void)
/* initialize the ISR stack for usage measurements */
thread_isr_stack_init();

/* initialize system call tables of ESP32x rom and newlib */
syscalls_init();

/* install exception handlers */
init_exceptions();

/* systemwide UART initialization */
extern void uart_system_init (void);
uart_system_init();

/* set log levels for SDK library outputs */
extern void esp_log_level_set(const char* tag, esp_log_level_t level);
esp_log_level_set("wifi", LOG_DEBUG);
Expand Down
62 changes: 40 additions & 22 deletions cpu/esp_common/periph/uart.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
#else /* defined(MCU_ESP8266) */

#include "esp_rom_gpio.h"
#include "esp_rom_uart.h"
#include "hal/interrupt_controller_types.h"
#include "hal/interrupt_controller_ll.h"
#include "soc/gpio_reg.h"
Expand All @@ -68,6 +69,7 @@
#include "soc/periph_defs.h"
#include "soc/rtc.h"
#include "soc/soc_caps.h"
#include "soc/uart_pins.h"
#include "soc/uart_reg.h"
#include "soc/uart_struct.h"

Expand Down Expand Up @@ -166,30 +168,34 @@ int uart_init(uart_t uart, uint32_t baudrate, uart_rx_cb_t rx_cb, void *arg)
assert(uart < UART_NUMOF);

#ifndef MCU_ESP8266
/* reset the pins when they were already used as UART pins */
if (gpio_get_pin_usage(uart_config[uart].txd) == _UART) {
gpio_set_pin_usage(uart_config[uart].txd, _GPIO);
}
if (gpio_get_pin_usage(uart_config[uart].rxd) == _UART) {
gpio_set_pin_usage(uart_config[uart].rxd, _GPIO);
}

/* try to initialize the pins as GPIOs first */
if ((uart_config[uart].txd != GPIO_UNDEF &&
gpio_init(uart_config[uart].txd, GPIO_OUT)) ||
(uart_config[uart].rxd != GPIO_UNDEF &&
gpio_init(uart_config[uart].rxd, GPIO_IN_PU))) {
return -1;
}
assert(uart_config[uart].txd != GPIO_UNDEF);
assert(uart_config[uart].rxd != GPIO_UNDEF);
Copy link
Contributor

@benpicco benpicco Jan 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are use cases of TX only / RX only UARTs.
This is generally allowed by other UART implementations, e.g sam0_common.

Copy link
Contributor Author

@gschorcht gschorcht Jan 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK understand, also the ESP8266 has a second UART device which can only be TX.

But this change is not a regression from the previous implementation, which didn't really work for TX-only and RX-only interface, because it didn't handle the TX and RX pin separately.

For now, this PR would fix the problem in the release version. The handling of TX- or RX-only UARTs could be improved as a follow-up.

Copy link
Contributor

@benpicco benpicco Jan 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let’s do that as a follow-up then

bors merge


/* don't reinitialize the pins if they are already configured as UART pins */
if ((gpio_get_pin_usage(uart_config[uart].txd) != _UART) ||
(gpio_get_pin_usage(uart_config[uart].rxd) != _UART)) {

/* try to initialize the pins where the TX line is set and temporarily
* configured as a pull-up open-drain output before configuring it as
* a push-pull output to avoid a several msec long LOW pulse resulting
* in some garbage */
gpio_set(uart_config[uart].txd);
if (gpio_init(uart_config[uart].txd, GPIO_OD_PU) ||
gpio_init(uart_config[uart].txd, GPIO_OUT) ||
gpio_init(uart_config[uart].rxd, GPIO_IN_PU)) {
return -1;
}

/* store the usage type in GPIO table */
gpio_set_pin_usage(uart_config[uart].txd, _UART);
gpio_set_pin_usage(uart_config[uart].rxd, _UART);
/* store the usage type in GPIO table */
gpio_set_pin_usage(uart_config[uart].txd, _UART);
gpio_set_pin_usage(uart_config[uart].rxd, _UART);

esp_rom_gpio_connect_out_signal(uart_config[uart].txd,
_uarts[uart].signal_txd, false, false);
esp_rom_gpio_connect_in_signal(uart_config[uart].rxd,
_uarts[uart].signal_rxd, false);
esp_rom_uart_tx_wait_idle(uart);
esp_rom_gpio_connect_out_signal(uart_config[uart].txd,
_uarts[uart].signal_txd, false, false);
esp_rom_gpio_connect_in_signal(uart_config[uart].rxd,
_uarts[uart].signal_rxd, false);
}
#endif /* MCU_ESP8266 */

_uarts[uart].baudrate = baudrate;
Expand Down Expand Up @@ -247,6 +253,18 @@ void uart_system_init(void)
/* reset all UART interrupt status registers */
_uarts[uart].regs->int_clr.val = ~0;
}
#ifndef MCU_ESP8266
/* reset the pin usage of the default UART0 pins to GPIO to allow
* reinitialization and usage for other purposes */
gpio_set_pin_usage(U0TXD_GPIO_NUM, _GPIO);
gpio_set_pin_usage(U0RXD_GPIO_NUM, _GPIO);
#if defined(CONFIG_CONSOLE_UART_TX) && defined(CONFIG_CONSOLE_UART_TX)
/* reset the pin usage of the UART pins used by the bootloader to
* _GPIO to be able to use them later for other purposes */
gpio_set_pin_usage(CONFIG_CONSOLE_UART_TX, _GPIO);
gpio_set_pin_usage(CONFIG_CONSOLE_UART_RX, _GPIO);
#endif
#endif
}

void uart_print_config(void)
Expand Down