somethings wrong

examples/embassy/Cargo.toml

@@ -1,5 +1,5 @@
 [package]
-name = "embassy"
+name = "embassy-example"
 version = "0.1.0"
 edition = "2021"
 
@@ -12,12 +12,17 @@ panic-rtt-target = { version = "0.1" }
 critical-section = "1"
 
 embassy-sync = { version = "0.6.0" }
-embassy-time = { version = "0.3.2", features = ["tick-hz-32_768"] }
+embassy-time = { version = "0.3.2", features = ["tick-hz-1_000"] }
 embassy-time-driver = { version = "0.1" }
 
 [dependencies.embassy-executor]
 version = "0.6.0"
-features = ["arch-cortex-m", "executor-thread", "executor-interrupt", "integrated-timers"]
+features = [
+    "arch-cortex-m",
+    "executor-thread",
+    "executor-interrupt",
+    "integrated-timers",
+]
 
 [dependencies.va416xx-hal]
 path = "../../va416xx-hal"

examples/embassy/src/lib.rs

@@ -2,21 +2,50 @@
 use core::{
     cell::Cell,
     mem, ptr,
-    sync::atomic::{AtomicU32, AtomicU8, Ordering}, u32,
+    sync::atomic::{AtomicU32, AtomicU8, Ordering},
 };
 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};
+use embassy_time_driver::{time_driver_impl, AlarmHandle, Driver, TICK_HZ};
-use va416xx_hal::pac;
+use va416xx_hal::{
+    clock::Clocks,
+    enable_interrupt,
+    pac::{self, interrupt},
+    pwm::{assert_tim_reset, deassert_tim_reset, enable_tim_clk, ValidTim},
+};
 
-fn alarm_tim() -> &'static pac::tim0::RegisterBlock {
+pub type TimekeeperClk = pac::Tim15;
-    unsafe { &*pac::Tim23::ptr() }
+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(
+    syscfg: &mut pac::Sysconfig,
+    timekeeper: TimekeeperClk,
+    alarm: AlarmClk0,
+    clocks: &Clocks,
+) {
+    DRIVER.init(syscfg, timekeeper, alarm, clocks)
 }
 
