Cherrypick old cortex-m PRs

.github/workflows/ci.yml

@@ -1,7 +1,4 @@
 on:
-  push:
-    branches-ignore:
-      - "gh-readonly-queue/**"
   pull_request:
   merge_group:
   workflow_dispatch:

.github/workflows/clippy.yml

@@ -1,7 +1,4 @@
 on:
-  push:
-    branches-ignore:
-      - "gh-readonly-queue/**"
   pull_request:
   merge_group:
   workflow_dispatch:

.github/workflows/on-target.yml

@@ -1,7 +1,4 @@
 on:
-  push:
-    branches-ignore:
-      - "gh-readonly-queue/**"
   pull_request:
   merge_group:
   # allows manual triggering

.github/workflows/rt-ci.yml

@@ -1,7 +1,4 @@
 on:
-  push:
-    branches-ignore:
-      - "gh-readonly-queue/**"
   pull_request:
   merge_group:
   workflow_dispatch:

.github/workflows/rustfmt.yml

@@ -1,7 +1,4 @@
 on:
-  push:
-    branches-ignore:
-      - "gh-readonly-queue/**"
   pull_request:
   merge_group:
   workflow_dispatch:

cortex-m/Cargo.toml

@@ -18,10 +18,11 @@ rust-version = "1.61"
 
 [dependencies]
 bare-metal = { version = "0.2.4", features = ["const-fn"] }
-critical-section = { version = "1.0.0", optional = true }
-volatile-register = "0.2.0"
+critical-section = "1.0.0"
+volatile-register = "0.2.2"
 bitfield = "0.13.2"
-embedded-hal = "0.2.4"
+eh0 = { package = "embedded-hal", version = "0.2.4" }
+eh1 = { package = "embedded-hal", version = "1.0.0" }
 
 [dependencies.serde]
 version = "1"
@@ -38,7 +39,7 @@ cm7-r0p1 = ["cm7"]
 inline-asm = []
 linker-plugin-lto = []
 std = []
