Fix nrf monotonics (#852)

* Fix nrf::timer

* Bootstrap nrf52840-blinky example

* More work on nrf blinky example

* Fix README

* Add asserts for correct timer functionality

* Add correctness check to other monotonics as well

* Update Changelog

* Fix potential timing issues

* Fix race condition in nrf::rtc

* Add changelog

* Add rtc blinky example

* Change rtc example to RC lf clock source

* Add changelog to rtic-time

* Add changelog

* Attempt to fix CI

* Update teensy4-blinky Cargo.lock
This commit is contained in:
Finomnis 2023-12-06 08:49:38 +01:00 committed by GitHub
parent 1622f6b953
commit 89160b7cb9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 1080 additions and 101 deletions

View file

@ -7,6 +7,12 @@ For each category, *Added*, *Changed*, *Fixed* add new entries at the top!
## Unreleased
### Fixed
- Race condition in `nrf::timer`.
- Race condition in `nrf::rtc`.
- Add internal counter integrity check to all half-period based monotonics.
## v1.4.0 - 2023-12-04
### Fixed

View file

@ -141,13 +141,15 @@ macro_rules! make_timer {
// so it gets combined with rollover interrupt
ral::write_reg!(ral::gpt, gpt, OCR[1], 0x0000_0000);
// Initialize timer queue
$tq.initialize(Self {});
// Enable the timer
ral::modify_reg!(ral::gpt, gpt, CR, EN: 1);
ral::modify_reg!(ral::gpt, gpt, CR,
ENMOD: 0, // Keep state when disabled
);
$tq.initialize(Self {});
// SAFETY: We take full ownership of the peripheral and interrupt vector,
// plus we are not using any external shared resources so we won't impact
@ -244,13 +246,15 @@ macro_rules! make_timer {
let (rollover, half_rollover) = ral::read_reg!(ral::gpt, gpt, SR, ROV, OF1);
if rollover != 0 {
$period.fetch_add(1, Ordering::Relaxed);
let prev = $period.fetch_add(1, Ordering::Relaxed);
ral::write_reg!(ral::gpt, gpt, SR, ROV: 1);
assert!(prev % 2 == 1, "Monotonic must have skipped an interrupt!");
}
if half_rollover != 0 {
$period.fetch_add(1, Ordering::Relaxed);
let prev = $period.fetch_add(1, Ordering::Relaxed);
ral::write_reg!(ral::gpt, gpt, SR, OF1: 1);
assert!(prev % 2 == 0, "Monotonic must have skipped an interrupt!");
}
}
}

View file

@ -44,6 +44,7 @@ use crate::{Monotonic, TimeoutError, TimerQueue};
use atomic_polyfill::{AtomicU32, Ordering};
use core::future::Future;
pub use fugit::{self, ExtU64, ExtU64Ceil};
use rtic_time::half_period_counter::calculate_now;
#[doc(hidden)]
#[macro_export]
@ -92,6 +93,16 @@ macro_rules! create_nrf_rtc2_monotonic_token {
}};
}
struct TimerValueU24(u32);
impl rtic_time::half_period_counter::TimerValue for TimerValueU24 {
const BITS: u32 = 24;
}
impl From<TimerValueU24> for u64 {
fn from(value: TimerValueU24) -> Self {
Self::from(value.0)
}
}
macro_rules! make_rtc {
($mono_name:ident, $rtc:ident, $overflow:ident, $tq:ident$(, doc: ($($doc:tt)*))?) => {
/// Monotonic timer queue implementation.
@ -107,13 +118,49 @@ macro_rules! make_rtc {
/// Start the timer monotonic.
pub fn start(rtc: $rtc, _interrupt_token: impl crate::InterruptToken<Self>) {
unsafe { rtc.prescaler.write(|w| w.bits(0)) };
rtc.intenset.write(|w| w.compare0().set().ovrflw().set());
rtc.evtenset.write(|w| w.compare0().set().ovrflw().set());
rtc.tasks_clear.write(|w| unsafe { w.bits(1) });
rtc.tasks_start.write(|w| unsafe { w.bits(1) });
// Disable interrupts, as preparation
rtc.intenclr.write(|w| w
.compare0().clear()
.compare1().clear()
.ovrflw().clear()
);
$tq.initialize(Self {});
// Configure compare registers
rtc.cc[0].write(|w| unsafe { w.bits(0) }); // Dynamic wakeup
rtc.cc[1].write(|w| unsafe { w.bits(0x80_0000) }); // Half-period
// Timing critical, make sure we don't get interrupted
critical_section::with(|_|{
// Reset the timer
rtc.tasks_clear.write(|w| unsafe { w.bits(1) });
rtc.tasks_start.write(|w| unsafe { w.bits(1) });
// Clear pending events.
// Should be close enough to the timer reset that we don't miss any events.
rtc.events_ovrflw.write(|w| w);
rtc.events_compare[0].write(|w| w);
rtc.events_compare[1].write(|w| w);
// Make sure overflow counter is synced with the timer value
$overflow.store(0, Ordering::SeqCst);
// Initialized the timer queue
$tq.initialize(Self {});
// Enable interrupts.
// Should be close enough to the timer reset that we don't miss any events.
rtc.intenset.write(|w| w
.compare0().set()
.compare1().set()
.ovrflw().set()
);
rtc.evtenset.write(|w| w
.compare0().set()
.compare1().set()
.ovrflw().set()
);
});
// SAFETY: We take full ownership of the peripheral and interrupt vector,
// plus we are not using any external shared resources so we won't impact
@ -130,12 +177,6 @@ macro_rules! make_rtc {
&$tq
}
#[inline(always)]
fn is_overflow() -> bool {
let rtc = unsafe { &*$rtc::PTR };
rtc.events_ovrflw.read().bits() == 1
}
/// Timeout at a specific time.
#[inline]
pub async fn timeout_at<F: Future>(
@ -181,31 +222,24 @@ macro_rules! make_rtc {
type Duration = fugit::TimerDurationU64<32_768>;
fn now() -> Self::Instant {
// In a critical section to not get a race between overflow updates and reading it
// and the flag here.
critical_section::with(|_| {
let rtc = unsafe { &*$rtc::PTR };
let cnt = rtc.counter.read().bits();
// OVERFLOW HAPPENS HERE race needs to be handled
let ovf = if Self::is_overflow() {
$overflow.load(Ordering::Relaxed) + 1
} else {
$overflow.load(Ordering::Relaxed)
} as u64;
// Check and fix if above race happened
let new_cnt = rtc.counter.read().bits();
let cnt = if new_cnt >= cnt { cnt } else { new_cnt } as u64;
Self::Instant::from_ticks((ovf << 24) | cnt)
})
let rtc = unsafe { &*$rtc::PTR };
Self::Instant::from_ticks(calculate_now(
$overflow.load(Ordering::Relaxed),
|| TimerValueU24(rtc.counter.read().bits())
))
}
fn on_interrupt() {
let rtc = unsafe { &*$rtc::PTR };
if Self::is_overflow() {
$overflow.fetch_add(1, Ordering::SeqCst);
if rtc.events_ovrflw.read().bits() == 1 {
rtc.events_ovrflw.write(|w| unsafe { w.bits(0) });
let prev = $overflow.fetch_add(1, Ordering::Relaxed);
assert!(prev % 2 == 1, "Monotonic must have skipped an interrupt!");
}
if rtc.events_compare[1].read().bits() == 1 {
rtc.events_compare[1].write(|w| unsafe { w.bits(0) });
let prev = $overflow.fetch_add(1, Ordering::Relaxed);
assert!(prev % 2 == 0, "Monotonic must have skipped an interrupt!");
}
}
@ -221,7 +255,7 @@ macro_rules! make_rtc {
fn set_compare(instant: Self::Instant) {
let rtc = unsafe { &*$rtc::PTR };
unsafe { rtc.cc[0].write(|w| w.bits(instant.ticks() as u32 & 0xffffff)) };
unsafe { rtc.cc[0].write(|w| w.bits(instant.ticks() as u32 & 0xff_ffff)) };
}
fn clear_compare_flag() {

View file

@ -30,6 +30,7 @@ use crate::{Monotonic, TimeoutError, TimerQueue};
use atomic_polyfill::{AtomicU32, Ordering};
use core::future::Future;
pub use fugit::{self, ExtU64, ExtU64Ceil};
use rtic_time::half_period_counter::calculate_now;
#[cfg(feature = "nrf52810")]
use nrf52810_pac::{self as pac, Interrupt, TIMER0, TIMER1, TIMER2};
@ -139,17 +140,45 @@ macro_rules! make_timer {
// 1 MHz
timer.prescaler.write(|w| unsafe { w.prescaler().bits(4) });
timer.bitmode.write(|w| w.bitmode()._32bit());
timer
.intenset
.modify(|_, w| w.compare0().set().compare1().set());
timer.cc[1].write(|w| unsafe { w.cc().bits(0) }); // Overflow
timer.tasks_clear.write(|w| unsafe { w.bits(1) });
timer.tasks_start.write(|w| unsafe { w.bits(1) });
$tq.initialize(Self {});
// Disable interrupts, as preparation
timer.intenclr.modify(|_, w| w
.compare0().clear()
.compare1().clear()
.compare2().clear()
);
timer.events_compare[0].write(|w| w);
timer.events_compare[1].write(|w| w);
// Configure compare registers
timer.cc[0].write(|w| unsafe { w.cc().bits(0) }); // Dynamic wakeup
timer.cc[1].write(|w| unsafe { w.cc().bits(0x0000_0000) }); // Overflow
timer.cc[2].write(|w| unsafe { w.cc().bits(0x8000_0000) }); // Half-period
// Timing critical, make sure we don't get interrupted
critical_section::with(|_|{
// Reset the timer
timer.tasks_clear.write(|w| unsafe { w.bits(1) });
timer.tasks_start.write(|w| unsafe { w.bits(1) });
// Clear pending events.
// Should be close enough to the timer reset that we don't miss any events.
timer.events_compare[0].write(|w| w);
timer.events_compare[1].write(|w| w);
timer.events_compare[2].write(|w| w);
// Make sure overflow counter is synced with the timer value
$overflow.store(0, Ordering::SeqCst);
// Initialized the timer queue
$tq.initialize(Self {});
// Enable interrupts.
// Should be close enough to the timer reset that we don't miss any events.
timer.intenset.modify(|_, w| w
.compare0().set()
.compare1().set()
.compare2().set()
);
});
// SAFETY: We take full ownership of the peripheral and interrupt vector,
// plus we are not using any external shared resources so we won't impact
@ -166,12 +195,6 @@ macro_rules! make_timer {
&$tq
}
#[inline(always)]
fn is_overflow() -> bool {
let timer = unsafe { &*$timer::PTR };
timer.events_compare[1].read().bits() & 1 != 0
}
/// Timeout at a specific time.
#[inline]
pub async fn timeout_at<F: Future>(
@ -216,45 +239,35 @@ macro_rules! make_timer {
type Duration = fugit::TimerDurationU64<1_000_000>;
fn now() -> Self::Instant {
// In a critical section to not get a race between overflow updates and reading it
// and the flag here.
critical_section::with(|_| {
let timer = unsafe { &*$timer::PTR };
timer.tasks_capture[2].write(|w| unsafe { w.bits(1) });
let cnt = timer.cc[2].read().bits();
let timer = unsafe { &*$timer::PTR };
let unhandled_overflow = if Self::is_overflow() {
// The overflow has not been handled yet, so add an extra to the read overflow.
1
} else {
0
};
timer.tasks_capture[2].write(|w| unsafe { w.bits(1) });
let new_cnt = timer.cc[2].read().bits();
let cnt = if new_cnt >= cnt { cnt } else { new_cnt } as u64;
Self::Instant::from_ticks(
(unhandled_overflow + $overflow.load(Ordering::Relaxed) as u64) << 32
| cnt as u64,
)
})
Self::Instant::from_ticks(calculate_now(
$overflow.load(Ordering::Relaxed),
|| {
timer.tasks_capture[3].write(|w| unsafe { w.bits(1) });
timer.cc[3].read().bits()
}
))
}
fn on_interrupt() {
let timer = unsafe { &*$timer::PTR };
// If there is a compare match on channel 1, it is an overflow
if Self::is_overflow() {
if timer.events_compare[1].read().bits() & 1 != 0 {
timer.events_compare[1].write(|w| w);
$overflow.fetch_add(1, Ordering::SeqCst);
let prev = $overflow.fetch_add(1, Ordering::Relaxed);
assert!(prev % 2 == 1, "Monotonic must have skipped an interrupt!");
}
// If there is a compare match on channel 2, it is a half-period overflow
if timer.events_compare[2].read().bits() & 1 != 0 {
timer.events_compare[2].write(|w| w);
let prev = $overflow.fetch_add(1, Ordering::Relaxed);
assert!(prev % 2 == 0, "Monotonic must have skipped an interrupt!");
}
}
fn enable_timer() {}
fn disable_timer() {}
fn set_compare(instant: Self::Instant) {
let timer = unsafe { &*$timer::PTR };
timer.cc[0].write(|w| unsafe { w.cc().bits(instant.ticks() as u32) });

View file

@ -272,12 +272,14 @@ macro_rules! make_timer {
// Full period
if $timer.sr().read().uif() {
$timer.sr().modify(|r| r.set_uif(false));
$overflow.fetch_add(1, Ordering::Relaxed);
let prev = $overflow.fetch_add(1, Ordering::Relaxed);
assert!(prev % 2 == 1, "Monotonic must have missed an interrupt!");
}
// Half period
if $timer.sr().read().ccif(2) {
$timer.sr().modify(|r| r.set_ccif(2, false));
$overflow.fetch_add(1, Ordering::Relaxed);
let prev = $overflow.fetch_add(1, Ordering::Relaxed);
assert!(prev % 2 == 0, "Monotonic must have missed an interrupt!");
}
}
}