Skip to content

cpu/esp32: fix pm_reboot function#18207

Merged
benpicco merged 1 commit intoRIOT-OS:masterfrom
gschorcht:cpu/esp32/fix_shell_reboot
Jun 16, 2022
Merged

cpu/esp32: fix pm_reboot function#18207
benpicco merged 1 commit intoRIOT-OS:masterfrom
gschorcht:cpu/esp32/fix_shell_reboot

Conversation

@gschorcht
Copy link
Copy Markdown
Contributor

@gschorcht gschorcht commented Jun 14, 2022

Contribution description

This PR fixes the blocking when the reboot command is used in shell under certain conditions and the pm_reboot function tries to reboot the system.

In the test run of PR #18202 in CI the test of tests/shell failed because the system blocks when executing command reboot if another command was executed before, see the log, see the log. The reason for this is that the system will block while waiting for the UART interfaces to become idle.

Testing procedure

  1. Make BOARD=esp32-wroom-32 make -C tests/shell flash term and execute:

    > reboot
> help
> reboot

    Without this PR, the system blocks on second reboot command. With this PR, the command sequence works as expected.

  2. Execute BOARD=esp32-wroom-32 make -C tests/shell test
    Without this PR, the test fails. With this PR, the test should succeed.

Issues/PRs references

Found when testing PR #18202

@gschorcht gschorcht requested a review from kaspar030 as a code owner June 14, 2022 21:06
@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Platform: ESP Platform: This PR/issue effects ESP-based platforms labels Jun 14, 2022
@gschorcht 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 and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 14, 2022
benpicco
benpicco reviewed Jun 15, 2022
View reviewed changes
@gschorcht gschorcht mentioned this pull request Jun 16, 2022
Merged
@gschorcht
Copy link
Copy Markdown
Contributor Author

gschorcht commented Jun 16, 2022

CI compilation failed only because of the strange hash problem which is only caused by the additional .debug information in the ELF file. So, we could skip the compilation test next time.

benpicco
benpicco approved these changes Jun 16, 2022
View reviewed changes
Copy link
Copy Markdown
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.

Change makes sense, but this could need a comment reassuring future readers that calling uart_tx_wait_idle() on non-initialized UARTs is fine to avoid raising eyebrows.

@gschorcht
Copy link
Copy Markdown
Contributor Author

gschorcht commented Jun 16, 2022

Change makes sense, but this could need a comment reassuring future readers that calling uart_tx_wait_idle() on non-initialized UARTs is fine to avoid raising eyebrows.

Ok, I will add a comment. Since the PR and this change is so small I guess that I can squash directly. I also guess that we could skip compilation for this change then to avoid a further complete CI compilation.

@gschorcht gschorcht added the CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs label Jun 16, 2022
@gschorcht gschorcht force-pushed the cpu/esp32/fix_shell_reboot branch from 3079162 to eb2acc6 Compare June 16, 2022 11:29
@gschorcht gschorcht force-pushed the cpu/esp32/fix_shell_reboot branch from eb2acc6 to 0622d45 Compare June 16, 2022 11:41
@benpicco benpicco enabled auto-merge June 16, 2022 11:48
@benpicco benpicco merged commit 809f9ed into RIOT-OS:master Jun 16, 2022
@gschorcht gschorcht deleted the cpu/esp32/fix_shell_reboot branch June 20, 2022 16:52
@chrysn chrysn added this to the Release 2022.07 milestone Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benpicco benpicco benpicco approved these changes

@kaspar030 kaspar030 Awaiting requested review from kaspar030 kaspar030 is a code owner

Assignees

No one assigned

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 CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2022.07

Development

Successfully merging this pull request may close these issues.

3 participants

@gschorcht @benpicco @chrysn