Skip to content

Conversation

@gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Jan 6, 2023

Contribution description

This PR

  • fixes the issue for ESP32 SoCs that UART0 signals can't be routed to arbitrary GPIOs and
  • allows the configuration of the UART device used by the bootloader.

The UART interface and its configuration used by the STDIO are defined in RIOT using the define STDIO_UART_DEV and the configuration of the corresponding UART device in periph_conf.h.

However, the bootloader compiled directly in ESP-IDF uses its own definitions CONFIG_ESP_CONSOLE_UART_* for the UART configuration. To be able to use a consistent UART configuration in RIOT and the bootloader, e.g. to see the output of the 2nd stage bootloader, these CONFIG_ESP_CONSOLE_UART_* can be defined via a set of KConfig variables in RIOT (not yet implemented in Kconfig):

  • CONSOLE_CONFIG_UART_NUM defines the UART device to be used by the bootloader and by STDIO_UART_DEV
  • CONSOLE_CONFIG_UART_RX and CONSOLE_CONFIG_UART_TX define the GPIOs to be used by the bootloader and should be the GPIOs as defined in periph_conf.h for the corresponding UART device.

Testing procedure

Any ESP32 node should still work with stdio_uart and the default configuration. To test an alternative configuration, use

CFLAGS='-DUART1_TXD=5 -DUART1_RXD=4 -DCONFIG_CONSOLE_UART_NUM=1 -DCONFIG_CONSOLE_UART_TX=5 -DCONFIG_CONSOLE_UART_RX=4' USEMODULE=esp_log_startup BOARD=esp32-wroom-32 make -C tests/shell flash

The bootloader output and the STDIO should be routed to UART1 at GPIO4 and GPIO5.

Issues/PRs references

Prerequisite for PR ##18863

@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Jan 6, 2023
@gschorcht gschorcht force-pushed the cpu/esp_common/uart0_config branch from 117a8a6 to 382a17f Compare January 6, 2023 14:13
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 6, 2023
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

This breaks stdio on esp8266-esp-12x

@gschorcht
Copy link
Contributor Author

BTW, if this PR works, it should be possible to define GPIO37 and GPIO39 in PR #18863 as UART0 and route the bootloader output to that UART as well by simply adding the following to boards/esp32s2-wemos-mini/Makefile.include:

CFLAGS += -DCONFIG_CONSOLE_UART_TX=39
CFLAGS += -DCONFIG_CONSOLE_UART_RX=37

@gschorcht
Copy link
Contributor Author

This breaks stdio on esp8266-esp-12x

I was afraid of that worried Let me check.

@gschorcht
Copy link
Contributor Author

I'm not sure whether the reconfiguration of UART make sense at all for ESP8266. It has only one full functional UART interface. The TRM for ESP8266 is so pure, that I don't know whether the signals are hardwired or routable. Futhermore, I don't know whether there is a ESP8266 board in universe that doesn't have connected the UART0 to the UART-to-USB bridge. So I would remove the configuration of GPIOs from uart_init.

Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Works now with default config on esp32-wroom-32 and non-default config on esp32s2-wemos-mini.

Please squash

bors merge

@bors
Copy link
Contributor

bors bot commented Jan 6, 2023

👎 Rejected by PR status

@riot-ci
Copy link

riot-ci commented Jan 6, 2023

Murdock results

✔️ PASSED

7230299 cpu/esp32: use same UART device in stdio_uart and bootloader

Success Failures Total Runtime
6769 0 6770 14m:09s

Artifacts

The UART interface and its configuration as used by the STDIO is defined in RIOT using `STDIO_UART_DEV` and the UART configuration in `periph_conf.h`.

However, the bootloader compiled directly in ESP-IDF uses its own definitions `CONFIG_ESP_CONSOLE_UART_*` for the UART configuration. To be able to use a consistent UART configuration in RIOT and the bootloader, e.g. to see the output of the 2nd stage bootloader, these `CONFIG_ESP_CONSOLE_UART_*` can be defined via a set of KConfig variables `CONSOLE_CONFIG_UART_*`. Here the variable `CONSOLE_CONFIG_UART_NUM` is then also used as `STDIO_UART_DEV` and the variables `CONSOLE_CONFIG_UART_RX` and `CONSOLE_CONFIG_UART_TX` of the configuration in `periph_conf.h` should be used accordingly.
f
@gschorcht gschorcht force-pushed the cpu/esp_common/uart0_config branch from 5b788ff to 7230299 Compare January 6, 2023 15:44
@benpicco
Copy link
Contributor