-fn timekeeping_tim() -> &'static pac::tim0::RegisterBlock {
+const fn alarm_tim(idx: usize) -> &'static pac::tim0::RegisterBlock {
-    unsafe { &*pac::Tim23::ptr() }
+    // Safety: This is a memory-mapped peripheral.
+    match idx {
+        0 => unsafe { &*AlarmClk0::ptr() },
+        1 => unsafe { &*AlarmClk1::ptr() },
+        2 => unsafe { &*AlarmClk2::ptr() },
+        _ => {
+            panic!("invalid alarm timer index")
+        }
+    }
+}
+
+const fn timekeeping_tim() -> &'static pac::tim0::RegisterBlock {
+    // Safety: This is a memory-mapped peripheral.
+    unsafe { &*TimekeeperClk::ptr() }
 }
 
 struct AlarmState {
@@ -41,36 +70,92 @@ 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 EmbassyVa416xxTimeDriver {
+pub struct TimerDriverEmbassy {
     periods: AtomicU32,
     alarm_count: AtomicU8,
     /// Timestamp at which to fire alarm. u64::MAX if no alarm is scheduled.
     alarms: Mutex<CriticalSectionRawMutex, [AlarmState; ALARM_COUNT]>,
 }
 
-impl EmbassyVa416xxTimeDriver {
+impl TimerDriverEmbassy {
+    fn init(
+        &self,
+        syscfg: &mut pac::Sysconfig,
+        timekeeper: TimekeeperClk,
+        alarm_tim: AlarmClk0,
+        clocks: &Clocks,
+    ) {
+        enable_tim_clk(syscfg, TimekeeperClk::TIM_ID);
+        assert_tim_reset(syscfg, TimekeeperClk::TIM_ID);
+        cortex_m::asm::nop();
+        cortex_m::asm::nop();
+        deassert_tim_reset(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) });
+        // Switch on. Timekeeping should always be done.
+        timekeeper.ctrl().modify(|_, w| {
+            w.irq_enb().set_bit();
+            w.enable().set_bit()
+        });
+        unsafe {
+            enable_interrupt(TimekeeperClk::IRQ);
+        }
+
+        enable_tim_clk(syscfg, AlarmClk0::TIM_ID);
+        assert_tim_reset(syscfg, AlarmClk0::TIM_ID);
+        cortex_m::asm::nop();
+        cortex_m::asm::nop();
+        deassert_tim_reset(syscfg, AlarmClk0::TIM_ID);
+
+        // Explicitely disable alarm timer until needed.
+        alarm_tim.ctrl().modify(|_, w| {
+            w.irq_enb().clear_bit();
+            w.enable().clear_bit()
+        });
+        // Enable general interrupts. The IRQ enable of the peripheral remains cleared.
+        unsafe {
+            enable_interrupt(AlarmClk0::IRQ);
+        }
+    }
+
     fn on_interrupt_timekeeping(&self) {
         self.next_period();
     }
 
     fn on_interrupt_alarm(&self, idx: usize) {
-        critical_section::with(|cs| {
+        critical_section::with(|cs| self.trigger_alarm(idx, cs))
-            let alarm = &self.alarms.borrow(cs)[idx];
-            let at = alarm.timestamp.get();
-            let period = self.periods.load(Ordering::Relaxed);
-            let t = (period as u64) << 32;
-
-            if at < t + u32::MAX as u64 {
-                //alarm_tim().
-                // just enable it. `set_alarm` has already set the correct CC val.
-                alarm_tim().ctrl().modify(|_, w| w.irq_enb().set_bit());  
-            }
-        })
     }
 
     fn next_period(&self) {
-        self.periods.fetch_add(1, Ordering::Release);
+        let period = self.periods.fetch_add(1, Ordering::AcqRel) + 1;
+        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()
+                    //alarm_tim(i)
+                    //.cnt_value()
+                    //.write(|w| unsafe { w.bits(rst_val) });
+                    alarm_tim(i).ctrl().modify(|_, w| {
+                        w.irq_enb().set_bit();
+                        w.enable().set_bit()
+                    });
+                }
+            }
+        })
     }
 
     fn get_alarm<'a>(&'a self, cs: CriticalSection<'a>, alarm: AlarmHandle) -> &'a AlarmState {
@@ -80,8 +165,10 @@ impl EmbassyVa416xxTimeDriver {
     }
 
     fn trigger_alarm(&self, n: usize, cs: CriticalSection) {
-        let r = alarm_tim();
+        alarm_tim(n).ctrl().modify(|_, w| {
-        r.ctrl().modify(|_, w| w.irq_enb().clear_bit());
+            w.irq_enb().clear_bit();
+            w.enable().clear_bit()
+        });
 
         let alarm = &self.alarms.borrow(cs)[n];
         // Setting the maximum value disables the alarm.
@@ -97,7 +184,7 @@ impl EmbassyVa416xxTimeDriver {
     }
 }
 
-impl Driver for EmbassyVa416xxTimeDriver {
+impl Driver for TimerDriverEmbassy {
     fn now(&self) -> u64 {
         let mut period1: u32;
         let mut period2: u32;
@@ -107,9 +194,10 @@ impl Driver for EmbassyVa416xxTimeDriver {
             // no instructions can be reordered before the load.
             period1 = self.periods.load(Ordering::Acquire);
 
-            counter_val = timekeeping_tim().cnt_value().read().bits();
+            counter_val = timekeeping_tim().rst_value().read().bits()
+                - timekeeping_tim().cnt_value().read().bits();
 
-            period2 = self.periods.load(Ordering::Acquire);
+            period2 = self.periods.load(Ordering::Relaxed);
             if period1 == period2 {
                 break;
             }
@@ -145,48 +233,52 @@ impl Driver for EmbassyVa416xxTimeDriver {
 
     fn set_alarm(&self, alarm: embassy_time_driver::AlarmHandle, timestamp: u64) -> bool {
         critical_section::with(|cs| {
-            let n = alarm.id() as _;
+            let n = alarm.id();
             let alarm = self.get_alarm(cs, alarm);
             alarm.timestamp.set(timestamp);
 
-            let r = alarm_tim();
+            let alarm_tim = alarm_tim(n.into());
 
             let t = self.now();
             if timestamp <= t {
-                r.ctrl().modify(|_, w| w.irq_enb().clear_bit());
+                alarm_tim.ctrl().modify(|_, w| {
+                    w.irq_enb().clear_bit();
+                    w.enable().clear_bit()
+                });
 
                 alarm.timestamp.set(u64::MAX);
 
                 return false;
             }
 
-            // If it hasn't triggered yet, setup it in the compare channel.
+            // If it hasn't triggered yet, setup the relevant reset value, regardless of whether
+            // the interrupts are enabled or not. When they are enabled at a later point, the
+            // right value is already set.
 
-            // Write the CC value regardless of whether we're going to enable it now or not.
+            // If the timestamp is in the next few ticks, add a bit of buffer to be sure the alarm
-            // This way, when we enable it later, the right value is already set.
+            // is not missed.
-
-            // nrf52 docs say:
-            //    If the COUNTER is N, writing N or N+1 to a CC register may not trigger a COMPARE event.
-            // To workaround this, we never write a timestamp smaller than N+3.
-            // N+2 is not safe because rtc can tick from N to N+1 between calling now() and writing cc.
-            //
-            // It is impossible for rtc to tick more than once because
-            //  - this code takes less time than 1 tick
-            //  - it runs with interrupts disabled so nothing else can preempt it.
             //
             // This means that an alarm can be delayed for up to 2 ticks (from t+1 to t+3), but this is allowed
             // 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);
-            r.cc[n].write(|w| unsafe { w.bits(safe_timestamp as u32 & 0xFFFFFF) });
+            alarm_tim
+                .rst_value()
+                .write(|w| unsafe { w.bits((safe_timestamp & u32::MAX as u64) as u32) });
 
             let diff = timestamp - t;
-            if diff < 0xc00000 {
+            if diff < TIMER_MARGIN {
-                r.intenset.write(|w| unsafe { w.bits(compare_n(n)) });
+                alarm_tim.ctrl().modify(|_, w| {
+                    w.irq_enb().set_bit();
+                    w.enable().set_bit()
+                });
             } else {
-                // If it's too far in the future, don't setup the compare channel yet.
+                // If it's too far in the future, don't enable timer yet.
-                // It will be setup later by `next_period`.
+                // It will be enabled later by `next_period`.
-                r.intenclr.write(|w| unsafe { w.bits(compare_n(n)) });
+                alarm_tim.ctrl().modify(|_, w| {
+                    w.irq_enb().clear_bit();
+                    w.enable().clear_bit()
+                });
             }
 
             true
@@ -195,8 +287,20 @@ impl Driver for EmbassyVa416xxTimeDriver {
 }
 
 time_driver_impl!(
-    static DRIVER: EmbassyVa416xxTimeDriver = EmbassyVa416xxTimeDriver {
+    static DRIVER: TimerDriverEmbassy = TimerDriverEmbassy {
         periods: AtomicU32::new(0),
         alarm_count: AtomicU8::new(0),
         alarms: Mutex::const_new(CriticalSectionRawMutex::new(), [AlarmState::new(); ALARM_COUNT])
 });
+
+#[interrupt]
+#[allow(non_snake_case)]
+fn TIM15() {
+    DRIVER.on_interrupt_timekeeping()
+}
+
+#[interrupt]
+#[allow(non_snake_case)]
+fn TIM14() {
+    DRIVER.on_interrupt_alarm(0)
+}

examples/embassy/src/main.rs

@@ -1,23 +1,36 @@
 #![no_std]
 #![no_main]
+use cortex_m::asm;
 use embassy_executor::Spawner;
 use embassy_time::Timer;
 use embedded_hal::digital::StatefulOutputPin;
 use panic_rtt_target as _;
 use rtt_target::{rprintln, rtt_init_print};
-use va416xx_hal::{gpio::PinsG, pac};
+use va416xx_hal::{gpio::PinsG, pac, prelude::*, time::Hertz};
+
+const EXTCLK_FREQ: u32 = 40_000_000;
 
 // main is itself an async function.
 #[embassy_executor::main]
 async fn main(_spawner: Spawner) {
     rtt_init_print!();
-    rprintln!("VA416xx RTT Demo");
+    rprintln!("VA416xx Embassy Demo");
 
     let mut dp = pac::Peripherals::take().unwrap();
+
+    // Initialize the systick interrupt & obtain the token to prove that we did
+    // Use the external clock connected to XTAL_N.
+    let clocks = dp
+        .clkgen
+        .constrain()
+        .xtal_n_clk_with_src_freq(Hertz::from_raw(EXTCLK_FREQ))
+        .freeze(&mut dp.sysconfig)
+        .unwrap();
+    embassy_example::init(&mut dp.sysconfig, 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(200).await;
+        Timer::after_millis(2000).await;
         led.toggle().ok();
     }
 }

va416xx-hal/src/clock.rs

@@ -494,7 +494,7 @@ pub struct Clocks {
 
 impl Clocks {
     /// Returns the frequency of the HBO clock
-    pub fn hbo(&self) -> Hertz {
+    pub const fn hbo(&self) -> Hertz {
         HBO_FREQ
     }
 
@@ -504,23 +504,23 @@ impl Clocks {
     }
 
     /// Returns system clock divied by 2.
-    pub fn apb1(&self) -> Hertz {
+    pub const fn apb1(&self) -> Hertz {
         self.apb1
     }
 
     /// Returns system clock divied by 4.
-    pub fn apb2(&self) -> Hertz {
+    pub const fn apb2(&self) -> Hertz {
         self.apb2
     }
 
     /// Returns the system (core) frequency
-    pub fn sysclk(&self) -> Hertz {
+    pub const fn sysclk(&self) -> Hertz {
         self.sysclk
     }
 
     /// Returns the ADC clock frequency which has a separate divider.
     #[cfg(not(feature = "va41628"))]
-    pub fn adc_clk(&self) -> Hertz {
+    pub const fn adc_clk(&self) -> Hertz {
         self.adc_clk
     }
 }

va416xx-hal/src/timer.rs

@@ -169,6 +169,14 @@ macro_rules! tim_markers {
     };
 }
 
+pub const fn const_clock<Tim: ValidTim + ?Sized>(_: &Tim, clocks: &Clocks) -> Hertz {
+    if Tim::TIM_ID <= 15 {
+        clocks.apb1()
+    } else {
+        clocks.apb2()
+    }
+}
+
 tim_markers!(
     (pac::Tim0, 0, pac::Interrupt::TIM0),
     (pac::Tim1, 1, pac::Interrupt::TIM1),
@@ -328,14 +336,14 @@ valid_pin_and_tims!(
 ///
 /// Only the bit related to the corresponding TIM peripheral is modified
 #[inline]
-fn assert_tim_reset(syscfg: &mut pac::Sysconfig, tim_id: u8) {
+pub fn assert_tim_reset(syscfg: &mut pac::Sysconfig, tim_id: u8) {
     syscfg
         .tim_reset()
         .modify(|r, w| unsafe { w.bits(r.bits() & !(1 << tim_id as u32)) })
 }
 
 #[inline]
-fn deassert_tim_reset(syscfg: &mut pac::Sysconfig, tim_id: u8) {
+pub fn deassert_tim_reset(syscfg: &mut pac::Sysconfig, tim_id: u8) {
     syscfg
         .tim_reset()
         .modify(|r, w| unsafe { w.bits(r.bits() | (1 << tim_id as u32)) })
@@ -481,7 +489,7 @@ pub struct CountdownTimer<TIM: ValidTim> {
 }
 
 #[inline]
-fn enable_tim_clk(syscfg: &mut pac::Sysconfig, idx: u8) {
+pub fn enable_tim_clk(syscfg: &mut pac::Sysconfig, idx: u8) {
     syscfg
         .tim_clk_enable()
         .modify(|r, w| unsafe { w.bits(r.bits() | (1 << idx)) });
@@ -581,6 +589,7 @@ impl<Tim: ValidTim> CountdownTimer<Tim> {
         self.curr_freq = timeout.into();
         self.rst_val = self.clock.raw() / self.curr_freq.raw();
         self.set_reload(self.rst_val);
+        // Decrementing counter, to set the reset value.
         self.set_count(0);
     }