Merge #651

651: Use `critical_section` for `Peripherals::take`. r=therealprof a=reitermarkus - `cortex_m` rust-embedded/cortex-m#447 - `msp430` rust-embedded/msp430#13 - `riscv` rust-embedded/riscv#110 - `xtensa_lx` esp-rs/xtensa-lx#20, esp-rs/esp-hal#151 - `mips_mcu` kiffie/pic32-rs#5 Co-authored-by: Markus Reiter <me@reitermark.us>
rust-embedded · Aug 24, 2022 · 7daa9ed · 7daa9ed
2 parents a39a61d + 47b7679
commit 7daa9ed
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 67 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+- Use `critical_section::with` instead of `interrupt::free` for `Peripherals::take`.
+
 ## [v0.25.1] - 2022-08-22
 
 - Fixed parentheses in RegisterBlock field accessors

diff --git a/ci/svd2rust-regress/src/svd_test.rs b/ci/svd2rust-regress/src/svd_test.rs
@@ -5,11 +5,11 @@ use std::io::prelude::*;
 use std::path::PathBuf;
 use std::process::{Command, Output};
 
-const CRATES_ALL: &[&str] = &["bare-metal = \"0.2.0\"", "vcell = \"0.1.2\""];
+const CRATES_ALL: &[&str] = &["critical-section = \"1.0\"", "vcell = \"0.1.2\""];
 const CRATES_MSP430: &[&str] = &["msp430 = \"0.2.2\"", "msp430-rt = \"0.2.0\""];
 const CRATES_MSP430_NIGHTLY: &[&str] = &["msp430-atomic = \"0.1.2\""];
-const CRATES_CORTEX_M: &[&str] = &["cortex-m = \"0.7.0\"", "cortex-m-rt = \"0.6.13\""];
-const CRATES_RISCV: &[&str] = &["riscv = \"0.5.0\"", "riscv-rt = \"0.6.0\""];
+const CRATES_CORTEX_M: &[&str] = &["cortex-m = \"0.7.6\"", "cortex-m-rt = \"0.6.13\""];
+const CRATES_RISCV: &[&str] = &["riscv = \"0.9.0\"", "riscv-rt = \"0.9.0\""];
 const CRATES_XTENSALX: &[&str] = &["xtensa-lx-rt = \"0.9.0\"", "xtensa-lx = \"0.6.0\""];
 const CRATES_MIPS: &[&str] = &["mips-mcu = \"0.1.0\""];
 const PROFILE_ALL: &[&str] = &["[profile.dev]", "incremental = false"];

diff --git a/src/generate/device.rs b/src/generate/device.rs
@@ -300,48 +300,42 @@ pub fn render(d: &Device, config: &Config, device_x: &mut String) -> Result<Toke
         }
     }
 
