Merge #451

451: Small critical-section-related fixes. r=adamgreig a=Dirbaio

See individual commit messages.

Co-authored-by: Dario Nieuwenhuis <[email protected]>

Author: 2 people

cortex-m/src/critical_section.rs

@@ -1,27 +1,24 @@
-#[cfg(all(cortex_m, feature = "critical-section-single-core"))]
-mod single_core_critical_section {
-    use critical_section::{set_impl, Impl, RawRestoreState};
+use critical_section::{set_impl, Impl, RawRestoreState};
 
-    use crate::interrupt;
-    use crate::register::primask;
+use crate::interrupt;
+use crate::register::primask;
 
-    struct SingleCoreCriticalSection;
-    set_impl!(SingleCoreCriticalSection);
+struct SingleCoreCriticalSection;
+set_impl!(SingleCoreCriticalSection);
 
-    unsafe impl Impl for SingleCoreCriticalSection {
-        unsafe fn acquire() -> RawRestoreState {
-            // Backup previous state of PRIMASK register. We access the entire register directly as a
-            // u32 instead of using the primask::read() function to minimize the number of processor
-            // cycles during which interrupts are disabled.
-            let restore_state = primask::read_raw();
-            // NOTE: Fence guarantees are provided by interrupt::disable(), which performs a `compiler_fence(SeqCst)`.
-            interrupt::disable();
-            restore_state
-        }
+unsafe impl Impl for SingleCoreCriticalSection {
+    unsafe fn acquire() -> RawRestoreState {
+        // Backup previous state of PRIMASK register. We access the entire register directly as a
+        // u32 instead of using the primask::read() function to minimize the number of processor
+        // cycles during which interrupts are disabled.
+        let restore_state = primask::read_raw();
+        // NOTE: Fence guarantees are provided by interrupt::disable(), which performs a `compiler_fence(SeqCst)`.
+        interrupt::disable();
+        restore_state
+    }
 
-        unsafe fn release(restore_state: RawRestoreState) {
-            // NOTE: Fence guarantees are provided by primask::write_raw(), which performs a `compiler_fence(SeqCst)`.
-            primask::write_raw(restore_state);
-        }
+    unsafe fn release(restore_state: RawRestoreState) {
+        // NOTE: Fence guarantees are provided by primask::write_raw(), which performs a `compiler_fence(SeqCst)`.
+        primask::write_raw(restore_state);
     }
 }

cortex-m/src/lib.rs

@@ -105,7 +105,6 @@ mod macros;
 pub mod asm;
 #[cfg(armv8m)]
 pub mod cmse;
-mod critical_section;
 pub mod delay;
 pub mod interrupt;
 #[cfg(all(not(armv6m), not(armv8m_base)))]
@@ -115,3 +114,13 @@ pub mod prelude;
 pub mod register;
 
 pub use crate::peripheral::Peripherals;
+
+#[cfg(all(cortex_m, feature = "critical-section-single-core"))]
+mod critical_section;
+
+/// 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;
+}

cortex-m/src/macros.rs

@@ -31,7 +31,10 @@ macro_rules! iprintln {
 /// at most once in the whole lifetime of the program.
 ///
 /// # Notes
-/// This macro is unsound on multi core systems.
+///
+/// This macro requires a `critical-section` implementation to be set. For most single core systems,
+/// you can enable the `critical-section-single-core` feature for this crate. For other systems, you
+/// have to provide one from elsewhere, typically your chip's HAL crate.
 ///
 /// For debuggability, you can set an explicit name for a singleton. This name only shows up the
 /// debugger and is not referenceable from other code. See example below.
@@ -62,7 +65,7 @@ macro_rules! iprintln {
 #[macro_export]
 macro_rules! singleton {
     ($(#[$meta:meta])* $name:ident: $ty:ty = $expr:expr) => {
-        $crate::interrupt::free(|_| {
+        $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`...