From 09511c750aae5cbc0f5d7def6ae4b7201768689a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Sat, 23 Nov 2024 02:01:37 +0100 Subject: [PATCH] Trim down `xtensa-lx` (#2357) * Remove mutex, InterruptNumber, bare_metal and spin * Changelog --- esp-hal/src/uart.rs | 2 +- xtensa-lx-rt/Cargo.toml | 1 - xtensa-lx/CHANGELOG.md | 7 +++ xtensa-lx/Cargo.toml | 9 +--- xtensa-lx/src/interrupt.rs | 40 ++++---------- xtensa-lx/src/lib.rs | 8 ++- xtensa-lx/src/macros.rs | 24 +++++---- xtensa-lx/src/mutex.rs | 104 ------------------------------------- 8 files changed, 39 insertions(+), 156 deletions(-) delete mode 100644 xtensa-lx/src/mutex.rs diff --git a/esp-hal/src/uart.rs b/esp-hal/src/uart.rs index 06cab8043b1..46d02e72cf6 100644 --- a/esp-hal/src/uart.rs +++ b/esp-hal/src/uart.rs @@ -796,7 +796,7 @@ where cfg_if::cfg_if! { if #[cfg(esp32)] { // https://docs.espressif.com/projects/esp-chip-errata/en/latest/esp32/03-errata-description/esp32/cpu-subsequent-access-halted-when-get-interrupted.html - xtensa_lx::interrupt::free(|_| { + xtensa_lx::interrupt::free(|| { *byte = fifo.read().rxfifo_rd_byte().bits(); }); } else { diff --git a/xtensa-lx-rt/Cargo.toml b/xtensa-lx-rt/Cargo.toml index aef79421bda..6172ef31590 100644 --- a/xtensa-lx-rt/Cargo.toml +++ b/xtensa-lx-rt/Cargo.toml @@ -13,7 +13,6 @@ categories = ["embedded", "hardware-support", "no-std"] features = ["esp32"] [dependencies] -bare-metal = "1.0.0" document-features = "0.2.10" macros = { version = "0.2.2", package = "xtensa-lx-rt-proc-macros", path = "./procmacros" } r0 = "1.0.0" diff --git a/xtensa-lx/CHANGELOG.md b/xtensa-lx/CHANGELOG.md index 4161f06a347..0eaa71db7a3 100644 --- a/xtensa-lx/CHANGELOG.md +++ b/xtensa-lx/CHANGELOG.md @@ -11,10 +11,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fixed `interrupt:free` incorrectly providing `CriticalSection` (#2537) + ### Changed +- The `singleton` macro has been updated to match the cortex-m counterpart (#2537) + ### Removed +- The `spin` feature and `mutex` module has been removed. (#2537) +- The `InterruptNumber` trait has been removed. (#2537) + ## [0.9.0] - 2024-02-21 ## [0.8.0] - 2023-02-23 diff --git a/xtensa-lx/Cargo.toml b/xtensa-lx/Cargo.toml index 04091fac59f..10d55c51d8a 100644 --- a/xtensa-lx/Cargo.toml +++ b/xtensa-lx/Cargo.toml @@ -10,15 +10,8 @@ categories = ["embedded", "hardware-support", "no-std"] keywords = ["lx", "peripheral", "register", "xtensa"] links = "xtensa-lx" -[package.metadata.docs.rs] -features = ["spin"] - [dependencies] -bare-metal = "1.0.0" +critical-section = "1.0.0" document-features = "0.2.10" -mutex-trait = "0.2.0" -spin = { version = "0.9.8", optional = true } [features] -## Use the [spin] package for synchronization -spin = ["dep:spin"] diff --git a/xtensa-lx/src/interrupt.rs b/xtensa-lx/src/interrupt.rs index c899398af15..14583d7b5f4 100644 --- a/xtensa-lx/src/interrupt.rs +++ b/xtensa-lx/src/interrupt.rs @@ -2,29 +2,6 @@ use core::arch::asm; -pub use bare_metal::CriticalSection; - -/// Trait for enums of external interrupt numbers. -/// -/// This trait should be implemented by a peripheral access crate (PAC) -/// on its enum of available external interrupts for a specific device. -/// Each variant must convert to a u16 of its interrupt number, -/// which is its exception number - 16. -/// -/// # Safety -/// -/// This trait must only be implemented on enums of device interrupts. Each -/// enum variant must represent a distinct value (no duplicates are permitted), -/// and must always return the same value (do not change at runtime). -/// -/// These requirements ensure safe nesting of critical sections. -pub unsafe trait InterruptNumber: Copy { - /// Return the interrupt number associated with this variant. - /// - /// See trait documentation for safety requirements. - fn number(self) -> u16; -} - /// Disables all interrupts and return the previous settings #[inline] pub fn disable() -> u32 { @@ -64,7 +41,7 @@ pub fn disable_mask(mask: u32) -> u32 { let _dummy: u32; unsafe { asm!(" - xsr.intenable {0} // get mask and temporarily disable interrupts + xsr.intenable {0} // get mask and temporarily disable interrupts and {1}, {1}, {0} rsync wsr.intenable {1} @@ -154,21 +131,22 @@ pub fn get_level() -> u32 { /// Execute closure `f` in an interrupt-free context. /// -/// This as also known as a "critical section". +/// This method does not synchronise multiple cores, so it is not suitable for +/// using as a critical section. See the `critical-section` crate for a +/// cross-platform way to enter a critical section which provides a +/// `CriticalSection` token. #[inline] pub fn free(f: F) -> R where - F: FnOnce(&CriticalSection) -> R, + F: FnOnce() -> R, { // disable interrupts and store old mask let old_mask = disable(); - let r = f(unsafe { &CriticalSection::new() }); + let r = f(); - // enable previously disable interrupts - unsafe { - enable_mask(old_mask); - } + // enable previously disabled interrupts + unsafe { enable_mask(old_mask) }; r } diff --git a/xtensa-lx/src/lib.rs b/xtensa-lx/src/lib.rs index d334dae215c..cc8eb076885 100644 --- a/xtensa-lx/src/lib.rs +++ b/xtensa-lx/src/lib.rs @@ -15,7 +15,6 @@ use core::arch::asm; pub mod interrupt; -pub mod mutex; pub mod timer; #[macro_use] @@ -111,3 +110,10 @@ pub fn is_debugger_attached() -> bool { pub fn debug_break() { unsafe { asm!("break 1, 15", options(nostack)) }; } + +/// Used to reexport items for use in macros. Do not use directly. +/// Not covered by semver guarantees. +#[doc(hidden)] +pub mod _export { + pub use critical_section; +} diff --git a/xtensa-lx/src/macros.rs b/xtensa-lx/src/macros.rs index 31c7a0a4b30..673fcfc21db 100644 --- a/xtensa-lx/src/macros.rs +++ b/xtensa-lx/src/macros.rs @@ -26,12 +26,17 @@ /// ``` #[macro_export] macro_rules! singleton { - (: $ty:ty = $expr:expr) => { - $crate::interrupt::free(|_| { - static mut VAR: Option<$ty> = None; + ($(#[$meta:meta])* $name:ident: $ty:ty = $expr:expr) => { + $crate::_export::critical_section::with(|_| { + // this is a tuple of a MaybeUninit and a bool because using an Option here is + // problematic: Due to niche-optimization, an Option could end up producing a non-zero + // initializer value which would move the entire static from `.bss` into `.data`... + $(#[$meta])* + static mut $name: (::core::mem::MaybeUninit<$ty>, bool) = + (::core::mem::MaybeUninit::uninit(), false); #[allow(unsafe_code)] - let used = unsafe { VAR.is_some() }; + let used = unsafe { $name.1 }; if used { None } else { @@ -39,14 +44,13 @@ macro_rules! singleton { #[allow(unsafe_code)] unsafe { - VAR = Some(expr) - } - - #[allow(unsafe_code)] - unsafe { - VAR.as_mut() + $name.1 = true; + Some($name.0.write(expr)) } } }) }; + ($(#[$meta:meta])* : $ty:ty = $expr:expr) => { + $crate::singleton!($(#[$meta])* VAR: $ty = $expr) + }; } diff --git a/xtensa-lx/src/mutex.rs b/xtensa-lx/src/mutex.rs deleted file mode 100644 index cbb0b7e4607..00000000000 --- a/xtensa-lx/src/mutex.rs +++ /dev/null @@ -1,104 +0,0 @@ -//! A series of Mutex's that also implements the `mutex-trait`. - -use core::cell::UnsafeCell; - -pub use mutex_trait::{self, Mutex}; - -/// A spinlock and critical section section based mutex. -#[cfg(feature = "spin")] -#[derive(Default)] -pub struct CriticalSectionSpinLockMutex { - data: spin::Mutex, -} - -#[cfg(feature = "spin")] -impl CriticalSectionSpinLockMutex { - /// Create a new mutex - pub const fn new(data: T) -> Self { - CriticalSectionSpinLockMutex { - data: spin::Mutex::new(data), - } - } -} - -#[cfg(feature = "spin")] -impl mutex_trait::Mutex for &'_ CriticalSectionSpinLockMutex { - type Data = T; - - fn lock(&mut self, f: impl FnOnce(&mut Self::Data) -> R) -> R { - crate::interrupt::free(|_| f(&mut (*self.data.lock()))) - } -} - -// NOTE A `Mutex` can be used as a channel so the protected data must be `Send` -// to prevent sending non-Sendable stuff (e.g. access tokens) across different -// execution contexts (e.g. interrupts) -#[cfg(feature = "spin")] -unsafe impl Sync for CriticalSectionSpinLockMutex where T: Send {} - -/// A Mutex based on critical sections -/// -/// # Safety -/// -/// **This Mutex is only safe on single-core applications.** -/// -/// A `CriticalSection` **is not sufficient** to ensure exclusive access across -/// cores. -#[derive(Default)] -pub struct CriticalSectionMutex { - data: UnsafeCell, -} - -impl CriticalSectionMutex { - /// Create a new mutex - pub const fn new(data: T) -> Self { - CriticalSectionMutex { - data: UnsafeCell::new(data), - } - } -} - -impl mutex_trait::Mutex for &'_ CriticalSectionMutex { - type Data = T; - - fn lock(&mut self, f: impl FnOnce(&mut Self::Data) -> R) -> R { - crate::interrupt::free(|_| f(unsafe { &mut *self.data.get() })) - } -} - -// NOTE A `Mutex` can be used as a channel so the protected data must be `Send` -// to prevent sending non-Sendable stuff (e.g. access tokens) across different -// execution contexts (e.g. interrupts) -unsafe impl Sync for CriticalSectionMutex where T: Send {} - -/// A spinlock based mutex. -#[cfg(feature = "spin")] -#[derive(Default)] -pub struct SpinLockMutex { - data: spin::Mutex, -} - -#[cfg(feature = "spin")] -impl SpinLockMutex { - /// Create a new mutex - pub const fn new(data: T) -> Self { - SpinLockMutex { - data: spin::Mutex::new(data), - } - } -} - -#[cfg(feature = "spin")] -impl mutex_trait::Mutex for &'_ SpinLockMutex { - type Data = T; - - fn lock(&mut self, f: impl FnOnce(&mut Self::Data) -> R) -> R { - f(&mut (*self.data.lock())) - } -} - -// NOTE A `Mutex` can be used as a channel so the protected data must be `Send` -// to prevent sending non-Sendable stuff (e.g. access tokens) across different -// execution contexts (e.g. interrupts) -#[cfg(feature = "spin")] -unsafe impl Sync for SpinLockMutex where T: Send {}