-critical-section-single-core = ["critical-section/restore-state-bool"]
+critical-section-single-core = ["critical-section/restore-state-u32"]
 
 [package.metadata.docs.rs]
 targets = [

cortex-m/asm/inline.rs

@@ -60,6 +60,10 @@ pub unsafe fn __delay(cyc: u32) {
     // Add 1 to prevent an integer underflow which would cause a long freeze
     let real_cyc = 1 + cyc / 2;
     asm!(
+        // The `bne` on some cores (eg Cortex-M4) will take a different number of instructions
+        // depending on the alignment of the branch target.  Set the alignment of the top of the
+        // loop to prevent surprising timing changes when the alignment of the delay() changes.
+        ".p2align 3",
         // Use local labels to avoid R_ARM_THM_JUMP8 relocations which fail on thumbv6m.
         "1:",
         "subs {}, #1",

cortex-m/src/asm.rs

@@ -17,13 +17,18 @@ pub fn bkpt() {
 
 /// Blocks the program for *at least* `cycles` CPU cycles.
 ///
-/// This is implemented in assembly so its execution time is independent of the optimization
-/// level, however it is dependent on the specific architecture and core configuration.
-///
-/// NOTE that the delay can take much longer if interrupts are serviced during its execution
-/// and the execution time may vary with other factors. This delay is mainly useful for simple
-/// timer-less initialization of peripherals if and only if accurate timing is not essential. In
-/// any other case please use a more accurate method to produce a delay.
+/// This is implemented in assembly as a fixed number of iterations of a loop, so that execution
+/// time is independent of the optimization level.
+///
+/// The loop code is the same for all architectures, however the number of CPU cycles required for
+/// one iteration varies substantially between architectures.  This means that with a 48MHz CPU
+/// clock, a call to `delay(48_000_000)` is guaranteed to take at least 1 second, but for example
+/// could take 2 seconds.
+///
+/// NOTE that the delay can take much longer if interrupts are serviced during its execution and the
+/// execution time may vary with other factors. This delay is mainly useful for simple timer-less
+/// initialization of peripherals if and only if accurate timing is not essential. In any other case
+/// please use a more accurate method to produce a delay.
 #[inline]
 pub fn delay(cycles: u32) {
     call_asm!(__delay(cycles: u32));

cortex-m/src/critical_section.rs

@@ -1,25 +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 {
-            let was_active = primask::read().is_active();
-            interrupt::disable();
-            was_active
-        }
+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(was_active: RawRestoreState) {
-            // Only re-enable interrupts if they were enabled before the critical section.
-            if was_active {
-                interrupt::enable()
-            }
-        }
+    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/delay.rs

@@ -1,7 +1,7 @@
 //! A delay driver based on SysTick.
 
 use crate::peripheral::{syst::SystClkSource, SYST};
-use embedded_hal::blocking::delay::{DelayMs, DelayUs};
+use eh1::delay::DelayNs;
 
 /// System timer (SysTick) as a delay provider.
 pub struct Delay {
@@ -75,62 +75,83 @@ impl Delay {
     }
 }
 
-impl DelayMs<u32> for Delay {
+impl eh0::blocking::delay::DelayMs<u32> for Delay {
     #[inline]
     fn delay_ms(&mut self, ms: u32) {
         Delay::delay_ms(self, ms);
     }
 }
 
 // This is a workaround to allow `delay_ms(42)` construction without specifying a type.
-impl DelayMs<i32> for Delay {
+impl eh0::blocking::delay::DelayMs<i32> for Delay {
     #[inline(always)]
     fn delay_ms(&mut self, ms: i32) {
         assert!(ms >= 0);
         Delay::delay_ms(self, ms as u32);
     }
 }
 
-impl DelayMs<u16> for Delay {
+impl eh0::blocking::delay::DelayMs<u16> for Delay {
     #[inline(always)]
     fn delay_ms(&mut self, ms: u16) {
         Delay::delay_ms(self, u32::from(ms));
     }
 }
 
-impl DelayMs<u8> for Delay {
+impl eh0::blocking::delay::DelayMs<u8> for Delay {
     #[inline(always)]
     fn delay_ms(&mut self, ms: u8) {
         Delay::delay_ms(self, u32::from(ms));
     }
 }
 
-impl DelayUs<u32> for Delay {
+impl eh0::blocking::delay::DelayUs<u32> for Delay {
     #[inline]
     fn delay_us(&mut self, us: u32) {
         Delay::delay_us(self, us);
     }
 }
 
 // This is a workaround to allow `delay_us(42)` construction without specifying a type.
-impl DelayUs<i32> for Delay {
+impl eh0::blocking::delay::DelayUs<i32> for Delay {
     #[inline(always)]
     fn delay_us(&mut self, us: i32) {
         assert!(us >= 0);
         Delay::delay_us(self, us as u32);
     }
 }
 
-impl DelayUs<u16> for Delay {
+impl eh0::blocking::delay::DelayUs<u16> for Delay {
     #[inline(always)]
     fn delay_us(&mut self, us: u16) {
         Delay::delay_us(self, u32::from(us))
     }
 }
 
-impl DelayUs<u8> for Delay {
+impl eh0::blocking::delay::DelayUs<u8> for Delay {
     #[inline(always)]
     fn delay_us(&mut self, us: u8) {
         Delay::delay_us(self, u32::from(us))
     }
 }
+
+impl DelayNs for Delay {
+    #[inline]
+    fn delay_ns(&mut self, ns: u32) {
+        // from the rp2040-hal:
+        let us = ns / 1000 + if ns % 1000 == 0 { 0 } else { 1 };
+        // With rustc 1.73, this can be replaced by:
+        // let us = ns.div_ceil(1000);
+        Delay::delay_us(self, us)
+    }
+
+    #[inline]
+    fn delay_us(&mut self, us: u32) {
+        Delay::delay_us(self, us)
+    }
+
+    #[inline]
+    fn delay_ms(&mut self, ms: u32) {
+        Delay::delay_ms(self, ms)
+    }
+}

cortex-m/src/interrupt.rs

@@ -51,23 +51,37 @@ pub unsafe fn enable() {
 /// Execute closure `f` in an interrupt-free context.
 ///
 /// This as also known as a "critical section".
+#[cfg(cortex_m)]
 #[inline]
 pub fn free<F, R>(f: F) -> R
 where
     F: FnOnce(&CriticalSection) -> R,
 {
-    let primask = crate::register::primask::read();
+    // 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 primask = crate::register::primask::read_raw();
 
     // disable interrupts
     disable();
 
     let r = f(unsafe { &CriticalSection::new() });
 
-    // If the interrupts were active before our `disable` call, then re-enable
-    // them. Otherwise, keep them disabled
-    if primask.is_active() {
-        unsafe { enable() }
+    unsafe {
+        crate::register::primask::write_raw(primask);
     }
 
     r
 }
+
+// Make a `free()` function available to allow checking dependencies without specifying a target,
+// but that will panic at runtime if executed.
+#[doc(hidden)]
+#[cfg(not(cortex_m))]
+#[inline]
+pub fn free<F, R>(_: F) -> R
+where
+    F: FnOnce(&CriticalSection) -> R,
+{
+    panic!("cortex_m::interrupt::free() is only functional on cortex-m platforms");
+}

cortex-m/src/lib.rs

@@ -35,6 +35,11 @@
 //! or critical sections are managed as part of an RTOS. In these cases, you should use
 //! a target-specific implementation instead, typically provided by a HAL or RTOS crate.
 //!
+//! The critical section has been optimized to block interrupts for as few cycles as possible,
+//! but -- due to `critical-section` implementation details -- incurs branches in a normal build
+//! configuration. For minimal interrupt latency, you can achieve inlining by enabling
+//! [linker-plugin-based LTO](https://doc.rust-lang.org/rustc/linker-plugin-lto.html).
+//!
 //! ## `cm7-r0p1`
 //!
 //! This feature enables workarounds for errata found on Cortex-M7 chips with revision r0p1. Some
@@ -100,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)))]
@@ -110,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,10 +31,13 @@ macro_rules! iprintln {
 /// at most once in the whole lifetime of the program.
 ///
 /// # Notes
-/// This macro is unsound on multi core systems.
 ///
-/// For debuggability, you can set an explicit name for a singleton.  This name only shows up the
-/// the debugger and is not referencable from other code.  See example below.
+/// 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.
 ///
 /// # Example
 ///
@@ -61,11 +64,12 @@ macro_rules! iprintln {
 /// ```
 #[macro_export]
 macro_rules! singleton {
-    ($name:ident: $ty:ty = $expr:expr) => {
-        $crate::interrupt::free(|_| {
+    ($(#[$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);
 
@@ -79,14 +83,13 @@ macro_rules! singleton {
                 #[allow(unsafe_code)]
                 unsafe {
                     $name.1 = true;
-                    $name.0 = ::core::mem::MaybeUninit::new(expr);
-                    Some(&mut *$name.0.as_mut_ptr())
+                    Some($name.0.write(expr))
                 }
             }
         })
     };
-    (: $ty:ty = $expr:expr) => {
-        $crate::singleton!(VAR: $ty = $expr)
+    ($(#[$meta:meta])* : $ty:ty = $expr:expr) => {
+        $crate::singleton!($(#[$meta])* VAR: $ty = $expr)
     };
 }
 
@@ -112,3 +115,15 @@ const CFAIL: () = ();
 /// ```
 #[allow(dead_code)]
 const CPASS: () = ();
+
+/// ```
+/// use cortex_m::singleton;
+///
+/// fn foo() {
+///     // check that attributes are forwarded
+///     singleton!(#[link_section = ".bss"] FOO: u8 = 0);
+///     singleton!(#[link_section = ".bss"]: u8 = 1);
+/// }
+/// ```
+#[allow(dead_code)]
+const CPASS_ATTR: () = ();