From 3873c94a800afe52542e322ffd465fdde21b7389 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Thu, 9 Jan 2025 18:39:03 +0100 Subject: [PATCH] Fix thread-mode executor on core1 not waking up --- esp-hal-embassy/CHANGELOG.md | 2 ++ esp-hal-embassy/src/executor/thread.rs | 24 ++++--------- esp-hal-embassy/src/lib.rs | 19 ++++++++++ hil-test/tests/embassy_interrupt_executor.rs | 38 +++++++++++++++++--- 4 files changed, 62 insertions(+), 21 deletions(-) diff --git a/esp-hal-embassy/CHANGELOG.md b/esp-hal-embassy/CHANGELOG.md index 03e83cbdcea..d221fb0e81b 100644 --- a/esp-hal-embassy/CHANGELOG.md +++ b/esp-hal-embassy/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fixed an issue where thread-mode executors running on core1 may not have waken up correctly (#2924) + ### Removed ## 0.5.0 - 2024-11-20 diff --git a/esp-hal-embassy/src/executor/thread.rs b/esp-hal-embassy/src/executor/thread.rs index 47d826c6f90..624de7e2592 100644 --- a/esp-hal-embassy/src/executor/thread.rs +++ b/esp-hal-embassy/src/executor/thread.rs @@ -3,9 +3,9 @@ use core::marker::PhantomData; use embassy_executor::{raw, Spawner}; -use esp_hal::Cpu; #[cfg(multi_core)] -use esp_hal::{interrupt::software::SoftwareInterrupt, macros::handler}; +use esp_hal::interrupt::software::SoftwareInterrupt; +use esp_hal::{interrupt::Priority, Cpu}; #[cfg(low_power_wait)] use portable_atomic::{AtomicBool, Ordering}; @@ -16,16 +16,6 @@ pub(crate) const THREAD_MODE_CONTEXT: usize = 16; static SIGNAL_WORK_THREAD_MODE: [AtomicBool; Cpu::COUNT] = [const { AtomicBool::new(false) }; Cpu::COUNT]; -#[cfg(all(multi_core, low_power_wait))] -#[handler] -fn software3_interrupt() { - // This interrupt is fired when the thread-mode executor's core needs to be - // woken. It doesn't matter which core handles this interrupt first, the - // point is just to wake up the core that is currently executing - // `waiti`. - unsafe { SoftwareInterrupt::<3>::steal().reset() }; -} - pub(crate) fn pend_thread_mode(_core: usize) { #[cfg(low_power_wait)] { @@ -68,11 +58,6 @@ impl Executor { This will use software-interrupt 3 which isn't available for anything else to wake the other core(s)."# )] pub fn new() -> Self { - #[cfg(all(multi_core, low_power_wait))] - unsafe { - SoftwareInterrupt::<3>::steal().set_interrupt_handler(software3_interrupt); - } - Self { inner: raw::Executor::new((THREAD_MODE_CONTEXT + Cpu::current() as usize) as *mut ()), not_send: PhantomData, @@ -100,6 +85,11 @@ This will use software-interrupt 3 which isn't available for anything else to wa /// /// This function never returns. pub fn run(&'static mut self, init: impl FnOnce(Spawner)) -> ! { + unwrap!(esp_hal::interrupt::enable( + esp_hal::peripherals::Interrupt::FROM_CPU_INTR3, + Priority::min(), + )); + init(self.inner.spawner()); #[cfg(low_power_wait)] diff --git a/esp-hal-embassy/src/lib.rs b/esp-hal-embassy/src/lib.rs index 72b733c32e5..5ff918f26ae 100644 --- a/esp-hal-embassy/src/lib.rs +++ b/esp-hal-embassy/src/lib.rs @@ -158,5 +158,24 @@ impl_array!(4); /// # } /// ``` pub fn init(time_driver: impl TimerCollection) { + #[cfg(all(feature = "executors", multi_core, low_power_wait))] + unsafe { + use esp_hal::interrupt::software::SoftwareInterrupt; + + #[esp_hal::macros::ram] + extern "C" fn software3_interrupt() { + // This interrupt is fired when the thread-mode executor's core needs to be + // woken. It doesn't matter which core handles this interrupt first, the + // point is just to wake up the core that is currently executing + // `waiti`. + unsafe { SoftwareInterrupt::<3>::steal().reset() }; + } + + esp_hal::interrupt::bind_interrupt( + esp_hal::peripherals::Interrupt::FROM_CPU_INTR3, + software3_interrupt, + ); + } + EmbassyTimer::init(time_driver.timers()) } diff --git a/hil-test/tests/embassy_interrupt_executor.rs b/hil-test/tests/embassy_interrupt_executor.rs index 51c5b9107e1..3f1ffafd169 100644 --- a/hil-test/tests/embassy_interrupt_executor.rs +++ b/hil-test/tests/embassy_interrupt_executor.rs @@ -18,6 +18,8 @@ use esp_hal::{ }, timer::AnyTimer, }; +#[cfg(multi_core)] +use esp_hal_embassy::Executor; use esp_hal_embassy::InterruptExecutor; use hil_test as _; @@ -31,10 +33,11 @@ macro_rules! mk_static { } #[embassy_executor::task] -async fn interrupt_driven_task( +async fn responder_task( signal: &'static Signal, response: &'static Signal, ) { + response.signal(()); loop { signal.wait().await; response.signal(()); @@ -100,9 +103,10 @@ mod test { let spawner = interrupt_executor.start(Priority::Priority3); spawner - .spawn(interrupt_driven_task(signal, response)) + .spawn(responder_task(signal, response)) .unwrap(); + response.wait().await; for _ in 0..3 { signal.signal(()); response.wait().await; @@ -124,19 +128,45 @@ mod test { let spawner = interrupt_executor.start(Priority::Priority3); spawner - .spawn(interrupt_driven_task(signal, response)) + .spawn(responder_task(signal, response)) .unwrap(); loop {} } }; - #[allow(static_mut_refs)] let _guard = ctx .cpu_control .start_app_core(app_core_stack, cpu1_fnctn) .unwrap(); + response.wait().await; + for _ in 0..3 { + signal.signal(()); + response.wait().await; + } + } + + #[test] + #[cfg(multi_core)] + async fn run_thread_executor_test_on_core_1(mut ctx: Context) { + let app_core_stack = mk_static!(Stack<8192>, Stack::new()); + let signal = mk_static!(Signal, Signal::new()); + let response = mk_static!(Signal, Signal::new()); + + let cpu1_fnctn = || { + let executor = mk_static!(Executor, Executor::new()); + executor.run(|spawner| { + spawner.spawn(responder_task(signal, response)).ok(); + }); + }; + + let _guard = ctx + .cpu_control + .start_app_core(app_core_stack, cpu1_fnctn) + .unwrap(); + + response.wait().await; for _ in 0..3 { signal.signal(()); response.wait().await;