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

@@ -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

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() + <Self as Monotonic>::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() {

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
 

rtic-time/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rtic-time"
-version = "1.1.0"
+version = "1.2.0"
 
 edition = "2021"
 authors = [

rtic-time/src/lib.rs

@@ -133,7 +133,7 @@ impl<Mono: Monotonic> TimerQueue<Mono> {
             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<Mono: Monotonic> TimerQueue<Mono> {
                     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;

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 {
-        <Self as Monotonic>::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.