benpicco commented Jan 6, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Jan 6, 2023

🕐 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
Copy link
Contributor

benpicco commented Jan 6, 2023

bors merge

bors bot added a commit that referenced this pull request Jan 6, 2023
18752: nanocoap_sock: deprecate nanocoap_get() r=benpicco a=benpicco





19100: cpu/esp_common: allow configuration of UART0 r=benpicco a=gschorcht

### Contribution description

This PR
- fixes the issue for ESP32 SoCs that UART0 signals can't be routed to arbitrary GPIOs and
- allows the configuration of the UART device used by the bootloader.

The UART interface and its configuration used by the STDIO are defined in RIOT using the define `STDIO_UART_DEV` and the configuration of the corresponding UART device in `periph_conf.h`. 

However, the bootloader compiled directly in ESP-IDF uses its own definitions `CONFIG_ESP_CONSOLE_UART_*` for the UART configuration. To be able to use a consistent UART configuration in RIOT and the bootloader, e.g. to see the output of the 2nd stage bootloader, these `CONFIG_ESP_CONSOLE_UART_*` can be defined via a set of KConfig variables in RIOT (not yet implemented in Kconfig):
- `CONSOLE_CONFIG_UART_NUM` defines the UART device to be used by the bootloader and by `STDIO_UART_DEV`
- `CONSOLE_CONFIG_UART_RX` and `CONSOLE_CONFIG_UART_TX` define the GPIOs to be used by the bootloader and should be the GPIOs as defined in `periph_conf.h` for the corresponding UART device.

### Testing procedure

Any ESP32 node should still work with `stdio_uart` and the default configuration. To test an alternative configuration, use
```
CFLAGS='-DUART1_TXD=5 -DUART1_RXD=4 -DCONFIG_CONSOLE_UART_NUM=1 -DCONFIG_CONSOLE_UART_TX=5 -DCONFIG_CONSOLE_UART_RX=4' USEMODULE=esp_log_startup BOARD=esp32-wroom-32 make -C tests/shell flash
```
The bootloader output and the STDIO should be routed to UART1 at GPIO4 and GPIO5.

### Issues/PRs references

Prerequisite for PR ##18863

19104: tests/periph_uart: only exclude STDIO_UART_DEV if stdio_uart is used r=benpicco a=benpicco



Co-authored-by: Benjamin Valentin <[email protected]>
Co-authored-by: Gunar Schorcht <[email protected]>
Co-authored-by: Benjamin Valentin <[email protected]>
@gschorcht gschorcht closed this Jan 6, 2023
@gschorcht gschorcht deleted the cpu/esp_common/uart0_config branch January 6, 2023 22:52
@bors
Copy link
Contributor

bors bot commented Jan 6, 2023

Build succeeded:

@benpicco
Copy link
Contributor

benpicco commented Jan 6, 2023

Huh, why close it?
Well, looks like bors merged it anyway.

gschorcht added a commit to gschorcht/RIOT-Xtensa-ESP that referenced this pull request Jan 14, 2023
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 by the bootloader are reset to `_GPIO`. This is done in `uart_system_init` which has to be called earlier in the startup procedure.
gschorcht added a commit to gschorcht/RIOT-Xtensa-ESP that referenced this pull request Jan 14, 2023
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 by the bootloader are reset to `_GPIO`. This is done in `uart_system_init` which has to be called earlier in the startup procedure.
gschorcht added a commit to gschorcht/RIOT-Xtensa-ESP that referenced this pull request Jan 14, 2023
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.
bors bot added a commit that referenced this pull request Jan 14, 2023
19146: cpu/esp32: fix and improve UART initialization r=benpicco a=gschorcht

### 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]>
gschorcht added a commit to gschorcht/RIOT-Xtensa-ESP that referenced this pull request Jan 15, 2023
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.
bors bot added a commit that referenced this pull request Jan 15, 2023
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]>
Einhornhool pushed a commit to Einhornhool/RIOT that referenced this pull request Jan 24, 2023
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.
dylad pushed a commit to dylad/RIOT that referenced this pull request Feb 3, 2023
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants