From f377471e440d8be0b2f9e9c8877ed015f62dc19e Mon Sep 17 00:00:00 2001 From: Finomnis Date: Wed, 6 Dec 2023 19:36:06 +0100 Subject: [PATCH] Fix `nrf::rtc` errata workaround (#858) * Deprecate `should_dequeue_check` * Fix errata by delaying the wakeup point * Add changelog * Fix changelog typos --- rtic-monotonics/CHANGELOG.md | 5 +++-- rtic-monotonics/src/nrf/rtc.rs | 26 ++++++++++++++++++-------- rtic-time/CHANGELOG.md | 1 + rtic-time/Cargo.toml | 2 +- rtic-time/src/lib.rs | 4 ++-- rtic-time/src/monotonic.rs | 14 +++++++++----- 6 files changed, 34 insertions(+), 18 deletions(-) diff --git a/rtic-monotonics/CHANGELOG.md b/rtic-monotonics/CHANGELOG.md index d6b43e2eea..041cff974c 100644 --- a/rtic-monotonics/CHANGELOG.md +++ b/rtic-monotonics/CHANGELOG.md @@ -9,8 +9,9 @@ For each category, *Added*, *Changed*, *Fixed* add new entries at the top! ### Fixed -- Race condition in `nrf::timer`. -- Race condition in `nrf::rtc`. +- Fix race condition in `nrf::timer`. +- Fix race condition in `nrf::rtc`. +- Fix errata in `nrf::rtc`. - Add internal counter integrity check to all half-period based monotonics. ## v1.4.0 - 2023-12-04 diff --git a/rtic-monotonics/src/nrf/rtc.rs b/rtic-monotonics/src/nrf/rtc.rs index 884b523ada..643d8bdb1c 100644 --- a/rtic-monotonics/src/nrf/rtc.rs +++ b/rtic-monotonics/src/nrf/rtc.rs @@ -243,19 +243,29 @@ macro_rules! make_rtc { } } - // NOTE: To fix errata for RTC, if the release time is within 4 ticks - // we release as the RTC will not generate a compare interrupt... - fn should_dequeue_check(release_at: Self::Instant) -> bool { - Self::now() + ::Duration::from_ticks(4) >= release_at - } - fn enable_timer() {} fn disable_timer() {} - fn set_compare(instant: Self::Instant) { + fn set_compare(mut instant: Self::Instant) { let rtc = unsafe { &*$rtc::PTR }; - unsafe { rtc.cc[0].write(|w| w.bits(instant.ticks() as u32 & 0xff_ffff)) }; + + // Disable interrupts because this section is timing critical. + // We rely on the fact that this entire section runs within one + // RTC clock tick. (which it will do easily if it doesn't get + // interrupted) + critical_section::with(|_|{ + let now = Self::now(); + if let Some(diff) = instant.checked_duration_since(now) { + // Errata: Timer interrupts don't fire if they are scheduled less than + // two ticks in the future. Make it three, because the timer could + // tick right now. + if diff.ticks() < 3 { + instant = Self::Instant::from_ticks(now.ticks().wrapping_add(3)); + } + unsafe { rtc.cc[0].write(|w| w.bits(instant.ticks() as u32 & 0xff_ffff)) }; + } + }); } fn clear_compare_flag() { diff --git a/rtic-time/CHANGELOG.md b/rtic-time/CHANGELOG.md index 2e02a9d024..8a99633aa8 100644 --- a/rtic-time/CHANGELOG.md +++ b/rtic-time/CHANGELOG.md @@ -12,6 +12,7 @@ For each category, *Added*, *Changed*, *Fixed* add new entries at the top! ### Changed - Docs: Add sanity check to `half_period_counter` code example +- Deprecate `Monotonic::should_dequeue_check` as it was erroneous ### Fixed diff --git a/rtic-time/Cargo.toml b/rtic-time/Cargo.toml index 8d2851ac06..de23c0f408 100644 --- a/rtic-time/Cargo.toml +++ b/rtic-time/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rtic-time" -version = "1.1.0" +version = "1.2.0" edition = "2021" authors = [ diff --git a/rtic-time/src/lib.rs b/rtic-time/src/lib.rs index 6254bca066..9cd20d5f22 100644 --- a/rtic-time/src/lib.rs +++ b/rtic-time/src/lib.rs @@ -133,7 +133,7 @@ impl TimerQueue { let head = self.queue.pop_if(|head| { release_at = Some(head.release_at); - let should_pop = Mono::should_dequeue_check(head.release_at); + let should_pop = Mono::now() >= head.release_at; head.was_popped.store(should_pop, Ordering::Relaxed); should_pop @@ -147,7 +147,7 @@ impl TimerQueue { Mono::enable_timer(); Mono::set_compare(instant); - if Mono::should_dequeue_check(instant) { + if Mono::now() >= instant { // The time for the next instant passed while handling it, // continue dequeueing continue; diff --git a/rtic-time/src/monotonic.rs b/rtic-time/src/monotonic.rs index 0c0d6f352a..0e8d2b830d 100644 --- a/rtic-time/src/monotonic.rs +++ b/rtic-time/src/monotonic.rs @@ -36,11 +36,15 @@ pub trait Monotonic { /// queue in RTIC checks this. fn set_compare(instant: Self::Instant); - /// Override for the dequeue check, override with timers that have bugs. - /// - /// E.g. nRF52 RTCs needs to be dequeued if the time is within 4 ticks. - fn should_dequeue_check(release_at: Self::Instant) -> bool { - ::now() >= release_at + /// This method used to be required by an errata workaround + /// for the nrf52 family, but it has been disabled as the + /// workaround was erroneous. + #[deprecated( + since = "1.2.0", + note = "this method is erroneous and has been disabled" + )] + fn should_dequeue_check(_: Self::Instant) -> bool { + panic!("This method should not be used as it is erroneous.") } /// Clear the compare interrupt flag.