WIP: remove signal_fn in executor.

embassy-nrf/src/time_driver.rs

@@ -1,8 +1,8 @@
 use core::cell::Cell;
 use core::sync::atomic::{compiler_fence, AtomicU32, AtomicU8, Ordering};
-use core::{mem, ptr};
 use critical_section::CriticalSection;
 use embassy::blocking_mutex::CriticalSectionMutex as Mutex;
+use embassy::executor::ExecutorWaker;
 use embassy::interrupt::{Interrupt, InterruptExt};
 use embassy::time::driver::{AlarmHandle, Driver};
 
@@ -61,11 +61,7 @@ mod test {
 
 struct AlarmState {
     timestamp: Cell<u64>,
-
-    // This is really a Option<(fn(*mut ()), *mut ())>
-    // but fn pointers aren't allowed in const yet
-    callback: Cell<*const ()>,
-    ctx: Cell<*mut ()>,
+    callback: Cell<ExecutorWaker>,
 }
 
 unsafe impl Send for AlarmState {}
@@ -74,8 +70,7 @@ impl AlarmState {
     const fn new() -> Self {
         Self {
             timestamp: Cell::new(u64::MAX),
-            callback: Cell::new(ptr::null()),
-            ctx: Cell::new(ptr::null_mut()),
+            callback: Cell::new(unsafe { ExecutorWaker::poison() }),
         }
     }
 }
@@ -173,12 +168,7 @@ impl RtcDriver {
         alarm.timestamp.set(u64::MAX);
 
         // Call after clearing alarm, so the callback can set another alarm.
-
-        // safety:
-        // - we can ignore the possiblity of `f` being unset (null) because of the safety contract of `allocate_alarm`.
-        // - other than that we only store valid function pointers into alarm.callback
-        let f: fn(*mut ()) = unsafe { mem::transmute(alarm.callback.get()) };
-        f(alarm.ctx.get());
+        alarm.callback.get().wake();
     }
 }
 
@@ -208,12 +198,10 @@ impl Driver for RtcDriver {
         }
     }
 
