Trim down xtensa-lx (#2357)

* Remove mutex, InterruptNumber, bare_metal and spin

* Changelog

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 {

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"

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

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"]

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, R>(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
 }

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;
+}

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)
+    };
 }

xtensa-lx/src/mutex.rs

@@ -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<T> {
-    data: spin::Mutex<T>,
-}
-
-#[cfg(feature = "spin")]
-impl<T> CriticalSectionSpinLockMutex<T> {
-    /// Create a new mutex
-    pub const fn new(data: T) -> Self {
-        CriticalSectionSpinLockMutex {
-            data: spin::Mutex::new(data),
-        }
-    }
-}
-
-#[cfg(feature = "spin")]
-impl<T> mutex_trait::Mutex for &'_ CriticalSectionSpinLockMutex<T> {
-    type Data = T;
-
-    fn lock<R>(&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<T> Sync for CriticalSectionSpinLockMutex<T> 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<T> {
-    data: UnsafeCell<T>,
-}
-
-impl<T> CriticalSectionMutex<T> {
-    /// Create a new mutex
-    pub const fn new(data: T) -> Self {
-        CriticalSectionMutex {
-            data: UnsafeCell::new(data),
-        }
-    }
-}
-
-impl<T> mutex_trait::Mutex for &'_ CriticalSectionMutex<T> {
-    type Data = T;
-
-    fn lock<R>(&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<T> Sync for CriticalSectionMutex<T> where T: Send {}
-
-/// A spinlock based mutex.
-#[cfg(feature = "spin")]
-#[derive(Default)]
-pub struct SpinLockMutex<T> {
-    data: spin::Mutex<T>,
-}
-
-#[cfg(feature = "spin")]
-impl<T> SpinLockMutex<T> {
-    /// Create a new mutex
-    pub const fn new(data: T) -> Self {
-        SpinLockMutex {
-            data: spin::Mutex::new(data),
-        }
-    }
-}
-
-#[cfg(feature = "spin")]
-impl<T> mutex_trait::Mutex for &'_ SpinLockMutex<T> {
-    type Data = T;
-
-    fn lock<R>(&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<T> Sync for SpinLockMutex<T> where T: Send {}