cpu/esp32: fix and improve UART initialization #19146

gschorcht · 2023-01-14T13:40:58Z

Contribution description

This PR fixes issue #19138 that was introduced with PR #19100. It contains the following changes to fix the problems and to improve the UART initialization:

If LOG_LEVEL is greater or equal 4, such as in tests/log_printfnoformat, the ESP-IDF config function called for the GPIO pins of the UART will output the configuration with printf before the _GLOBAL_REENT structure is initialized. This causes a crash during system startup. Therefore the initialization by syscalls_init must be called by `earlier in the startup procedure.
Since PR cpu/esp_common: allow configuration of UART0 #19100 it is possible to define:
- other pins for UART_DEV(0) than the default pins
- different UART_DEV(0) pins for the bootloader and RIOT
To allow correct reinitialization of the UART pins used by the bootloader as well as their usage for other purposes, the pin usage for the default UART0 pins and the UART pinsused by the bootloader are reset to _GPIO. This is done in uart_system_init which has to be called earlier in the startup procedure.
To avoid garbage on reconfiguring the UART console pins, e.g. in initialization of the arduino module, pins that are already configured as UART pins must not be initialized.
To avoid a several msec long LOW pulse resulting in some garbage during the UART initialization, the TX line is set to HIGH and temporarily configured as a pull-up open-drain output before configuring it as a push-pull output.

The PR requires a backport to 2023.1

Testing procedure

The following tests should work with this PR:

tests/log_color
tests/log_printfnoformat
tests/sys_arduino

Issues/PRs references

Fixes #19138

If LOG_LEVEL >= 4, such as in `tests/log_printfnoformat`, the ESP-IDF config function called for the GPIO pins of the UART will output the configuration with `printf` before the `_GLOBAL_REENT` structure is initialized. This causes a crash during system startup. Therefore the initialization by `syscalls_init` must be called earlier in the startup procedure.

Since PR RIOT-OS#19100 it is possible to define: - other pins for `UART_DEV(0)` than the default pins - different `UART_DEV(0)` pins for the bootloader and RIOT To allow correct reinitialization of the UART pins used by the bootloader as well as their usage for other purposes, the pin usage for the default UART0 pins and the UART pins used by the bootloader are reset to `_GPIO`. This is done in `uart_system_init` which has to be called earlier in the startup procedure.

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.

To avoid garbage on reconfiguring the UART console pins, e.g. in initialization of the `arduino` module, pins that are already configured as UART pins must not be initialized.

riot-ci · 2023-01-14T13:54:57Z

Murdock results

✔️ PASSED

f933fde cpu/esp32: don't initialize the UART pins if already initialized

Success	Failures	Total	Runtime
6768	0	6768	11m:26s

Artifacts

Documentation preview

maribu · 2023-01-14T16:13:31Z

At least the log_noformat test was run prior to cancelation and succeeded, so this indeed seems to fix the issue.

maribu

bors merge

Code looks good and at least one test previously failing now succeeds in the CI.

bors · 2023-01-14T16:14:42Z

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

benpicco · 2023-01-14T17:04:25Z

cpu/esp_common/periph/uart.c

-        return -1;
-    }
+    assert(uart_config[uart].txd != GPIO_UNDEF);
+    assert(uart_config[uart].rxd != GPIO_UNDEF);


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

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.

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

bors merge

benpicco · 2023-01-14T17:05:05Z

bors cancel

bors · 2023-01-14T21:26:11Z

Build succeeded:

gschorcht · 2023-01-15T09:27:36Z

Thanks for reviewing and testing.

19148: cpu/esp32: fix and improve UART initialization [backport 2023.01] r=maribu a=gschorcht # Backport of PR #19146 ### Contribution description This PR fixes issue #19138 that was introduced with PR #19100. It contains the following changes to fix the problems and to improve the UART initialization: - If `LOG_LEVEL` is greater or equal 4, such as in `tests/log_printfnoformat`, the ESP-IDF config function called for the GPIO pins of the UART will output the configuration with `printf` before the `_GLOBAL_REENT` structure is initialized. This causes a crash during system startup. Therefore the initialization by `syscalls_init` must be called by `earlier in the startup procedure. - Since PR #19100 it is possible to define: - other pins for `UART_DEV(0)` than the default pins - different `UART_DEV(0)` pins for the bootloader and RIOT To allow correct reinitialization of the UART pins used by the bootloader as well as their usage for other purposes, the pin usage for the default UART0 pins and the UART pinsused by the bootloader are reset to `_GPIO`. This is done in `uart_system_init` which has to be called earlier in the startup procedure. - To avoid garbage on reconfiguring the UART console pins, e.g. in initialization of the `arduino` module, pins that are already configured as UART pins must not be initialized. - To avoid a several msec long LOW pulse resulting in some garbage during the UART initialization, the TX line is set to HIGH and temporarily configured as a pull-up open-drain output before configuring it as a push-pull output. The PR requires a backport to 2023.1 ### Testing procedure The following tests should work with this PR: - [ ] `tests/log_color` - [ ] `tests/log_printfnoformat` - [ ] `tests/sys_arduino` ### Issues/PRs references Fixes #19138 Co-authored-by: Gunar Schorcht <[email protected]>