-    fn set_alarm_callback(&self, alarm: AlarmHandle, callback: fn(*mut ()), ctx: *mut ()) {
+    fn set_alarm_callback(&self, alarm: AlarmHandle, callback: ExecutorWaker) {
         critical_section::with(|cs| {
             let alarm = self.get_alarm(cs, alarm);
-
-            alarm.callback.set(callback as *const ());
-            alarm.ctx.set(ctx);
+            alarm.callback.set(callback);
         })
     }
 

embassy/src/executor/arch/arm.rs → embassy/src/executor/arch/arm_irq.rs

@@ -1,70 +1,26 @@
 use core::marker::PhantomData;
-use core::ptr;
+use core::mem;
 
 use super::{raw, Spawner};
-use crate::interrupt::{Interrupt, InterruptExt};
+use crate::interrupt::{Interrupt, InterruptExt, NrWrap};
 
-/// Thread mode executor, using WFE/SEV.
-///
-/// This is the simplest and most common kind of executor. It runs on
-/// thread mode (at the lowest priority level), and uses the `WFE` ARM instruction
-/// to sleep when it has no more work to do. When a task is woken, a `SEV` instruction
-/// is executed, to make the `WFE` exit from sleep and poll the task.
-///
-/// This executor allows for ultra low power consumption for chips where `WFE`
-/// triggers low-power sleep without extra steps. If your chip requires extra steps,
-/// you may use [`raw::Executor`] directly to program custom behavior.
-pub struct Executor {
-    inner: raw::Executor,
-    not_send: PhantomData<*mut ()>,
+/// TODO
+#[derive(Clone, Copy)]
+pub struct ExecutorWaker {
+    irqn: u16,
 }
 
-impl Executor {
-    /// Create a new Executor.
-    pub fn new() -> Self {
-        Self {
-            inner: raw::Executor::new(|_| cortex_m::asm::sev(), ptr::null_mut()),
-            not_send: PhantomData,
-        }
+impl ExecutorWaker {
+    /// TODO
+    pub const unsafe fn poison() -> Self {
+        Self { irqn: 0 }
     }
 
-    /// Run the executor.
-    ///
-    /// The `init` closure is called with a [`Spawner`] that spawns tasks on
-    /// this executor. Use it to spawn the initial task(s). After `init` returns,
-    /// the executor starts running the tasks.
-    ///
-    /// To spawn more tasks later, you may keep copies of the [`Spawner`] (it is `Copy`),
-    /// for example by passing it as an argument to the initial tasks.
-    ///
-    /// This function requires `&'static mut self`. This means you have to store the
-    /// Executor instance in a place where it'll live forever and grants you mutable
-    /// access. There's a few ways to do this:
-    ///
-    /// - a [Forever](crate::util::Forever) (safe)
-    /// - a `static mut` (unsafe)
-    /// - a local variable in a function you know never returns (like `fn main() -> !`), upgrading its lifetime with `transmute`. (unsafe)
-    ///
-    /// This function never returns.
-    pub fn run(&'static mut self, init: impl FnOnce(Spawner)) -> ! {
-        init(self.inner.spawner());
-
-        loop {
-            unsafe { self.inner.poll() };
-            cortex_m::asm::wfe();
-        }
-    }
-}
-
-fn pend_by_number(n: u16) {
-    #[derive(Clone, Copy)]
-    struct N(u16);
-    unsafe impl cortex_m::interrupt::InterruptNumber for N {
-        fn number(self) -> u16 {
-            self.0
-        }
+    /// TODO
+    pub fn wake(&self) {
+        let mut nvic: cortex_m::peripheral::NVIC = unsafe { mem::transmute(()) };
+        nvic.request(NrWrap(self.irqn));
     }
-    cortex_m::peripheral::NVIC::pend(N(n))
 }
 
 /// Interrupt mode executor.
@@ -88,19 +44,19 @@ fn pend_by_number(n: u16) {
 ///
 /// It is somewhat more complex to use, it's recommended to use the thread-mode
 /// [`Executor`] instead, if it works for your use case.
-pub struct InterruptExecutor<I: Interrupt> {
+pub struct Executor<I: Interrupt> {
     irq: I,
     inner: raw::Executor,
     not_send: PhantomData<*mut ()>,
 }
 
-impl<I: Interrupt> InterruptExecutor<I> {
+impl<I: Interrupt> Executor<I> {
     /// Create a new Executor.
     pub fn new(irq: I) -> Self {
-        let ctx = irq.number() as *mut ();
+        let irqn = irq.number();
         Self {
             irq,
-            inner: raw::Executor::new(|ctx| pend_by_number(ctx as u16), ctx),
+            inner: raw::Executor::new(ExecutorWaker { irqn }),
             not_send: PhantomData,
         }
     }

embassy/src/executor/arch/arm_wfe.rs

@@ -0,0 +1,73 @@
+use core::marker::PhantomData;
+use core::ptr;
+
+use super::{raw, Spawner};
+use crate::interrupt::{Interrupt, InterruptExt};
+
+/// TODO
+#[derive(Clone, Copy)]
+pub struct ExecutorWaker {}
+
+impl ExecutorWaker {
+    /// TODO
+    pub const unsafe fn poison() -> Self {
+        Self {}
+    }
+
+    /// TODO
+    pub fn wake(&self) {
+        cortex_m::asm::sev()
+    }
+}
+
+/// Thread mode executor, using WFE/SEV.
+///
+/// This is the simplest and most common kind of executor. It runs on
+/// thread mode (at the lowest priority level), and uses the `WFE` ARM instruction
+/// to sleep when it has no more work to do. When a task is woken, a `SEV` instruction
+/// is executed, to make the `WFE` exit from sleep and poll the task.
+///
+/// This executor allows for ultra low power consumption for chips where `WFE`
+/// triggers low-power sleep without extra steps. If your chip requires extra steps,
+/// you may use [`raw::Executor`] directly to program custom behavior.
+pub struct Executor {
+    inner: raw::Executor,
+    not_send: PhantomData<*mut ()>,
+}
+
+impl Executor {
+    /// Create a new Executor.
+    pub fn new() -> Self {
+        Self {
+            inner: raw::Executor::new(ExecutorWaker {}),
+            not_send: PhantomData,
+        }
+    }
+
+    /// Run the executor.
+    ///
+    /// The `init` closure is called with a [`Spawner`] that spawns tasks on
+    /// this executor. Use it to spawn the initial task(s). After `init` returns,
+    /// the executor starts running the tasks.
+    ///
+    /// To spawn more tasks later, you may keep copies of the [`Spawner`] (it is `Copy`),
+    /// for example by passing it as an argument to the initial tasks.
+    ///
+    /// This function requires `&'static mut self`. This means you have to store the
+    /// Executor instance in a place where it'll live forever and grants you mutable
+    /// access. There's a few ways to do this:
+    ///
+    /// - a [Forever](crate::util::Forever) (safe)
+    /// - a `static mut` (unsafe)
+    /// - a local variable in a function you know never returns (like `fn main() -> !`), upgrading its lifetime with `transmute`. (unsafe)
+    ///
+    /// This function never returns.
+    pub fn run(&'static mut self, init: impl FnOnce(Spawner)) -> ! {
+        init(self.inner.spawner());
+
+        loop {
+            unsafe { self.inner.poll() };
+            cortex_m::asm::wfe();
+        }
+    }
+}

embassy/src/executor/mod.rs

@@ -4,7 +4,7 @@
 
 #[cfg_attr(feature = "std", path = "arch/std.rs")]
 #[cfg_attr(feature = "wasm", path = "arch/wasm.rs")]
-#[cfg_attr(not(any(feature = "std", feature = "wasm")), path = "arch/arm.rs")]
+#[cfg_attr(not(any(feature = "std", feature = "wasm")), path = "arch/arm_irq.rs")]
 mod arch;
 pub mod raw;
 mod spawner;

embassy/src/executor/raw/mod.rs

@@ -24,7 +24,7 @@ use critical_section::CriticalSection;
 
 use self::run_queue::{RunQueue, RunQueueItem};
 use self::util::UninitCell;
-use super::SpawnToken;
+use super::{ExecutorWaker, SpawnToken};
 #[cfg(feature = "time")]
 use crate::time::driver::{self, AlarmHandle};
 #[cfg(feature = "time")]
@@ -217,8 +217,7 @@ unsafe impl<F: Future + 'static> Sync for TaskStorage<F> {}
 /// the requirement for `poll` to not be called reentrantly.
 pub struct Executor {
     run_queue: RunQueue,
-    signal_fn: fn(*mut ()),
-    signal_ctx: *mut (),
+    waker: ExecutorWaker,
 
     #[cfg(feature = "time")]
     pub(crate) timer_queue: timer_queue::TimerQueue,
@@ -233,16 +232,15 @@ impl Executor {
     /// `signal_ctx` as argument.
     ///
     /// See [`Executor`] docs for details on `signal_fn`.
-    pub fn new(signal_fn: fn(*mut ()), signal_ctx: *mut ()) -> Self {
+    pub fn new(waker: ExecutorWaker) -> Self {
         #[cfg(feature = "time")]
         let alarm = unsafe { unwrap!(driver::allocate_alarm()) };
         #[cfg(feature = "time")]
-        driver::set_alarm_callback(alarm, signal_fn, signal_ctx);
+        driver::set_alarm_callback(alarm, waker);
 
         Self {
             run_queue: RunQueue::new(),
-            signal_fn,
-            signal_ctx,
+            waker,
 
             #[cfg(feature = "time")]
             timer_queue: timer_queue::TimerQueue::new(),
@@ -259,9 +257,8 @@ impl Executor {
     /// - `task` must NOT be already enqueued (in this executor or another one).
     #[inline(always)]
     unsafe fn enqueue(&self, cs: CriticalSection, task: *mut TaskHeader) {
-        if self.run_queue.enqueue(cs, task) {
-            (self.signal_fn)(self.signal_ctx)
-        }
+        self.run_queue.enqueue(cs, task);
+        self.waker.wake();
     }
 
     /// Spawn a task in this executor.

embassy/src/time/driver.rs

@@ -46,7 +46,7 @@
 //!     unsafe fn allocate_alarm(&self) -> Option<AlarmHandle> {
 //!         todo!()
 //!     }
-//!     fn set_alarm_callback(&self, alarm: AlarmHandle, callback: fn(*mut ()), ctx: *mut ()) {
+//!     fn set_alarm_callback(&self, alarm: AlarmHandle, signaler: Signaler) {
 //!         todo!()
 //!     }
 //!     fn set_alarm(&self, alarm: AlarmHandle, timestamp: u64) {
@@ -55,6 +55,8 @@
 //! }
 //! ```
 
+use crate::executor::ExecutorWaker;
+
 /// Alarm handle, assigned by the driver.
 #[derive(Clone, Copy)]
 pub struct AlarmHandle {
@@ -91,9 +93,9 @@ pub trait Driver: Send + Sync + 'static {
     /// It is UB to make the alarm fire before setting a callback.
     unsafe fn allocate_alarm(&self) -> Option<AlarmHandle>;
 
-    /// Sets the callback function to be called when the alarm triggers.
-    /// The callback may be called from any context (interrupt or thread mode).
-    fn set_alarm_callback(&self, alarm: AlarmHandle, callback: fn(*mut ()), ctx: *mut ());
+    /// Sets the callback ExecutorWaker to be woken when the alarm triggers.
+    /// The waker may be called from any context (interrupt or thread mode).
+    fn set_alarm_callback(&self, alarm: AlarmHandle, callback: ExecutorWaker);
 
     /// Sets an alarm at the given timestamp. When the current timestamp reaches the alarm
     /// timestamp, the provided callback funcion will be called.
@@ -110,7 +112,7 @@ pub trait Driver: Send + Sync + 'static {
 extern "Rust" {
     fn _embassy_time_now() -> u64;
     fn _embassy_time_allocate_alarm() -> Option<AlarmHandle>;
-    fn _embassy_time_set_alarm_callback(alarm: AlarmHandle, callback: fn(*mut ()), ctx: *mut ());
+    fn _embassy_time_set_alarm_callback(alarm: AlarmHandle, callback: ExecutorWaker);
     fn _embassy_time_set_alarm(alarm: AlarmHandle, timestamp: u64);
 }
 
@@ -121,8 +123,8 @@ pub(crate) fn now() -> u64 {
 pub(crate) unsafe fn allocate_alarm() -> Option<AlarmHandle> {
     _embassy_time_allocate_alarm()
 }
-pub(crate) fn set_alarm_callback(alarm: AlarmHandle, callback: fn(*mut ()), ctx: *mut ()) {
-    unsafe { _embassy_time_set_alarm_callback(alarm, callback, ctx) }
+pub(crate) fn set_alarm_callback(alarm: AlarmHandle, callback: ExecutorWaker) {
+    unsafe { _embassy_time_set_alarm_callback(alarm, callback) }
 }
 pub(crate) fn set_alarm(alarm: AlarmHandle, timestamp: u64) {
     unsafe { _embassy_time_set_alarm(alarm, timestamp) }
@@ -149,10 +151,9 @@ macro_rules! time_driver_impl {
         #[no_mangle]
         fn _embassy_time_set_alarm_callback(
             alarm: $crate::time::driver::AlarmHandle,
-            callback: fn(*mut ()),
-            ctx: *mut (),
+            callback: $crate::executor::ExecutorWaker,
         ) {
-            <$t as $crate::time::driver::Driver>::set_alarm_callback(&$name, alarm, callback, ctx)
+            <$t as $crate::time::driver::Driver>::set_alarm_callback(&$name, alarm, callback)
         }
 
         #[no_mangle]