From b4d0d613e3d641f400a1c71278339621c2920cc9 Mon Sep 17 00:00:00 2001 From: Jan Niehusmann Date: Thu, 28 Jul 2022 13:10:08 +0000 Subject: [PATCH 1/2] Update to critical-section 1.0.0 --- rp2040-hal/CHANGELOG.md | 4 ++++ rp2040-hal/Cargo.toml | 5 +++-- rp2040-hal/README.md | 5 +++++ rp2040-hal/src/critical_section_impl.rs | 23 ++++++++++++++++++++++- rp2040-hal/src/usb.rs | 22 +++++++++++----------- 5 files changed, 45 insertions(+), 14 deletions(-) diff --git a/rp2040-hal/CHANGELOG.md b/rp2040-hal/CHANGELOG.md index 7ca7162..0e09b66 100644 --- a/rp2040-hal/CHANGELOG.md +++ b/rp2040-hal/CHANGELOG.md @@ -10,6 +10,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - `rp2040-e5` feature enabling the workaround for errata 5 on the USB device peripheral. +- Support for critical-section 1.0.0. + Critical-section 0.2 is still supported (ie. a custom-impl is provided, compatible + with the 1.0.0 implementation), to avoid a breaking change. It will be removed + later. ### Changed diff --git a/rp2040-hal/Cargo.toml b/rp2040-hal/Cargo.toml index 30d5503..65fba4c 100644 --- a/rp2040-hal/Cargo.toml +++ b/rp2040-hal/Cargo.toml @@ -13,7 +13,7 @@ license = "MIT OR Apache-2.0" cortex-m = "0.7.2" cortex-m-rt = ">=0.6.15,<0.8" embedded-hal = { version = "0.2.5", features = ["unproven"] } -eh1_0_alpha = { version = "=1.0.0-alpha.8", package="embedded-hal", optional=true } +eh1_0_alpha = { package = "embedded-hal", version = "=1.0.0-alpha.8", optional = true } embedded-time = "0.12.0" itertools = { version = "0.10.1", default-features = false } nb = "1.0" @@ -25,7 +25,8 @@ usb-device = "0.2.9" vcell = "0.1" void = { version = "1.0.2", default-features = false } rand_core = "0.6.3" -critical-section = { version = "0.2.4", features = ["custom-impl"] } +critical-section_0_2 = { package = "critical-section", version = "0.2.4", features = ["custom-impl"] } +critical-section = { version = "1.0.0", features = ["restore-state-u8"] } futures = { version = "0.3", default-features = false, optional = true } chrono = { version = "0.4", default-features = false, optional = true } diff --git a/rp2040-hal/README.md b/rp2040-hal/README.md index 6f5db1d..2299492 100644 --- a/rp2040-hal/README.md +++ b/rp2040-hal/README.md @@ -105,6 +105,11 @@ at the same time. The new blocking [SPI traits](https://docs.rs/embedded-hal/1.0.0-alpha.8/embedded_hal/spi/blocking/index.html) are not yet implemented. +### Support for critical-section 0.2 + +While `rp2040-hal` uses critical-section 1.0, it still provides support for version 0.2. +That version is deprecated, and support will be removed in a future release. Please upgrade. + ## Contributing diff --git a/rp2040-hal/src/critical_section_impl.rs b/rp2040-hal/src/critical_section_impl.rs index e26cfd9..3eaa241 100644 --- a/rp2040-hal/src/critical_section_impl.rs +++ b/rp2040-hal/src/critical_section_impl.rs @@ -1,7 +1,8 @@ use core::sync::atomic::{AtomicU8, Ordering}; struct RpSpinlockCs; -critical_section::custom_impl!(RpSpinlockCs); +critical_section_0_2::custom_impl!(RpSpinlockCs); +critical_section::set_impl!(RpSpinlockCs); /// Marker value to indicate no-one has the lock. /// @@ -21,7 +22,27 @@ static mut LOCK_OWNER: AtomicU8 = AtomicU8::new(LOCK_UNOWNED); /// The value 2 indicates that we aren't the outermost call, and should not release the spinlock or re-enable interrupts in `release` const LOCK_ALREADY_OWNED: u8 = 2; +unsafe impl critical_section_0_2::Impl for RpSpinlockCs { + unsafe fn acquire() -> u8 { + RpSpinlockCs::acquire() + } + + unsafe fn release(token: u8) { + RpSpinlockCs::release(token); + } +} + unsafe impl critical_section::Impl for RpSpinlockCs { + unsafe fn acquire() -> u8 { + RpSpinlockCs::acquire() + } + + unsafe fn release(token: u8) { + RpSpinlockCs::release(token); + } +} + +impl RpSpinlockCs { unsafe fn acquire() -> u8 { // Store the initial interrupt state and current core id in stack variables let interrupts_active = cortex_m::register::primask::read().is_active(); diff --git a/rp2040-hal/src/usb.rs b/rp2040-hal/src/usb.rs index 9c4b9ed..0609695 100644 --- a/rp2040-hal/src/usb.rs +++ b/rp2040-hal/src/usb.rs @@ -113,7 +113,7 @@ use crate::pac::USBCTRL_DPRAM; use crate::pac::USBCTRL_REGS; use crate::resets::SubsystemReset; -use cortex_m::interrupt::{self, Mutex}; +use critical_section::{self, Mutex}; use usb_device::{ bus::{PollResult, UsbBus as UsbBusTrait}, @@ -471,7 +471,7 @@ impl UsbBus { /// Generates a resume request on the bus. pub fn remote_wakeup(&self) { - interrupt::free(|cs| { + critical_section::with(|cs| { let inner = self.inner.borrow(cs).borrow_mut(); inner.ctrl_reg.sie_ctrl.modify(|_, w| w.resume().set_bit()); }); @@ -487,7 +487,7 @@ impl UsbBusTrait for UsbBus { max_packet_size: u16, _interval: u8, ) -> UsbResult { - interrupt::free(|cs| { + critical_section::with(|cs| { let mut inner = self.inner.borrow(cs).borrow_mut(); inner.ep_allocate(ep_addr, ep_dir, ep_type, max_packet_size) @@ -495,7 +495,7 @@ impl UsbBusTrait for UsbBus { } fn enable(&mut self) { - interrupt::free(|cs| { + critical_section::with(|cs| { let inner = self.inner.borrow(cs).borrow_mut(); // at this stage ep's are expected to be in their reset state // TODO: is it worth having a debug_assert for that here? @@ -524,7 +524,7 @@ impl UsbBusTrait for UsbBus { }) } fn reset(&self) { - interrupt::free(|cs| { + critical_section::with(|cs| { let mut inner = self.inner.borrow(cs).borrow_mut(); // clear reset flag @@ -544,7 +544,7 @@ impl UsbBusTrait for UsbBus { }) } fn set_device_address(&self, addr: u8) { - interrupt::free(|cs| { + critical_section::with(|cs| { let inner = self.inner.borrow(cs).borrow_mut(); inner .ctrl_reg @@ -556,19 +556,19 @@ impl UsbBusTrait for UsbBus { }) } fn write(&self, ep_addr: EndpointAddress, buf: &[u8]) -> UsbResult { - interrupt::free(|cs| { + critical_section::with(|cs| { let mut inner = self.inner.borrow(cs).borrow_mut(); inner.ep_write(ep_addr, buf) }) } fn read(&self, ep_addr: EndpointAddress, buf: &mut [u8]) -> UsbResult { - interrupt::free(|cs| { + critical_section::with(|cs| { let mut inner = self.inner.borrow(cs).borrow_mut(); inner.ep_read(ep_addr, buf) }) } fn set_stalled(&self, ep_addr: EndpointAddress, stalled: bool) { - interrupt::free(|cs| { + critical_section::with(|cs| { let inner = self.inner.borrow(cs).borrow_mut(); if ep_addr.index() == 0 { @@ -586,7 +586,7 @@ impl UsbBusTrait for UsbBus { }) } fn is_stalled(&self, ep_addr: EndpointAddress) -> bool { - interrupt::free(|cs| { + critical_section::with(|cs| { let inner = self.inner.borrow(cs).borrow_mut(); let index = ep_addr_to_ep_buf_ctrl_idx(ep_addr); inner.ctrl_dpram.ep_buffer_control[index] @@ -598,7 +598,7 @@ impl UsbBusTrait for UsbBus { fn suspend(&self) {} fn resume(&self) {} fn poll(&self) -> PollResult { - interrupt::free(|cs| { + critical_section::with(|cs| { let mut inner = self.inner.borrow(cs).borrow_mut(); #[cfg(feature = "rp2040-e5")] From c1c5e05989f3fbf676d6f149e806d535d64a273d Mon Sep 17 00:00:00 2001 From: Jan Niehusmann Date: Wed, 10 Aug 2022 17:33:07 +0000 Subject: [PATCH 2/2] Feature-gate critical-section-impl For compatibility with earlier versions, this feature is activated by default. This decision can be revisited later. --- rp2040-hal/Cargo.toml | 6 +++++- rp2040-hal/src/critical_section_impl.rs | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/rp2040-hal/Cargo.toml b/rp2040-hal/Cargo.toml index 65fba4c..a5ed3e2 100644 --- a/rp2040-hal/Cargo.toml +++ b/rp2040-hal/Cargo.toml @@ -25,8 +25,10 @@ usb-device = "0.2.9" vcell = "0.1" void = { version = "1.0.2", default-features = false } rand_core = "0.6.3" +# Always set the custom-impl feature of legacy critical-section, even if we don't provide our own, +# as the default implementation should no longer be used. critical-section_0_2 = { package = "critical-section", version = "0.2.4", features = ["custom-impl"] } -critical-section = { version = "1.0.0", features = ["restore-state-u8"] } +critical-section = { version = "1.0.0" } futures = { version = "0.3", default-features = false, optional = true } chrono = { version = "0.4", default-features = false, optional = true } @@ -42,11 +44,13 @@ pio-proc = "0.2.0" dht-sensor = "0.2.1" [features] +default = ["critical-section-impl"] rt = ["rp2040-pac/rt"] rom-func-cache = [] disable-intrinsics = [] rom-v2-intrinsics = [] rp2040-e5 = [] # USB errata 5: USB device fails to exit RESET state on busy USB bus. +critical-section-impl = ["critical-section/restore-state-u8"] [[example]] # irq example uses cortex-m-rt::interrupt, need rt feature for that diff --git a/rp2040-hal/src/critical_section_impl.rs b/rp2040-hal/src/critical_section_impl.rs index 3eaa241..a4c259f 100644 --- a/rp2040-hal/src/critical_section_impl.rs +++ b/rp2040-hal/src/critical_section_impl.rs @@ -1,7 +1,9 @@ use core::sync::atomic::{AtomicU8, Ordering}; struct RpSpinlockCs; +#[cfg(feature = "critical-section-impl")] critical_section_0_2::custom_impl!(RpSpinlockCs); +#[cfg(feature = "critical-section-impl")] critical_section::set_impl!(RpSpinlockCs); /// Marker value to indicate no-one has the lock. @@ -22,6 +24,7 @@ static mut LOCK_OWNER: AtomicU8 = AtomicU8::new(LOCK_UNOWNED); /// The value 2 indicates that we aren't the outermost call, and should not release the spinlock or re-enable interrupts in `release` const LOCK_ALREADY_OWNED: u8 = 2; +#[cfg(feature = "critical-section-impl")] unsafe impl critical_section_0_2::Impl for RpSpinlockCs { unsafe fn acquire() -> u8 { RpSpinlockCs::acquire() @@ -32,6 +35,7 @@ unsafe impl critical_section_0_2::Impl for RpSpinlockCs { } } +#[cfg(feature = "critical-section-impl")] unsafe impl critical_section::Impl for RpSpinlockCs { unsafe fn acquire() -> u8 { RpSpinlockCs::acquire()