-    let span = Span::call_site();
-    let take = match config.target {
-        Target::CortexM => Some(Ident::new("cortex_m", span)),
-        Target::Msp430 => Some(Ident::new("msp430", span)),
-        Target::RISCV => Some(Ident::new("riscv", span)),
-        Target::XtensaLX => Some(Ident::new("xtensa_lx", span)),
-        Target::Mips => Some(Ident::new("mips_mcu", span)),
-        Target::None => None,
-    }
-    .map(|krate| {
-        quote! {
-            ///Returns all the peripherals *once*
-            #[inline]
-            pub fn take() -> Option<Self> {
-                #krate::interrupt::free(|_| {
-                    if unsafe { DEVICE_PERIPHERALS } {
-                        None
-                    } else {
-                        Some(unsafe { Peripherals::steal() })
-                    }
-                })
-            }
-        }
-    });
-
     out.extend(quote! {
         // NOTE `no_mangle` is used here to prevent linking different minor versions of the device
         // crate as that would let you `take` the device peripherals more than once (one per minor
         // version)
         #[no_mangle]
         static mut DEVICE_PERIPHERALS: bool = false;
 
-        ///All the peripherals
+        /// All the peripherals.
         #[allow(non_snake_case)]
         pub struct Peripherals {
             #fields
         }
 
         impl Peripherals {
-            #take
+            /// Returns all the peripherals *once*.
+            #[cfg(feature = "critical-section")]
+            #[inline]
+            pub fn take() -> Option<Self> {
+                critical_section::with(|_| {
+                    // SAFETY: We are in a critical section, so we have exclusive access
+                    // to `DEVICE_PERIPHERALS`.
+                    if unsafe { DEVICE_PERIPHERALS } {
+                        return None
+                    }
+
+                    // SAFETY: `DEVICE_PERIPHERALS` is set to `true` by `Peripherals::steal`,
+                    // ensuring the peripherals can only be returned once.
+                    Some(unsafe { Peripherals::steal() })
+                })
+            }
 
-            ///Unchecked version of `Peripherals::take`
+            /// Unchecked version of `Peripherals::take`.
+            ///
+            /// # Safety
+            ///
+            /// Each of the returned peripherals must be used at most once.
             #[inline]
             pub unsafe fn steal() -> Self {
                 DEVICE_PERIPHERALS = true;

diff --git a/src/lib.rs b/src/lib.rs
@@ -2,7 +2,7 @@
 //!
 //! [CMSIS-SVD]: http://www.keil.com/pack/doc/CMSIS/SVD/html/index.html
 //!
-//! A SVD file is an XML file that describes the hardware features of a
+//! An SVD file is an XML file that describes the hardware features of a
 //! microcontroller. In particular, it lists all the peripherals available to the
 //! device, where the registers associated to each device are located in memory,
 //! and what's the function of each register.
@@ -56,20 +56,23 @@
 //! $ cargo fmt
 //! ```
 //!
-//! The resulting crate must provide an opt-in "rt" feature and depend on these crates:
-//! `cortex-m` v0.7, `cortex-m-rt` >=v0.6.13 and `vcell` >=v0.1.2. Furthermore
-//! the "device" feature of `cortex-m-rt` must be enabled when the "rt" feature is enabled. The
-//! `Cargo.toml` of the device crate will look like this:
+//! The resulting crate must provide an opt-in `rt` feature and depend on these crates:
+//!
+//! - [`critical-section`](https://crates.io/crates/critical-section) v1.x
+//! - [`cortex-m`](https://crates.io/crates/cortex-m) >=v0.7.6
+//! - [`cortex-m-rt`](https://crates.io/crates/cortex-m-rt) >=v0.6.13
+//! - [`vcell`](https://crates.io/crates/vcell) >=v0.1.2
+//!
+//! Furthermore, the "device" feature of `cortex-m-rt` must be enabled when the `rt` feature
+//! is enabled. The `Cargo.toml` of the device crate will look like this:
 //!
 //! ``` toml
 //! [dependencies]
-//! cortex-m = "0.7"
+//! critical-section = { version = "1.0", optional = true }
+//! cortex-m = "0.7.6"
+//! cortex-m-rt = { version = "0.6.13", optional = true }
 //! vcell = "0.1.2"
 //!
-//! [dependencies.cortex-m-rt]
-//! optional = true
-//! version = "0.6.13"
-//!
 //! [features]
 //! rt = ["cortex-m-rt/device"]
 //! ```
@@ -110,21 +113,24 @@
 //! $ cargo fmt
 //! ```
 //!
-//! The resulting crate must provide opt-in "rt" feature and depend on these crates: `msp430`
-//! v0.3.x, `msp430-rt` v0.3.x, and `vcell` v0.1.x. If the `--nightly` flag is provided to
-//! `svd2rust`, then `msp430-atomic` v0.1.4 is also needed. Furthermore the "device" feature of
-//! `msp430-rt` must be enabled when the "rt" feature is enabled. The `Cargo.toml` of the device
-//! crate will look like this:
+//! The resulting crate must provide opt-in `rt` feature and depend on these crates:
+//!
+//! - [`critical-section`](https://crates.io/crates/critical-section) v1.x
+//! - [`msp430`](https://crates.io/crates/msp430) v0.3.x
+//! - [`msp430-rt`](https://crates.io/crates/msp430-rt) v0.3.x
+//! - [`vcell`](https://crates.io/crates/vcell) v0.1.x
+//!
+//! If the `--nightly` flag is provided to `svd2rust`, then `msp430-atomic` v0.1.4 is also needed.
+//! Furthermore the "device" feature of `msp430-rt` must be enabled when the `rt` feature is
+//! enabled. The `Cargo.toml` of the device crate will look like this:
 //!
 //! ``` toml
 //! [dependencies]
+//! critical-section = { version = "1.0", optional = true }
 //! msp430 = "0.3.0"
-//! vcell = "0.1.0"
 //! msp430-atomic = "0.1.4" # Only when using the --nightly flag
-//!
-//! [dependencies.msp430-rt]
-//! optional = true
-//! version = "0.3.0"
+//! msp430-rt = { version = "0.3.0", optional = true }
+//! vcell = "0.1.0"
 //!
 //! [features]
 //! rt = ["msp430-rt/device"]
@@ -134,28 +140,25 @@
 //! ## Other targets
 //!
 //! When the target is riscv or none `svd2rust` will emit only the `lib.rs` file. Like in
-//! the cortex-m case we recommend you use `form` and `rustfmt` on the output.
+//! the `cortex-m` case, we recommend you use `form` and `rustfmt` on the output.
 //!
-//! The resulting crate must provide an opt-in "rt" feature and depend on these crates:
+//! The resulting crate must provide an opt-in `rt` feature and depend on these crates:
 //!
-//! - [`bare-metal`](https://crates.io/crates/bare-metal) v0.2.x
+//! - [`critical-section`](https://crates.io/crates/critical-section) v1.x
+//! - [`riscv`](https://crates.io/crates/riscv) v0.9.x (if target is RISC-V)
+//! - [`riscv-rt`](https://crates.io/crates/riscv-rt) v0.9.x (if target is RISC-V)
 //! - [`vcell`](https://crates.io/crates/vcell) v0.1.x
-//! - [`riscv`](https://crates.io/crates/riscv) v0.4.x if target = riscv.
-//! - [`riscv-rt`](https://crates.io/crates/riscv-rt) v0.4.x if target = riscv.
 //!
-//! The `*-rt` dependencies must be optional only enabled when the "rt" feature is enabled. The
-//! `Cargo.toml` of the device crate will look like this for a riscv target:
+//! The `*-rt` dependencies must be optional only enabled when the `rt` feature is enabled. The
+//! `Cargo.toml` of the device crate will look like this for a RISC-V target:
 //!
 //! ``` toml
 //! [dependencies]
-//! bare-metal = "0.2.0"
-//! riscv = "0.4.0"
+//! critical-section = { version = "1.0", optional = true }
+//! riscv = "0.9.0"
+//! riscv-rt = { version = "0.9.0", optional = true }
 //! vcell = "0.1.0"
 //!
-//! [dependencies.riscv-rt]
-//! optional = true
-//! version = "0.4.0"
-//!
 //! [features]
 //! rt = ["riscv-rt"]
 //! ```
@@ -458,7 +461,6 @@
 //! used with the `cortex-m` crate `NVIC` API.
 //!
 //! ```ignore
-//! use cortex_m::interrupt;
 //! use cortex_m::peripheral::Peripherals;
 //! use stm32f30x::Interrupt;
 //!
@@ -469,9 +471,9 @@
 //! nvic.enable(Interrupt::TIM3);
 //! ```
 //!
-//! ## the "rt" feature
+//! ## the `rt` feature
 //!
-//! If the "rt" Cargo feature of the svd2rust generated crate is enabled, the crate will populate the
+//! If the `rt` Cargo feature of the svd2rust generated crate is enabled, the crate will populate the
 //! part of the vector table that contains the interrupt vectors and provide an
 //! [`interrupt!`](macro.interrupt.html) macro (non Cortex-M/MSP430 targets) or [`interrupt`] attribute
 //! (Cortex-M or [MSP430](https://docs.rs/msp430-rt-macros/0.1/msp430_rt_macros/attr.interrupt.html))