19192: cpu/esp32: fixes for boot issues and crashes on ESP32 r=gschorcht a=Flole998 ### Contribution description In syscalls_init() there is a call to malloc(), which will return NULL if the heap is not initialized before, causing the entire board to fail booting if MODULE_ESP_IDF_HEAP is used. API of [vTaskDelete()](https://www.freertos.org/a00126.html) says, that if NULL is passed to vTaskDelete the calling task should be deleted. This PR needs a backport to 2023.01. ### Testing procedure Just compiling on the ESP32S2 and running it with WiFi caused it to not start anymore, no output, nothing. When Null is written to that null-pointer it hangs. The second commit fixed an issue/assertion fail that happens when the WiFi connection drops/disconnects. ### Issues/PRs references Issue was introduced with PR #19146 Co-authored-by: Flole998 <[email protected]>

19192: cpu/esp32: fixes for boot issues and crashes on ESP32 r=kaspar030 a=Flole998 ### Contribution description In syscalls_init() there is a call to malloc(), which will return NULL if the heap is not initialized before, causing the entire board to fail booting if MODULE_ESP_IDF_HEAP is used. API of [vTaskDelete()](https://www.freertos.org/a00126.html) says, that if NULL is passed to vTaskDelete the calling task should be deleted. This PR needs a backport to 2023.01. ### Testing procedure Just compiling on the ESP32S2 and running it with WiFi caused it to not start anymore, no output, nothing. When Null is written to that null-pointer it hangs. The second commit fixed an issue/assertion fail that happens when the WiFi connection drops/disconnects. ### Issues/PRs references Issue was introduced with PR #19146 Co-authored-by: Flole998 <[email protected]>

19196: cpu/esp32: fixes for boot issues and crashes on ESP32 [backport 2023.1] r=benpicco a=gschorcht # Backport of PR #19192 ### Contribution description In `syscalls_init()` there is a call to `malloc()`, which will return `NULL` if the heap is not initialized before, causing the entire board to fail booting if module `esp_idf_heap` is used, for example if the WiFi network interface is used. API of [vTaskDelete()](https://www.freertos.org/a00126.html) says, that if NULL is passed to vTaskDelete the calling task should be deleted. This PR needs a backport to 2023.01. ### Testing procedure ``` USEMODULE=esp_idf_heap BOARD=esp32-wroom-32 make -C tests/malloc ```` should work now but hangs in startup without the PR. ### Issues/PRs references Issue was introduced with PR #19146 Co-authored-by: Flole998 <[email protected]>

gschorcht added 5 commits January 14, 2023 14:27

cpu/esp32/bootloader: fix the UART pin configuration

9004867

cpu/esp32: improve UART initialization

3085b92

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.

cpu/esp32: don't initialize the UART pins if already initialized

f933fde

To avoid garbage on reconfiguring the UART console pins, e.g. in initialization of the `arduino` module, pins that are already configured as UART pins must not be initialized.

github-actions bot added Area: cpu Area: CPU/MCU ports Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Jan 14, 2023

gschorcht added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 14, 2023

gschorcht requested a review from benpicco January 14, 2023 13:42

kaspar030 added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Jan 14, 2023

maribu approved these changes Jan 14, 2023

View reviewed changes

benpicco reviewed Jan 14, 2023

View reviewed changes

bors bot merged commit e70fd44 into RIOT-OS:master Jan 14, 2023

gschorcht mentioned this pull request Jan 15, 2023

cpu/esp32: fix and improve UART initialization [backport 2023.01] #19148

Merged

3 tasks

gschorcht mentioned this pull request Jan 24, 2023

cpu/esp32: fixes for boot issues and crashes on ESP32 #19192

Merged

gschorcht mentioned this pull request Jan 25, 2023

cpu/esp32: fixes for boot issues and crashes on ESP32 [backport 2023.1] #19196

Merged

gschorcht deleted the cpu/esp32/fix_uart_initialization branch January 31, 2023 19:45

jia200x added this to the Release 2023.04 milestone Apr 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

cpu/esp32: fix and improve UART initialization #19146

cpu/esp32: fix and improve UART initialization #19146

Uh oh!

gschorcht commented Jan 14, 2023 •

edited

Loading

Uh oh!

riot-ci commented Jan 14, 2023 •

edited

Loading

Uh oh!

maribu commented Jan 14, 2023

Uh oh!

maribu left a comment

Uh oh!

bors bot commented Jan 14, 2023

Uh oh!

benpicco Jan 14, 2023

Uh oh!

gschorcht Jan 14, 2023

Uh oh!

benpicco Jan 14, 2023

Uh oh!

benpicco commented Jan 14, 2023

Uh oh!

bors bot commented Jan 14, 2023

Uh oh!

gschorcht commented Jan 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

cpu/esp32: fix and improve UART initialization #19146

cpu/esp32: fix and improve UART initialization #19146

Uh oh!

Conversation

gschorcht commented Jan 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

riot-ci commented Jan 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Murdock results

Artifacts

Uh oh!

maribu commented Jan 14, 2023

Uh oh!

maribu left a comment

Choose a reason for hiding this comment

Uh oh!

bors bot commented Jan 14, 2023

Uh oh!

benpicco Jan 14, 2023

Choose a reason for hiding this comment

Uh oh!

gschorcht Jan 14, 2023

Choose a reason for hiding this comment

Uh oh!

benpicco Jan 14, 2023

Choose a reason for hiding this comment

Uh oh!

benpicco commented Jan 14, 2023

Uh oh!

bors bot commented Jan 14, 2023

Uh oh!

gschorcht commented Jan 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

gschorcht commented Jan 14, 2023 •

edited

Loading

riot-ci commented Jan 14, 2023 •

edited

Loading