Merge pull request #1937 from xoviat/low-power

stm32/low-power: create one critical-section for all time ops

3 files changed, 101 insertions, 82 deletions

diff --git a/embassy-stm32/src/rtc/v2.rs b/embassy-stm32/src/rtc/v2.rs
index aa3c31ee1..d139f2f47 100644
--- a/embassy-stm32/src/rtc/v2.rs
+++ b/embassy-stm32/src/rtc/v2.rs
@@ -64,7 +64,11 @@ impl super::Rtc {
     #[cfg(feature = "low-power")]
     /// start the wakeup alarm and wtih a duration that is as close to but less than
     /// the requested duration, and record the instant the wakeup alarm was started
-    pub(crate) fn start_wakeup_alarm(&self, requested_duration: embassy_time::Duration) {
+    pub(crate) fn start_wakeup_alarm(
+        &self,
+        requested_duration: embassy_time::Duration,
+        cs: critical_section::CriticalSection,
+    ) {
         use embassy_time::{Duration, TICK_HZ};
 
         #[cfg(any(rcc_wb, rcc_f4, rcc_f410))]
@@ -102,25 +106,13 @@ impl super::Rtc {
             self.instant(),
         );
 
-        critical_section::with(|cs| assert!(self.stop_time.borrow(cs).replace(Some(self.instant())).is_none()))
-    }
-
-    #[cfg(feature = "low-power")]
-    pub(crate) fn enable_wakeup_line(&self) {
-        use crate::interrupt::typelevel::Interrupt;
-        use crate::pac::EXTI;
-
-        <RTC as crate::rtc::sealed::Instance>::WakeupInterrupt::unpend();
-        unsafe { <RTC as crate::rtc::sealed::Instance>::WakeupInterrupt::enable() };
-
-        EXTI.rtsr(0).modify(|w| w.set_line(RTC::EXTI_WAKEUP_LINE, true));
-        EXTI.imr(0).modify(|w| w.set_line(RTC::EXTI_WAKEUP_LINE, true));
+        assert!(self.stop_time.borrow(cs).replace(Some(self.instant())).is_none())
     }
 
     #[cfg(feature = "low-power")]
     /// stop the wakeup alarm and return the time elapsed since `start_wakeup_alarm`
     /// was called, otherwise none
-    pub(crate) fn stop_wakeup_alarm(&self) -> Option<embassy_time::Duration> {
+    pub(crate) fn stop_wakeup_alarm(&self, cs: critical_section::CriticalSection) -> Option<embassy_time::Duration> {
         use crate::interrupt::typelevel::Interrupt;
 
         trace!("rtc: stop wakeup alarm at {}", self.instant());
@@ -137,13 +129,23 @@ impl super::Rtc {
             <RTC as crate::rtc::sealed::Instance>::WakeupInterrupt::unpend();
         });
 
-        critical_section::with(|cs| {
-            if let Some(stop_time) = self.stop_time.borrow(cs).take() {
-                Some(self.instant() - stop_time)
-            } else {
-                None
-            }
-        })
+        if let Some(stop_time) = self.stop_time.borrow(cs).take() {
+            Some(self.instant() - stop_time)
+        } else {
+            None
+        }
+    }
+
+    #[cfg(feature = "low-power")]
+    pub(crate) fn enable_wakeup_line(&self) {
+        use crate::interrupt::typelevel::Interrupt;
+        use crate::pac::EXTI;
+
+        <RTC as crate::rtc::sealed::Instance>::WakeupInterrupt::unpend();
+        unsafe { <RTC as crate::rtc::sealed::Instance>::WakeupInterrupt::enable() };
+
+        EXTI.rtsr(0).modify(|w| w.set_line(RTC::EXTI_WAKEUP_LINE, true));
+        EXTI.imr(0).modify(|w| w.set_line(RTC::EXTI_WAKEUP_LINE, true));
     }
 
     /// Applies the RTC config
diff --git a/embassy-stm32/src/time_driver.rs b/embassy-stm32/src/time_driver.rs
index 887e54f65..5b01937f5 100644
--- a/embassy-stm32/src/time_driver.rs
+++ b/embassy-stm32/src/time_driver.rs
@@ -266,32 +266,28 @@ impl RtcDriver {
         f(alarm.ctx.get());
     }
 
-    #[cfg(feature = "low-power")]
-    /// Set the rtc but panic if it's already been set
-    pub(crate) fn set_rtc(&self, rtc: &'static Rtc) {
-        critical_section::with(|cs| assert!(self.rtc.borrow(cs).replace(Some(rtc)).is_none()));
-    }
+    /*
+        Low-power private functions: all operate within a critical seciton
+    */
 
     #[cfg(feature = "low-power")]
     /// Compute the approximate amount of time until the next alarm
-    fn time_until_next_alarm(&self) -> embassy_time::Duration {
-        critical_section::with(|cs| {
-            let now = self.now() + 32;
-
-            embassy_time::Duration::from_ticks(
-                self.alarms
-                    .borrow(cs)
-                    .iter()
-                    .map(|alarm: &AlarmState| alarm.timestamp.get().saturating_sub(now))
-                    .min()
-                    .unwrap_or(u64::MAX),
-            )
-        })
+    fn time_until_next_alarm(&self, cs: CriticalSection) -> embassy_time::Duration {
+        let now = self.now() + 32;
+
+        embassy_time::Duration::from_ticks(
+            self.alarms
+                .borrow(cs)
+                .iter()
+                .map(|alarm: &AlarmState| alarm.timestamp.get().saturating_sub(now))
+                .min()
+                .unwrap_or(u64::MAX),
+        )
     }
 
     #[cfg(feature = "low-power")]
     /// Add the given offset to the current time
-    fn add_time(&self, offset: embassy_time::Duration) {
+    fn add_time(&self, offset: embassy_time::Duration, cs: CriticalSection) {
         let offset = offset.as_ticks();
         let cnt = T::regs_gp16().cnt().read().cnt() as u32;
         let period = self.period.load(Ordering::SeqCst);
@@ -322,51 +318,57 @@ impl RtcDriver {
         T::regs_gp16().cnt().write(|w| w.set_cnt(cnt as u16));
 
         // Now, recompute all alarms
-        critical_section::with(|cs| {
-            for i in 0..ALARM_COUNT {
-                let alarm_handle = unsafe { AlarmHandle::new(i as u8) };
-                let alarm = self.get_alarm(cs, alarm_handle);
+        for i in 0..ALARM_COUNT {
+            let alarm_handle = unsafe { AlarmHandle::new(i as u8) };
+            let alarm = self.get_alarm(cs, alarm_handle);
 
-                self.set_alarm(alarm_handle, alarm.timestamp.get());
-            }
-        })
+            self.set_alarm(alarm_handle, alarm.timestamp.get());
+        }
     }
 
     #[cfg(feature = "low-power")]
     /// Stop the wakeup alarm, if enabled, and add the appropriate offset
-    fn stop_wakeup_alarm(&self) {
-        critical_section::with(|cs| {
-            if let Some(offset) = self.rtc.borrow(cs).get().unwrap().stop_wakeup_alarm() {
-                self.add_time(offset);
-            }
-        });
+    fn stop_wakeup_alarm(&self, cs: CriticalSection) {
+        if let Some(offset) = self.rtc.borrow(cs).get().unwrap().stop_wakeup_alarm(cs) {
+            self.add_time(offset, cs);
+        }
+    }
+
+    /*
+        Low-power public functions: all create a critical section
+    */
+    #[cfg(feature = "low-power")]
+    /// Set the rtc but panic if it's already been set
+    pub(crate) fn set_rtc(&self, rtc: &'static Rtc) {
+        critical_section::with(|cs| assert!(self.rtc.borrow(cs).replace(Some(rtc)).is_none()));
     }
 
     #[cfg(feature = "low-power")]
     /// Pause the timer if ready; return err if not
     pub(crate) fn pause_time(&self) -> Result<(), ()> {
-        /*
-            If the wakeup timer is currently running, then we need to stop it and
-            add the elapsed time to the current time
-        */
-        self.stop_wakeup_alarm();
-
-        let time_until_next_alarm = self.time_until_next_alarm();
-        if time_until_next_alarm < embassy_time::Duration::from_millis(250) {
-            Err(())
-        } else {
-            critical_section::with(|cs| {
+        critical_section::with(|cs| {
+            /*
+                If the wakeup timer is currently running, then we need to stop it and
+                add the elapsed time to the current time, as this will impact the result
+                of `time_until_next_alarm`.
+            */
+            self.stop_wakeup_alarm(cs);
+
+            let time_until_next_alarm = self.time_until_next_alarm(cs);
+            if time_until_next_alarm < embassy_time::Duration::from_millis(250) {
+                Err(())
+            } else {
                 self.rtc
                     .borrow(cs)
                     .get()
                     .unwrap()
-                    .start_wakeup_alarm(time_until_next_alarm);
-            });
+                    .start_wakeup_alarm(time_until_next_alarm, cs);
 
-            T::regs_gp16().cr1().modify(|w| w.set_cen(false));
+                T::regs_gp16().cr1().modify(|w| w.set_cen(false));
 
-            Ok(())
-        }
+                Ok(())
+            }
+        })
     }
 
     #[cfg(feature = "low-power")]
@@ -378,9 +380,11 @@ impl RtcDriver {
             return;
         }
 
-        self.stop_wakeup_alarm();
+        critical_section::with(|cs| {
+            self.stop_wakeup_alarm(cs);
 
-        T::regs_gp16().cr1().modify(|w| w.set_cen(true));
+            T::regs_gp16().cr1().modify(|w| w.set_cen(true));
+        })
     }
 }
 
diff --git a/tests/stm32/src/bin/stop.rs b/tests/stm32/src/bin/stop.rs
index f60ab271a..48d59b794 100644
--- a/tests/stm32/src/bin/stop.rs
+++ b/tests/stm32/src/bin/stop.rs
@@ -19,14 +19,32 @@ use static_cell::make_static;
 
 #[entry]
 fn main() -> ! {
-    let executor = Executor::take();
-    executor.run(|spawner| {
+    Executor::take().run(|spawner| {
         unwrap!(spawner.spawn(async_main(spawner)));
     });
 }
 
 #[embassy_executor::task]
-async fn async_main(_spawner: Spawner) {
+async fn task_1() {
+    for _ in 0..9 {
+        info!("task 1: waiting for 500ms...");
+        Timer::after(Duration::from_millis(500)).await;
+    }
+}
+
+#[embassy_executor::task]
+async fn task_2() {
+    for _ in 0..5 {
+        info!("task 2: waiting for 1000ms...");
+        Timer::after(Duration::from_millis(1000)).await;
+    }
+
+    info!("Test OK");
+    cortex_m::asm::bkpt();
+}
+
+#[embassy_executor::task]
+async fn async_main(spawner: Spawner) {
     let mut config = config();
 
     config.rcc.lse = Some(Hertz(32_768));
@@ -48,11 +66,6 @@ async fn async_main(_spawner: Spawner) {
 
     stop_with_rtc(rtc);
 
-    info!("Waiting...");
-    Timer::after(Duration::from_secs(2)).await;
-    info!("Waiting...");
-    Timer::after(Duration::from_secs(3)).await;
-
-    info!("Test OK");
-    cortex_m::asm::bkpt();
+    spawner.spawn(task_1()).unwrap();
+    spawner.spawn(task_2()).unwrap();
 }