Skip to content

mbed_die: Use wait_us not wait_ms #9547

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.

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
merged 1 commit into from
Jan 30, 2019
Merged

Conversation

kjbracey
Copy link
Contributor

Description

mbed_die was calling wait_ms in a critical section - this is deprecated behaviour, and caused a fatal error in debug builds, potentially leading to an infinite reboot loop if this was caused due to a reboot limit halt.

Switch to using wait_us - this is safe in a critical section. This does trigger a call to mbed_warning, due to the large parameter, but that is harmless - it doesn't output anything to the console, and won't
overwrite the error context if it already contains something.

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@SenRamakri, @TeemuKultala

mbed_die was calling wait_ms in a critical section - this is deprecated
behaviour, and caused a fatal error in debug builds, potentially leading
to an infinite reboot loop if this was caused due to a reboot limit
halt.

Switch to using wait_us - this is safe in a critical section. This does
trigger a call to mbed_warning, due to the large parameter, but that is
harmless - it doesn't output anything to the console, and won't
overwrite the error context if it already contains something.
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

We could avoid warning by invoking wait_us multiple times but rather keep it as it is?

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 30, 2019

Waiting for complete the review. Meanwhile CI started

@kjbracey
Copy link
Contributor Author

The warning call was actually already happening if it didn't crash, because wait_ms in develop and release builds just calls wait_us with ms * 1000 itself. So we're not actually making anything worse here.

I don't think it's worth trying to avoid it. Only real downside is if you look at the error capture, if there was no preceding error or warning you will see this wait_us warning, which might mislead.

Maybe something to fix later, but feels like it will be adding code for no huge benefit.

@mbed-ci
Copy link

mbed-ci commented Jan 30, 2019

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 1
Build artifacts

@cmonr cmonr merged commit debec09 into ARMmbed:master Jan 30, 2019
cmonr pushed a commit that referenced this pull request Jan 30, 2019
mbed_die: Use wait_us not wait_ms
@cmonr
Copy link
Contributor

cmonr commented Jan 30, 2019

#9508 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants