some improvements

examples/embassy/Cargo.toml

@@ -7,14 +7,20 @@ edition = "2021"
 cortex-m = { version = "0.7", features = ["critical-section-single-core"] }
 cortex-m-rt = "0.7"
 embedded-hal = "1"
+
 rtt-target = { version = "0.5" }
 panic-rtt-target = { version = "0.1" }
 critical-section = "1"
 
 embassy-sync = { version = "0.6.0" }
-embassy-time = { version = "0.3.2", features = ["tick-hz-1_000"] }
+embassy-time = { version = "0.3.2", features = ["tick-hz-1"] }
 embassy-time-driver = { version = "0.1" }
 
+[dependencies.once_cell]
+version = "1"
+default-features = false
+features = ["critical-section"]
+
 [dependencies.embassy-executor]
 version = "0.6.0"
 features = [

examples/embassy/src/lib.rs

@@ -3,22 +3,19 @@ use core::{
     cell::Cell,
     mem, ptr,
     sync::atomic::{AtomicU32, AtomicU8, Ordering},
-    time,
 };
 use critical_section::CriticalSection;
 use embassy_sync::blocking_mutex::raw::CriticalSectionRawMutex;
 use embassy_sync::blocking_mutex::Mutex;
 
 use embassy_time_driver::{time_driver_impl, AlarmHandle, Driver, TICK_HZ};
+use rtt_target::rprintln;
 use va416xx_hal::{
     clock::Clocks,
     enable_interrupt,
     irq_router::enable_and_init_irq_router,
     pac::{self, interrupt},
-    pwm::{
-        assert_tim_reset, assert_tim_reset_for_two_cycles, deassert_tim_reset, enable_tim_clk,
-        ValidTim,
-    },
+    pwm::{assert_tim_reset_for_two_cycles, enable_tim_clk, ValidTim},
 };
 
 pub type TimekeeperClk = pac::Tim15;
@@ -26,8 +23,17 @@ pub type AlarmClk0 = pac::Tim14;
 pub type AlarmClk1 = pac::Tim13;
 pub type AlarmClk2 = pac::Tim12;
 
-/// This has to be called to initiate the time driver for embassy.
-pub fn init(
+// Uses integer division to get a margin of 75 % of the base value added on the ticks
+const fn three_quarters_of_period(period: u64) -> u64 {
+    (period * 3) / 4
+}
+
+/// Initialization method for embassy
+///
+/// # Safety
+/// This has to be called once at initialization time to initiate the time driver for
+/// embassy.
+pub unsafe fn init(
     syscfg: &mut pac::Sysconfig,
     irq_router: &pac::IrqRouter,
     timekeeper: TimekeeperClk,
@@ -39,7 +45,7 @@ pub fn init(
 }
 
 const fn alarm_tim(idx: usize) -> &'static pac::tim0::RegisterBlock {
-    // Safety: This is a memory-mapped peripheral.
+    // Safety: This is a static memory-mapped peripheral.
     match idx {
         0 => unsafe { &*AlarmClk0::ptr() },
         1 => unsafe { &*AlarmClk1::ptr() },
@@ -77,8 +83,6 @@ impl AlarmState {
 unsafe impl Send for AlarmState {}
 
 const ALARM_COUNT: usize = 1;
-// Margin value which is used when detecting whether an alarm should fire soon.
-const TIMER_MARGIN: u64 = 0xc000_0000;
 
 pub struct TimerDriverEmbassy {
     periods: AtomicU32,
@@ -98,14 +102,10 @@ impl TimerDriverEmbassy {
         enable_tim_clk(syscfg, TimekeeperClk::TIM_ID);
         assert_tim_reset_for_two_cycles(syscfg, TimekeeperClk::TIM_ID);
 
-        let rst_value = TimekeeperClk::clock(clocks).raw() / TICK_HZ as u32 - 1;
-        // Safety: We have a valid instance of the tim peripheral.
-        timekeeper
-            .rst_value()
-            .write(|w| unsafe { w.bits(rst_value) });
-        timekeeper
-            .cnt_value()
-            .write(|w| unsafe { w.bits(rst_value) });
+        let rst_val = (TimekeeperClk::clock(clocks).raw() / TICK_HZ as u32) - 1;
+        timekeeper.rst_value().write(|w| unsafe { w.bits(rst_val) });
+        // Decrementing counter.
+        timekeeper.cnt_value().write(|w| unsafe { w.bits(rst_val) });
         // Switch on. Timekeeping should always be done.
         unsafe {
             enable_interrupt(TimekeeperClk::IRQ);
@@ -127,28 +127,31 @@ impl TimerDriverEmbassy {
         }
     }
 
+    // Should be called inside the IRQ of the timekeeper timer.
     fn on_interrupt_timekeeping(&self) {
         self.next_period();
     }
 
+    // Should be called inside the IRQ of the alarm timer.
     fn on_interrupt_alarm(&self, idx: usize) {
         critical_section::with(|cs| self.trigger_alarm(idx, cs))
     }
 
     fn next_period(&self) {
         let period = self.periods.fetch_add(1, Ordering::AcqRel) + 1;
+        let rst_val = timekeeping_tim().rst_value().read().bits() as u64;
+        let t = period as u64 * rst_val;
         critical_section::with(|cs| {
             for i in 0..ALARM_COUNT {
                 let alarm = &self.alarms.borrow(cs)[i];
                 let at = alarm.timestamp.get();
-                let t = (period as u64) << 32;
-                if at < t + TIMER_MARGIN {
-                    let rst_val = alarm_tim(i).rst_value().read().bits();
-                    alarm_tim(i)
-                        .cnt_value()
-                        .write(|w| unsafe { w.bits(rst_val) });
-                    alarm_tim(i).ctrl().modify(|_, w| w.irq_enb().set_bit());
-                    alarm_tim(i).enable().write(|w| unsafe { w.bits(1) })
+                let alarm_tim = alarm_tim(0);
+                if at < t + three_quarters_of_period(alarm_tim.rst_value().read().bits() as u64) {
+                    alarm_tim.enable().write(|w| unsafe { w.bits(0) });
+                    let rst_val = alarm_tim.rst_value().read().bits();
+                    alarm_tim.cnt_value().write(|w| unsafe { w.bits(rst_val) });
+                    alarm_tim.ctrl().modify(|_, w| w.irq_enb().set_bit());
+                    alarm_tim.enable().write(|w| unsafe { w.bits(1) })
                 }
             }
         })
@@ -161,10 +164,7 @@ impl TimerDriverEmbassy {
     }
 
     fn trigger_alarm(&self, n: usize, cs: CriticalSection) {
-        alarm_tim(n).ctrl().modify(|_, w| {
-            w.irq_enb().clear_bit();
-            w.enable().clear_bit()
-        });
+        alarm_tim(n).ctrl().modify(|_, w| w.irq_enb().clear_bit());
 
         let alarm = &self.alarms.borrow(cs)[n];
         // Setting the maximum value disables the alarm.
@@ -185,32 +185,39 @@ impl Driver for TimerDriverEmbassy {
         let mut period1: u32;
         let mut period2: u32;
         let mut counter_val: u32;
+
+        let rst_val = timekeeping_tim().rst_value().read().bits();
         loop {
             // Acquire ensures that we get the latest value of `periods` and
             // no instructions can be reordered before the load.
             period1 = self.periods.load(Ordering::Acquire);
 
-            counter_val = timekeeping_tim().rst_value().read().bits()
-                - timekeeping_tim().cnt_value().read().bits();
+            counter_val = rst_val - timekeeping_tim().cnt_value().read().bits();
 
+            // Double read to protect against race conditions when the counter is overflowing.
             period2 = self.periods.load(Ordering::Relaxed);
             if period1 == period2 {
                 break;
             }
         }
-        ((period1 as u64) << 32) | counter_val as u64
+        (period1 as u64 * rst_val as u64) + counter_val as u64
     }
 
-    unsafe fn allocate_alarm(&self) -> Option<embassy_time_driver::AlarmHandle> {
-        critical_section::with(|_| {
-            let id = self.alarm_count.load(Ordering::Relaxed);
-            if id < ALARM_COUNT as u8 {
-                self.alarm_count.store(id + 1, Ordering::Relaxed);
-                Some(AlarmHandle::new(id))
+    unsafe fn allocate_alarm(&self) -> Option<AlarmHandle> {
+        let id = self
+            .alarm_count
+            .fetch_update(Ordering::AcqRel, Ordering::Acquire, |x| {
+                if x < ALARM_COUNT as u8 {
+                    Some(x + 1)
                 } else {
                     None
                 }
-        })
+            });
+
+        match id {
+            Ok(id) => Some(AlarmHandle::new(id)),
+            Err(_) => None,
+        }
     }
 
     fn set_alarm_callback(
@@ -230,20 +237,18 @@ impl Driver for TimerDriverEmbassy {
     fn set_alarm(&self, alarm: embassy_time_driver::AlarmHandle, timestamp: u64) -> bool {
         critical_section::with(|cs| {
             let n = alarm.id();
-            let alarm = self.get_alarm(cs, alarm);
-            alarm.timestamp.set(timestamp);
-
             let alarm_tim = alarm_tim(n.into());
-
-            let t = self.now();
-            if timestamp <= t {
             alarm_tim.ctrl().modify(|_, w| {
                 w.irq_enb().clear_bit();
                 w.enable().clear_bit()
             });
 
-                alarm.timestamp.set(u64::MAX);
+            let alarm = self.get_alarm(cs, alarm);
+            alarm.timestamp.set(timestamp);
 
+            let t = self.now();
+            if timestamp <= t {
+                alarm.timestamp.set(u64::MAX);
                 return false;
             }
 
@@ -258,22 +263,22 @@ impl Driver for TimerDriverEmbassy {
             // by the Alarm trait contract. What's not allowed is triggering alarms *before* their scheduled time,
             // and we don't do that here.
             let safe_timestamp = timestamp.max(t + 3);
+            let rst_val = timekeeping_tim().rst_value().read().bits();
+            let rst_val_alarm = (safe_timestamp % rst_val as u64) as u32;
             alarm_tim
                 .rst_value()
-                .write(|w| unsafe { w.bits((safe_timestamp & u32::MAX as u64) as u32) });
+                .write(|w| unsafe { w.bits(rst_val_alarm) });
 
             let diff = timestamp - t;
-            if diff < TIMER_MARGIN {
+            if diff < (three_quarters_of_period(rst_val_alarm as u64)) {
+                alarm_tim
+                    .cnt_value()
+                    .write(|w| unsafe { w.bits(rst_val_alarm) });
                 alarm_tim.ctrl().modify(|_, w| w.irq_enb().set_bit());
                 alarm_tim.enable().write(|w| unsafe { w.bits(1) });
-            } else {
+            }
             // If it's too far in the future, don't enable timer yet.
             // It will be enabled later by `next_period`.
-                alarm_tim.ctrl().modify(|_, w| {
-                    w.irq_enb().clear_bit();
-                    w.enable().clear_bit()
-                });
-            }
 
             true
         })

examples/embassy/src/main.rs

@@ -25,17 +25,20 @@ async fn main(_spawner: Spawner) {
         .xtal_n_clk_with_src_freq(Hertz::from_raw(EXTCLK_FREQ))
         .freeze(&mut dp.sysconfig)
         .unwrap();
+    // Safety: Only called once here.
+    unsafe {
         embassy_example::init(
             &mut dp.sysconfig,
             &dp.irq_router,
             dp.tim15,
             dp.tim14,
             &clocks,
-    );
+        )
+    };
     let portg = PinsG::new(&mut dp.sysconfig, dp.portg);
     let mut led = portg.pg5.into_readable_push_pull_output();
     loop {
-        Timer::after_millis(2000).await;
+        Timer::after_secs(1).await;
         led.toggle().ok();
     }
 }