From 33bdc9e85ff391da4ff4ff439abe86c2bae32986 Mon Sep 17 00:00:00 2001
From: Dario Nieuwenhuis <dirbaio@dirbaio.net>
Date: Fri, 24 May 2024 20:04:14 +0200
Subject: [PATCH 1/2] rp: fix spinlocks staying locked after reset.
Fixes #1736
---
embassy-rp/src/lib.rs | 44 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/embassy-rp/src/lib.rs b/embassy-rp/src/lib.rs
index 1c83e306d..4602e66f4 100644
--- a/embassy-rp/src/lib.rs
+++ b/embassy-rp/src/lib.rs
@@ -352,6 +352,50 @@ pub fn init(config: config::Config) -> Peripherals {
peripherals
}
+#[cfg(feature = "rt")]
+#[cortex_m_rt::pre_init]
+unsafe fn pre_init() {
+ // SIO does not get reset when core0 is reset with either `scb::sys_reset()` or with SWD.
+ // Since we're using SIO spinlock 31 for the critical-section impl, this causes random
+ // hangs if we reset in the middle of a CS, because the next boot sees the spinlock
+ // as locked and waits forever.
+ //
+ // See https://github.com/embassy-rs/embassy/issues/1736
+ // and https://github.com/rp-rs/rp-hal/issues/292
+ // and https://matrix.to/#/!vhKMWjizPZBgKeknOo:matrix.org/$VfOkQgyf1PjmaXZbtycFzrCje1RorAXd8BQFHTl4d5M
+ //
+ // According to Raspberry Pi, this is considered Working As Intended, and not an errata,
+ // even though this behavior is different from every other ARM chip (sys_reset usually resets
+ // the *system* as its name implies, not just the current core).
+ //
+ // To fix this, reset SIO on boot. We must do this in pre_init because it's unsound to do it
+ // in `embassy_rp::init`, since the user could've acquired a CS by then. pre_init is guaranteed
+ // to run before any user code.
+ //
+ // A similar thing could happen with PROC1. It is unclear whether it's possible for PROC1
+ // to stay unreset through a PROC0 reset, so we reset it anyway just in case.
+ //
+ // Important info from PSM logic (from Luke Wren in above Matrix thread)
+ //
+ // The logic is, each PSM stage is reset if either of the following is true:
+ // - The previous stage is in reset and FRCE_ON is false
+ // - FRCE_OFF is true
+ //
+ // The PSM order is SIO -> PROC0 -> PROC1.
+ // So, we have to force-on PROC0 to prevent it from getting reset when resetting SIO.
+ pac::PSM.frce_on().write(|w| {
+ w.set_proc0(true);
+ });
+ // Then reset SIO and PROC1.
+ pac::PSM.frce_off().write(|w| {
+ w.set_sio(true);
+ w.set_proc1(true);
+ });
+ // clear force_off first, force_on second. The other way around would reset PROC0.
+ pac::PSM.frce_off().write(|_| {});
+ pac::PSM.frce_on().write(|_| {});
+}
+
/// Extension trait for PAC regs, adding atomic xor/bitset/bitclear writes.
trait RegExt<T: Copy> {
#[allow(unused)]
From d18a919ab9cfa1c07d339dd885d8268ab0abb7e6 Mon Sep 17 00:00:00 2001
From: Dario Nieuwenhuis <dirbaio@dirbaio.net>
Date: Mon, 27 May 2024 00:11:13 +0200
Subject: [PATCH 2/2] rp: wait until read matches for PSM accesses.
---
embassy-rp/src/lib.rs | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/embassy-rp/src/lib.rs b/embassy-rp/src/lib.rs
index 4602e66f4..507d42280 100644
--- a/embassy-rp/src/lib.rs
+++ b/embassy-rp/src/lib.rs
@@ -383,25 +383,29 @@ unsafe fn pre_init() {
//
// The PSM order is SIO -> PROC0 -> PROC1.
// So, we have to force-on PROC0 to prevent it from getting reset when resetting SIO.
- pac::PSM.frce_on().write(|w| {
+ pac::PSM.frce_on().write_and_wait(|w| {
w.set_proc0(true);
});
// Then reset SIO and PROC1.
- pac::PSM.frce_off().write(|w| {
+ pac::PSM.frce_off().write_and_wait(|w| {
w.set_sio(true);
w.set_proc1(true);
});
// clear force_off first, force_on second. The other way around would reset PROC0.
- pac::PSM.frce_off().write(|_| {});
- pac::PSM.frce_on().write(|_| {});
+ pac::PSM.frce_off().write_and_wait(|_| {});
+ pac::PSM.frce_on().write_and_wait(|_| {});
}
/// Extension trait for PAC regs, adding atomic xor/bitset/bitclear writes.
+#[allow(unused)]
trait RegExt<T: Copy> {
#[allow(unused)]
fn write_xor<R>(&self, f: impl FnOnce(&mut T) -> R) -> R;
fn write_set<R>(&self, f: impl FnOnce(&mut T) -> R) -> R;
fn write_clear<R>(&self, f: impl FnOnce(&mut T) -> R) -> R;
+ fn write_and_wait<R>(&self, f: impl FnOnce(&mut T) -> R) -> R
+ where
+ T: PartialEq;
}
impl<T: Default + Copy, A: pac::common::Write> RegExt<T> for pac::common::Reg<T, A> {
@@ -434,4 +438,17 @@ impl<T: Default + Copy, A: pac::common::Write> RegExt<T> for pac::common::Reg<T,
}
res
}
+
+ fn write_and_wait<R>(&self, f: impl FnOnce(&mut T) -> R) -> R
+ where
+ T: PartialEq,
+ {
+ let mut val = Default::default();
+ let res = f(&mut val);
+ unsafe {
+ self.as_ptr().write_volatile(val);
+ while self.as_ptr().read_volatile() != val {}
+ }
+ res
+ }
}