From 17ebad4a3c8f8d5af909da9b4cc541478680739e Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 28 Jan 2021 16:02:53 +0100 Subject: [PATCH 1/2] adapt to breaking changes in cortex-m-rtic 0.5.x at some point cortex-m-rtic fixed a memory safety bug: RTIC was not marking the cortex_m::Peripherals singleton as "taken" in its pre-init code. This made it possible to create two instances of the singleton, e.g. by calling `cortex_m::Peripherals::take` in an RTIC app. the fix included a breaking change: it makes `Peripherals::take` return `None` from within an RTIC app. This breaks the use of `dk::init`, which `take`s those `Peripherals`, within an RTIC app and thus breaks most of the exercises / examples in the advanced workshop this PR fixes the issue by making `dk::init` not `take` the `Peripherals`. For more details read in-line comments in the diff --- boards/dk/src/lib.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/boards/dk/src/lib.rs b/boards/dk/src/lib.rs index 2da358c..7b129fa 100644 --- a/boards/dk/src/lib.rs +++ b/boards/dk/src/lib.rs @@ -10,7 +10,7 @@ use core::{ time::Duration, }; -use cortex_m::asm; +use cortex_m::{asm, peripheral::NVIC}; use embedded_hal::digital::v2::{OutputPin as _, StatefulOutputPin}; #[cfg(feature = "beginner")] pub use hal::ieee802154; @@ -176,10 +176,7 @@ impl ops::DerefMut for Timer { /// /// This return an `Err`or if called more than once pub fn init() -> Result { - if let (Some(mut core), Some(periph)) = ( - cortex_m::Peripherals::take(), - hal::target::Peripherals::take(), - ) { + if let Some(periph) = hal::target::Peripherals::take() { // NOTE(static mut) this branch runs at most once #[cfg(feature = "advanced")] static mut EP0IN_BUF: [u8; 64] = [0; 64]; @@ -216,8 +213,21 @@ pub fn init() -> Result { log::debug!("Clocks configured"); let mut rtc = Rtc::new(periph.RTC0); - rtc.enable_interrupt(RtcInterrupt::Overflow, Some(&mut core.NVIC)); + rtc.enable_interrupt(RtcInterrupt::Overflow, None); rtc.enable_counter(); + // NOTE(unsafe) because this crate defines the `#[interrupt] fn RTC0` interrupt handler, + // RTIC cannot manage that interrupt (trying to do so results in a linker error). Thus it + // is the task of this crate to mask/unmask the interrupt in a safe manner. + // + // Because the RTC0 interrupt handler does *not* access static variables through a critical + // section (that disables interrupts) this `unmask` operation cannot break critical sections + // and thus won't lead to undefined behavior (e.g. torn reads/writes) + // + // the preceding `enable_conuter` method consumes the `rtc` value. This is a semantic move + // of the RTC0 peripheral from this function (which can only be called at most once) to the + // interrupt handler (where the peripheral is accessed without any synchronization + // mechanism) + unsafe { NVIC::unmask(Interrupt::RTC0) }; log::debug!("RTC started"); @@ -291,7 +301,7 @@ impl Log for Logger { // Counter of OVERFLOW events -- an OVERFLOW occurs every (1<<24) ticks static OVERFLOWS: AtomicU32 = AtomicU32::new(0); -// NOTE this will at the highest priority, higher priority than RTIC tasks +// NOTE this will run at the highest priority, higher priority than RTIC tasks #[interrupt] fn RTC0() { let curr = OVERFLOWS.load(Ordering::Relaxed); From 450b3b77a9400f435cfc8fca7e7db2f909b1fef7 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Thu, 28 Jan 2021 16:14:44 +0100 Subject: [PATCH 2/2] make rustfmt happy --- boards/dk/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/boards/dk/src/lib.rs b/boards/dk/src/lib.rs index 7b129fa..f93f641 100644 --- a/boards/dk/src/lib.rs +++ b/boards/dk/src/lib.rs @@ -176,7 +176,7 @@ impl ops::DerefMut for Timer { /// /// This return an `Err`or if called more than once pub fn init() -> Result { - if let Some(periph) = hal::target::Peripherals::take() { + if let Some(periph) = hal::target::Peripherals::take() { // NOTE(static mut) this branch runs at most once #[cfg(feature = "advanced")] static mut EP0IN_BUF: [u8; 64] = [0; 64]; @@ -226,7 +226,7 @@ pub fn init() -> Result { // the preceding `enable_conuter` method consumes the `rtc` value. This is a semantic move // of the RTC0 peripheral from this function (which can only be called at most once) to the // interrupt handler (where the peripheral is accessed without any synchronization - // mechanism) + // mechanism) unsafe { NVIC::unmask(Interrupt::RTC0) }; log::debug!("RTC started");