-
Notifications
You must be signed in to change notification settings - Fork 171
add asm::delay #84
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
add asm::delay #84
Conversation
bors try |
src/asm.rs
Outdated
/// level. | ||
/// | ||
/// NOTE that the delay can take much longer if interrupts are serviced during its execution. | ||
#[inline(never)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without #[inline(never)]
the compiler seems to misoptimize this function: it doesn't reload the $0
register on every invocation but instead it starts decrementing it from its previous value (0).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not misoptimized, it is intended behavior. You used the "r" constraint and promised to the compiler that you will not modify the value in this register. There is a way to fix it, use "r+" modifier to let compiler know that you will write this input register. Or you can copy the value in some other register, add it to the clobber list and modify it instead of input register.
tryTimed out |
That's a goodie. Far better than the loops I've been using every now and then. I agree with @pftbest that you'll need to change the register clobbers to make it not be inline, also I'd prefer something a bit more obvious than a delay calculated in times 3 cycles. Maybe throw in a NOP for good measure to make it stretch it a cycle to a more obvious 4? I don't think interrupts are going to be a problem. Someone using this mechanism usually only cares about a minimum delay, not absolute precision. |
I see why it doesn't make sense to implement it in embedded-hal, but I also have done some research and the requirement for small delays seems not to be as uncommon as I first thought. Would it make sense to have something like 'BogoMips' and corresponding busy waiting traits in embedded-hal, like they are used in the Linux Kernel? I'm not an OS or experienced embedded guy, so I might be totally wrong with this. |
addressed the review comments bors r+ |
84: add asm::delay r=japaric a=japaric This PR adds an `asm::delay` function that can be used to block the program for the specified number of instruction cycles (NB in absence of interrupts, see API docs). The implementation is done in assembly so the execution time of that function is the same regardless of the used optimization level. Rationale: external devices (sensors) sometimes require delays in their initialization, or to workaround silicon errata. Having to set up a hardware peripheral like the SysTick can be overkill when the delay is to be done once. Why not provide an implementation of `embedded_hal::{DelayUs, DelayMs}`? Going from instruction cycles to human time requires knowing the clock configuration and in some cases the Flash access latency. As all that information is device specific it's best left to device crates to implement. Thoughts? Perhaps this is too error prone due to Flash access latency and interrupts? The interrupt issue could be avoided by calling the asm! block within an interrupt::free call. cc @therealprof @kunerd Co-authored-by: Jorge Aparicio <[email protected]>
Build succeeded |
84: refactor the linker script r=therealprof a=japaric to make it more compatible with LLD. This commit contains no functional changes. fixes #70 Overview of changes: - Alignment checks are enabled now that rust-lld (LLD 7.0) supports the modulo operator. - Removed some private symbols (e.g. __foo) in favor of ADDR and SIZEOF. - Turned .got into a NOLOAD section now that rust-lld supports it. - Replaced `ABSOLUTE(.)` with `.` as an old LLD overlap bug seems to be gone and ABSOLUTE seems to cause problems, like #70, on bigger programs. - Made the linker assertion messages more uniform. - Extended test suite to check that linking works with both rust-lld and GNU LD. r? therealprof (chosen at random) Co-authored-by: Jorge Aparicio <[email protected]>
This PR adds an
asm::delay
function that can be used to block the program for the specified numberof instruction cycles (NB in absence of interrupts, see API docs). The implementation is done in
assembly so the execution time of that function is the same regardless of the used optimization
level.
Rationale: external devices (sensors) sometimes require delays in their initialization, or to
workaround silicon errata. Having to set up a hardware peripheral like the SysTick can be overkill
when the delay is to be done once.
Why not provide an implementation of
embedded_hal::{DelayUs, DelayMs}
? Going from instructioncycles to human time requires knowing the clock configuration and in some cases the Flash access
latency. As all that information is device specific it's best left to device crates to implement.
Thoughts? Perhaps this is too error prone due to Flash access latency and interrupts? The interrupt
issue could be avoided by calling the asm! block within an interrupt::free call.
cc @therealprof @kunerd