From 7fa0e571724a15fd73de65d869cb1b755e12802a Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Tue, 11 May 2021 00:57:52 +0200 Subject: [PATCH] Use `critical_section` crate --- embassy-extras/src/macros.rs | 2 +- embassy-nrf/Cargo.toml | 1 + embassy-nrf/src/interrupt.rs | 30 ---------------------------- embassy-nrf/src/rtc.rs | 17 ++++++++-------- embassy-rp/Cargo.toml | 1 + embassy-rp/src/interrupt.rs | 30 ---------------------------- embassy-stm32/Cargo.toml | 1 + embassy-stm32/src/exti.rs | 6 +++--- embassy-stm32/src/f4/rtc.rs | 15 +++++++------- embassy-stm32/src/interrupt.rs | 30 ---------------------------- embassy-stm32/src/l0/rtc.rs | 15 +++++++------- embassy/Cargo.toml | 1 + embassy/src/util/critical_section.rs | 30 ---------------------------- embassy/src/util/mod.rs | 2 -- embassy/src/util/mutex.rs | 4 ++-- embassy/src/util/signal.rs | 9 ++++----- embassy/src/util/waker_agnostic.rs | 4 ++-- 17 files changed, 41 insertions(+), 157 deletions(-) delete mode 100644 embassy/src/util/critical_section.rs diff --git a/embassy-extras/src/macros.rs b/embassy-extras/src/macros.rs index d8543935..860c0795 100644 --- a/embassy-extras/src/macros.rs +++ b/embassy-extras/src/macros.rs @@ -51,7 +51,7 @@ macro_rules! peripherals { #[no_mangle] static mut _EMBASSY_DEVICE_PERIPHERALS: bool = false; - cortex_m::interrupt::free(|_| { + critical_section::with(|_| { if unsafe { _EMBASSY_DEVICE_PERIPHERALS } { None } else { diff --git a/embassy-nrf/Cargo.toml b/embassy-nrf/Cargo.toml index 8ec5b087..b8b63bbb 100644 --- a/embassy-nrf/Cargo.toml +++ b/embassy-nrf/Cargo.toml @@ -36,3 +36,4 @@ nrf52811-pac = { version = "0.9.1", optional = true, features = [ "rt" ]} nrf52832-pac = { version = "0.9.0", optional = true, features = [ "rt" ]} nrf52833-pac = { version = "0.9.0", optional = true, features = [ "rt" ]} nrf52840-pac = { version = "0.9.0", optional = true, features = [ "rt" ]} +critical-section = "0.2.0" diff --git a/embassy-nrf/src/interrupt.rs b/embassy-nrf/src/interrupt.rs index 741dbaee..8d069e32 100644 --- a/embassy-nrf/src/interrupt.rs +++ b/embassy-nrf/src/interrupt.rs @@ -8,7 +8,6 @@ use core::sync::atomic::{compiler_fence, Ordering}; use crate::pac::NVIC_PRIO_BITS; // Re-exports -pub use cortex_m::interrupt::{CriticalSection, Mutex}; pub use embassy::interrupt::{declare, take, Interrupt}; #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)] @@ -47,35 +46,6 @@ impl From for u8 { } } -#[inline] -pub fn free(f: F) -> R -where - F: FnOnce(&CriticalSection) -> R, -{ - unsafe { - // TODO: assert that we're in privileged level - // Needed because disabling irqs in non-privileged level is a noop, which would break safety. - - let primask: u32; - asm!("mrs {}, PRIMASK", out(reg) primask); - - asm!("cpsid i"); - - // Prevent compiler from reordering operations inside/outside the critical section. - compiler_fence(Ordering::SeqCst); - - let r = f(&CriticalSection::new()); - - compiler_fence(Ordering::SeqCst); - - if primask & 1 == 0 { - asm!("cpsie i"); - } - - r - } -} - #[cfg(feature = "52810")] mod irqs { use super::*; diff --git a/embassy-nrf/src/rtc.rs b/embassy-nrf/src/rtc.rs index cbb5a87e..dc0e3ceb 100644 --- a/embassy-nrf/src/rtc.rs +++ b/embassy-nrf/src/rtc.rs @@ -1,10 +1,11 @@ use core::cell::Cell; use core::sync::atomic::{compiler_fence, AtomicU32, Ordering}; - +use critical_section::CriticalSection; use embassy::interrupt::InterruptExt; use embassy::time::Clock; +use embassy::util::CriticalSectionMutex as Mutex; -use crate::interrupt::{CriticalSection, Interrupt, Mutex}; +use crate::interrupt::Interrupt; use crate::pac; use crate::{interrupt, peripherals}; @@ -134,7 +135,7 @@ impl RTC { for n in 0..ALARM_COUNT { if r.events_compare[n].read().bits() == 1 { r.events_compare[n].write(|w| w); - interrupt::free(|cs| { + critical_section::with(|cs| { self.trigger_alarm(n, cs); }) } @@ -142,7 +143,7 @@ impl RTC { } fn next_period(&self) { - interrupt::free(|cs| { + critical_section::with(|cs| { let r = self.rtc.regs(); let period = self.period.fetch_add(1, Ordering::Relaxed) + 1; let t = (period as u64) << 23; @@ -160,7 +161,7 @@ impl RTC { }) } - fn trigger_alarm(&self, n: usize, cs: &CriticalSection) { + fn trigger_alarm(&self, n: usize, cs: CriticalSection) { let r = self.rtc.regs(); r.intenclr.write(|w| unsafe { w.bits(compare_n(n)) }); @@ -174,14 +175,14 @@ impl RTC { } fn set_alarm_callback(&self, n: usize, callback: fn(*mut ()), ctx: *mut ()) { - interrupt::free(|cs| { + critical_section::with(|cs| { let alarm = &self.alarms.borrow(cs)[n]; alarm.callback.set(Some((callback, ctx))); }) } fn set_alarm(&self, n: usize, timestamp: u64) { - interrupt::free(|cs| { + critical_section::with(|cs| { let alarm = &self.alarms.borrow(cs)[n]; alarm.timestamp.set(timestamp); @@ -289,5 +290,5 @@ pub trait Instance: sealed::Instance + 'static { impl_instance!(RTC0, RTC0); impl_instance!(RTC1, RTC1); -#[cfg(any(feature = "52832", feature = "52833", feature = "52840"))] +#[cfg(any(feature = "nrf52832", feature = "nrf52833", feature = "nrf52840"))] impl_instance!(RTC2, RTC2); diff --git a/embassy-rp/Cargo.toml b/embassy-rp/Cargo.toml index 1d11fb0d..59331dd1 100644 --- a/embassy-rp/Cargo.toml +++ b/embassy-rp/Cargo.toml @@ -20,6 +20,7 @@ defmt = { version = "0.2.0", optional = true } log = { version = "0.4.11", optional = true } cortex-m-rt = "0.6.13" cortex-m = "0.7.1" +critical-section = "0.2.0" rp2040-pac2 = { git = "https://github.com/Dirbaio/rp2040-pac", rev="254f4677937801155ca3cb17c7bb9d38eb62683e", features = ["rt"] } embedded-hal = { version = "0.2.4", features = [ "unproven" ] } diff --git a/embassy-rp/src/interrupt.rs b/embassy-rp/src/interrupt.rs index a913ea60..cb9f3654 100644 --- a/embassy-rp/src/interrupt.rs +++ b/embassy-rp/src/interrupt.rs @@ -8,7 +8,6 @@ use core::sync::atomic::{compiler_fence, Ordering}; use crate::pac::NVIC_PRIO_BITS; // Re-exports -pub use cortex_m::interrupt::{CriticalSection, Mutex}; pub use embassy::interrupt::{declare, take, Interrupt}; #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)] @@ -47,35 +46,6 @@ impl From for u8 { } } -#[inline] -pub fn free(f: F) -> R -where - F: FnOnce(&CriticalSection) -> R, -{ - unsafe { - // TODO: assert that we're in privileged level - // Needed because disabling irqs in non-privileged level is a noop, which would break safety. - - let primask: u32; - asm!("mrs {}, PRIMASK", out(reg) primask); - - asm!("cpsid i"); - - // Prevent compiler from reordering operations inside/outside the critical section. - compiler_fence(Ordering::SeqCst); - - let r = f(&CriticalSection::new()); - - compiler_fence(Ordering::SeqCst); - - if primask & 1 == 0 { - asm!("cpsie i"); - } - - r - } -} - mod irqs { use super::*; diff --git a/embassy-stm32/Cargo.toml b/embassy-stm32/Cargo.toml index af30f1aa..36bc0f25 100644 --- a/embassy-stm32/Cargo.toml +++ b/embassy-stm32/Cargo.toml @@ -39,6 +39,7 @@ embassy-macros = { version = "0.1.0", path = "../embassy-macros", features = ["s embassy-extras = {version = "0.1.0", path = "../embassy-extras" } atomic-polyfill = "0.1.1" +critical-section = "0.2.0" defmt = { version = "0.2.0", optional = true } log = { version = "0.4.11", optional = true } cortex-m-rt = "0.6.13" diff --git a/embassy-stm32/src/exti.rs b/embassy-stm32/src/exti.rs index 9a12f811..3d83c2d5 100644 --- a/embassy-stm32/src/exti.rs +++ b/embassy-stm32/src/exti.rs @@ -44,7 +44,7 @@ pub struct ExtiPin { impl ExtiPin { pub fn new(mut pin: T, interrupt: T::Interrupt, syscfg: &mut SysCfg) -> Self { - cortex_m::interrupt::free(|_| { + critical_section::with(|_| { pin.make_source(syscfg); }); @@ -99,7 +99,7 @@ impl ExtiPin { async move { let fut = InterruptFuture::new(&mut self.interrupt); let pin = &mut self.pin; - cortex_m::interrupt::free(|_| { + critical_section::with(|_| { pin.trigger_edge(if state { EdgeOption::Rising } else { @@ -126,7 +126,7 @@ impl ExtiPin { async move { let fut = InterruptFuture::new(&mut self.interrupt); let pin = &mut self.pin; - cortex_m::interrupt::free(|_| { + critical_section::with(|_| { pin.trigger_edge(state); }); diff --git a/embassy-stm32/src/f4/rtc.rs b/embassy-stm32/src/f4/rtc.rs index b1abba32..df7a82d8 100644 --- a/embassy-stm32/src/f4/rtc.rs +++ b/embassy-stm32/src/f4/rtc.rs @@ -3,12 +3,13 @@ use crate::hal::rcc::Clocks; use core::cell::Cell; use core::convert::TryInto; use core::sync::atomic::{compiler_fence, AtomicU32, Ordering}; - +use critical_section::CriticalSection; use embassy::interrupt::InterruptExt; use embassy::time::{Clock, TICKS_PER_SECOND}; +use embassy::util::CriticalSectionMutex as Mutex; use crate::interrupt; -use crate::interrupt::{CriticalSection, Interrupt, Mutex}; +use crate::interrupt::Interrupt; // RTC timekeeping works with something we call "periods", which are time intervals // of 2^15 ticks. The RTC counter value is 16 bits, so one "overflow cycle" is 2 periods. @@ -119,13 +120,13 @@ impl RTC { for n in 1..=ALARM_COUNT { if self.rtc.compare_interrupt_status(n) { self.rtc.compare_clear_flag(n); - interrupt::free(|cs| self.trigger_alarm(n, cs)); + critical_section::with(|cs| self.trigger_alarm(n, cs)); } } } fn next_period(&self) { - interrupt::free(|cs| { + critical_section::with(|cs| { let period = self.period.fetch_add(1, Ordering::Relaxed) + 1; let t = (period as u64) << 15; @@ -142,7 +143,7 @@ impl RTC { }) } - fn trigger_alarm(&self, n: usize, cs: &CriticalSection) { + fn trigger_alarm(&self, n: usize, cs: CriticalSection) { self.rtc.set_compare_interrupt(n, false); let alarm = &self.alarms.borrow(cs)[n - 1]; @@ -155,14 +156,14 @@ impl RTC { } fn set_alarm_callback(&self, n: usize, callback: fn(*mut ()), ctx: *mut ()) { - interrupt::free(|cs| { + critical_section::with(|cs| { let alarm = &self.alarms.borrow(cs)[n - 1]; alarm.callback.set(Some((callback, ctx))); }) } fn set_alarm(&self, n: usize, timestamp: u64) { - interrupt::free(|cs| { + critical_section::with(|cs| { let alarm = &self.alarms.borrow(cs)[n - 1]; alarm.timestamp.set(timestamp); diff --git a/embassy-stm32/src/interrupt.rs b/embassy-stm32/src/interrupt.rs index 63b80a6c..1d48dc58 100644 --- a/embassy-stm32/src/interrupt.rs +++ b/embassy-stm32/src/interrupt.rs @@ -8,7 +8,6 @@ use core::sync::atomic::{compiler_fence, Ordering}; use crate::pac::NVIC_PRIO_BITS; // Re-exports -pub use cortex_m::interrupt::{CriticalSection, Mutex}; pub use embassy::interrupt::{declare, take, Interrupt}; #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)] @@ -63,35 +62,6 @@ impl From for u8 { } } -#[inline] -pub fn free(f: F) -> R -where - F: FnOnce(&CriticalSection) -> R, -{ - unsafe { - // TODO: assert that we're in privileged level - // Needed because disabling irqs in non-privileged level is a noop, which would break safety. - - let primask: u32; - asm!("mrs {}, PRIMASK", out(reg) primask); - - asm!("cpsid i"); - - // Prevent compiler from reordering operations inside/outside the critical section. - compiler_fence(Ordering::SeqCst); - - let r = f(&CriticalSection::new()); - - compiler_fence(Ordering::SeqCst); - - if primask & 1 == 0 { - asm!("cpsie i"); - } - - r - } -} - #[cfg(feature = "stm32f401")] mod irqs { use super::*; diff --git a/embassy-stm32/src/l0/rtc.rs b/embassy-stm32/src/l0/rtc.rs index 9a134204..fa5011d3 100644 --- a/embassy-stm32/src/l0/rtc.rs +++ b/embassy-stm32/src/l0/rtc.rs @@ -2,12 +2,13 @@ use crate::hal::rcc::Clocks; use atomic_polyfill::{compiler_fence, AtomicU32, Ordering}; use core::cell::Cell; use core::convert::TryInto; - +use critical_section::CriticalSection; use embassy::interrupt::InterruptExt; use embassy::time::{Clock, TICKS_PER_SECOND}; +use embassy::util::CriticalSectionMutex as Mutex; use crate::interrupt; -use crate::interrupt::{CriticalSection, Interrupt, Mutex}; +use crate::interrupt::Interrupt; // RTC timekeeping works with something we call "periods", which are time intervals // of 2^15 ticks. The RTC counter value is 16 bits, so one "overflow cycle" is 2 periods. @@ -117,13 +118,13 @@ impl RTC { for n in 1..=ALARM_COUNT { if self.rtc.compare_interrupt_status(n) { self.rtc.compare_clear_flag(n); - interrupt::free(|cs| self.trigger_alarm(n, cs)); + critical_section::with(|cs| self.trigger_alarm(n, cs)); } } } fn next_period(&self) { - interrupt::free(|cs| { + critical_section::with(|cs| { let period = self.period.fetch_add(1, Ordering::Relaxed) + 1; let t = (period as u64) << 15; @@ -140,7 +141,7 @@ impl RTC { }) } - fn trigger_alarm(&self, n: usize, cs: &CriticalSection) { + fn trigger_alarm(&self, n: usize, cs: CriticalSection) { self.rtc.set_compare_interrupt(n, false); let alarm = &self.alarms.borrow(cs)[n - 1]; @@ -153,14 +154,14 @@ impl RTC { } fn set_alarm_callback(&self, n: usize, callback: fn(*mut ()), ctx: *mut ()) { - interrupt::free(|cs| { + critical_section::with(|cs| { let alarm = &self.alarms.borrow(cs)[n - 1]; alarm.callback.set(Some((callback, ctx))); }) } fn set_alarm(&self, n: usize, timestamp: u64) { - interrupt::free(|cs| { + critical_section::with(|cs| { let alarm = &self.alarms.borrow(cs)[n - 1]; alarm.timestamp.set(timestamp); diff --git a/embassy/Cargo.toml b/embassy/Cargo.toml index 2f3ee453..2395eeae 100644 --- a/embassy/Cargo.toml +++ b/embassy/Cargo.toml @@ -25,6 +25,7 @@ pin-project = { version = "1.0.2", default-features = false } embassy-macros = { version = "0.1.0", path = "../embassy-macros"} embassy-traits = { version = "0.1.0", path = "../embassy-traits"} atomic-polyfill = { version = "0.1.1" } +critical-section = "0.2.0" # Workaround https://github.com/japaric/cast.rs/pull/27 cast = { version = "=0.2.3", default-features = false } diff --git a/embassy/src/util/critical_section.rs b/embassy/src/util/critical_section.rs deleted file mode 100644 index 12ad6f20..00000000 --- a/embassy/src/util/critical_section.rs +++ /dev/null @@ -1,30 +0,0 @@ -pub use cs::{critical_section, CriticalSection}; - -#[cfg(feature = "std")] -mod cs { - static INIT: std::sync::Once = std::sync::Once::new(); - static mut BKL: Option> = None; - - pub type CriticalSection = std::sync::MutexGuard<'static, ()>; - pub fn critical_section(f: F) -> R - where - F: FnOnce(&CriticalSection) -> R, - { - INIT.call_once(|| unsafe { - BKL.replace(std::sync::Mutex::new(())); - }); - let guard = unsafe { BKL.as_ref().unwrap().lock().unwrap() }; - f(&guard) - } -} - -#[cfg(not(feature = "std"))] -mod cs { - pub use cortex_m::interrupt::CriticalSection; - pub fn critical_section(f: F) -> R - where - F: FnOnce(&CriticalSection) -> R, - { - cortex_m::interrupt::free(f) - } -} diff --git a/embassy/src/util/mod.rs b/embassy/src/util/mod.rs index cace0d19..6aa3a733 100644 --- a/embassy/src/util/mod.rs +++ b/embassy/src/util/mod.rs @@ -1,5 +1,4 @@ //! Async utilities -mod critical_section; mod drop_bomb; mod forever; mod mutex; @@ -10,7 +9,6 @@ mod signal; #[cfg_attr(feature = "executor-agnostic", path = "waker_agnostic.rs")] mod waker; -pub use critical_section::*; pub use drop_bomb::*; pub use forever::*; pub use mutex::*; diff --git a/embassy/src/util/mutex.rs b/embassy/src/util/mutex.rs index ed2fd785..86ab070a 100644 --- a/embassy/src/util/mutex.rs +++ b/embassy/src/util/mutex.rs @@ -1,5 +1,5 @@ use core::cell::UnsafeCell; -use cortex_m::interrupt::CriticalSection; +use critical_section::CriticalSection; use crate::fmt::assert; @@ -30,7 +30,7 @@ impl CriticalSectionMutex { impl CriticalSectionMutex { /// Borrows the data for the duration of the critical section - pub fn borrow<'cs>(&'cs self, _cs: &'cs CriticalSection) -> &'cs T { + pub fn borrow<'cs>(&'cs self, _cs: CriticalSection<'cs>) -> &'cs T { unsafe { &*self.inner.get() } } } diff --git a/embassy/src/util/signal.rs b/embassy/src/util/signal.rs index a62abcd5..2eefe658 100644 --- a/embassy/src/util/signal.rs +++ b/embassy/src/util/signal.rs @@ -11,7 +11,6 @@ use ptr::NonNull; use crate::executor; use crate::fmt::panic; use crate::interrupt::{Interrupt, InterruptExt}; -use crate::util::critical_section::critical_section; /// Synchronization primitive. Allows creating awaitable signals that may be passed between tasks. /// @@ -38,7 +37,7 @@ impl Signal { /// Mark this Signal as completed. pub fn signal(&self, val: T) { - critical_section(|_| unsafe { + critical_section::with(|_| unsafe { let state = &mut *self.state.get(); if let State::Waiting(waker) = mem::replace(state, State::Signaled(val)) { waker.wake(); @@ -47,14 +46,14 @@ impl Signal { } pub fn reset(&self) { - critical_section(|_| unsafe { + critical_section::with(|_| unsafe { let state = &mut *self.state.get(); *state = State::None }) } pub fn poll_wait(&self, cx: &mut Context<'_>) -> Poll { - critical_section(|_| unsafe { + critical_section::with(|_| unsafe { let state = &mut *self.state.get(); match state { State::None => { @@ -78,7 +77,7 @@ impl Signal { /// non-blocking method to check whether this signal has been signaled. pub fn signaled(&self) -> bool { - critical_section(|_| matches!(unsafe { &*self.state.get() }, State::Signaled(_))) + critical_section::with(|_| matches!(unsafe { &*self.state.get() }, State::Signaled(_))) } } diff --git a/embassy/src/util/waker_agnostic.rs b/embassy/src/util/waker_agnostic.rs index 3f6ad373..53f1ec13 100644 --- a/embassy/src/util/waker_agnostic.rs +++ b/embassy/src/util/waker_agnostic.rs @@ -62,7 +62,7 @@ impl AtomicWaker { /// Register a waker. Overwrites the previous waker, if any. pub fn register(&mut self, w: &Waker) { - cortex_m::interrupt::free(|cs| { + critical_section::with(|cs| { let cell = self.waker.borrow(cs); cell.set(match cell.replace(None) { Some(w2) if (w2.will_wake(w)) => Some(w2), @@ -73,7 +73,7 @@ impl AtomicWaker { /// Wake the registered waker, if any. pub fn wake(&mut self) { - cortex_m::interrupt::free(|cs| { + critical_section::with(|cs| { let cell = self.waker.borrow(cs); if let Some(w) = cell.replace(None) { w.wake_by_ref();