cpu/esp32: periph uart cleanups #11276
Conversation
An additional _ for static symbols has been added by mistake and should be removed. This will make future merging with the reimplementation of ESP8266 easier.
For consistency reasons, internal function _uart_config was moved to the section of internal functions.
For consistency reasons, external functions were moved to the section of external functions.
The interrupt handler is only used internally and declared to be static.
Function uart_set_baudrate which is only used internally was made static and renamed to _uart_set_baudrate to indicate that it is an internal function. Furthermore, an additional waiting for flushed TX FIFO added. The reconfiguration is now handled as critical section.
gschorcht
added
Type: cleanup
MrKevinWeiss
added
the
CI: ready for build
There was a problem hiding this comment.
Well I tested on the esp32-wroom and it seems to be fine.
I did notice an issue when leaving the GPIO9 and GPIO10 connected after a reset then trying to init gives some weird output then crashes. This is in master as well so maybe it is me?
# Have GPIO9 and GPIO10 connected
make BOARD=esp32-wroom-32 -C tests/periph_uart flash term
# After term is connected
init 1 9600
# it crashes, wait until reset
# disconnect gpio pins
init 1 9600
# it works...
# reconnect the gpio pins for loopback
send 1 test
# it works
# init again with gpio connected
init 1 9600
send 1 test
# it works
# press the reset button with gpio left in
init 1 9600
# it crashes
Seeing as this is in master as well, I don't think it is an issue for the PR but I would like someone else to verify or explain why this happens...
|
The crash output I get |
|
@MrKevinWeiss Thank you for testing and figuring out the problem.
Hm ... interessting. I haven't seen this problem before with the boards I used for testing. But, I have a board where I can reproduce it. The crash output is the beginning of an exception message. GPIO9 and GPIO10 are critical on some boards because they are connected as additional data lines to the SPI flash memory for the QOUT and QIO flash modes. Normally, these GPIOs should be available when QOUT/QIO flash modes are not used. Obviously this does not seem to be the case with some boards. I have to think about whether it makes sense to forbit the use of these GPIOs to avoid such problems. Could you test again with other GPIOs by overwriting the default board configuration: |
|
@MrKevinWeiss Thanks you again for figuring out this problem. I could also reproduce it with The problem was caused by initialization order of RX and TX GPIOs. Unfortunatly, TX GPIO was initialized first. This might lead to crashes if RX GPIO was initialized as output before. Normally, all GPIOs should be initialized as inputs after startup, and the order of GPIO initialization should not matter. However since GPIO9 and GPIO10 are used as data lines during boot, the mode of GPIO9 could be output mode. I have provided an additional commit to this PR which fixes this Problem. It would be great, if you could check again whether it now also works with your board. |
The GPIO for RX has to be initialized as input before the GPIO for TX can be initialized as output. Otherwise it could lead to creash if RX GPIO was used as output before.
|
Very good, I will test in two hours but everything looks good. |
|
@MrKevinWeiss Thanks |
Contribution description
This PR contains different cleanups which will make future merging with the reimplementation of ESP8266 easier. These are in detail:
_of static symbols that has been added by mistake._uart_configmoved to internal function section.uart_set_baudratewhich is used only internally as static and renames it to_uart_set_baudrate.Testing procedure
Compile and flash the
tests/uartapplication for a board that definesUART_DEV(1), e.g.,esp32-wroom-32:Connect GPIO9 and GPIO10 and execute the following test in terminal program
You should be able to observe the following output:
Issues/PRs references
Prerequisite for PR #11231