From 5792daf3afb9366c362fc57c89870ffb05df8b8c Mon Sep 17 00:00:00 2001 From: Bjorn Beishline <75190918+BjornTheProgrammer@users.noreply.github.com> Date: Tue, 25 Nov 2025 11:28:55 -0700 Subject: Remove atomic-polyfill --- embassy-rp/Cargo.toml | 3 +-- embassy-rp/src/pio/mod.rs | 26 ++++++++++++++++++-------- embassy-rp/src/uart/buffered.rs | 13 ++++++++++--- embassy-rp/src/uart/mod.rs | 28 +++++++++++++++++++++++----- 4 files changed, 52 insertions(+), 18 deletions(-) diff --git a/embassy-rp/Cargo.toml b/embassy-rp/Cargo.toml index 9ad4b47a3..421f0b0f6 100644 --- a/embassy-rp/Cargo.toml +++ b/embassy-rp/Cargo.toml @@ -47,7 +47,7 @@ rt = [ "rp-pac/rt" ] defmt = ["dep:defmt", "embassy-usb-driver/defmt", "embassy-hal-internal/defmt"] ## Enable log support log = ["dep:log"] -## Enable chrono support +## Enable chrono support chrono = ["dep:chrono"] ## Configure the [`critical-section`](https://docs.rs/critical-section) crate to use an implementation that is safe for multicore use on rp2040. @@ -159,7 +159,6 @@ embassy-futures = { version = "0.1.2", path = "../embassy-futures" } embassy-hal-internal = { version = "0.3.0", path = "../embassy-hal-internal", features = ["cortex-m", "prio-bits-2"] } embassy-embedded-hal = { version = "0.5.0", path = "../embassy-embedded-hal" } embassy-usb-driver = { version = "0.2.0", path = "../embassy-usb-driver" } -atomic-polyfill = "1.0.1" defmt = { version = "1.0.1", optional = true } log = { version = "0.4.14", optional = true } nb = "1.1.0" diff --git a/embassy-rp/src/pio/mod.rs b/embassy-rp/src/pio/mod.rs index 92b2c603e..fd0b4c072 100644 --- a/embassy-rp/src/pio/mod.rs +++ b/embassy-rp/src/pio/mod.rs @@ -1,11 +1,11 @@ //! PIO driver. +use core::cell::Cell; use core::future::Future; use core::marker::PhantomData; use core::pin::Pin as FuturePin; -use core::sync::atomic::{Ordering, compiler_fence}; +use core::sync::atomic::{AtomicU8, Ordering, compiler_fence}; use core::task::{Context, Poll}; -use atomic_polyfill::{AtomicU8, AtomicU64}; use embassy_hal_internal::{Peri, PeripheralType}; use embassy_sync::waitqueue::AtomicWaker; use fixed::FixedU32; @@ -1232,7 +1232,10 @@ impl<'d, PIO: Instance> Common<'d, PIO> { w.set_pde(false); }); // we can be relaxed about this because we're &mut here and nothing is cached - PIO::state().used_pins.fetch_or(1 << pin.pin_bank(), Ordering::Relaxed); + critical_section::with(|_| { + let val = PIO::state().used_pins.get(); + PIO::state().used_pins.set(val | 1 << pin.pin_bank()); + }); Pin { pin: pin.into(), pio: PhantomData::default(), @@ -1369,7 +1372,7 @@ impl<'d, PIO: Instance> Pio<'d, PIO> { /// Create a new instance of a PIO peripheral. pub fn new(_pio: Peri<'d, PIO>, _irq: impl Binding>) -> Self { PIO::state().users.store(5, Ordering::Release); - PIO::state().used_pins.store(0, Ordering::Release); + critical_section::with(|_| PIO::state().used_pins.set(0)); PIO::Interrupt::unpend(); unsafe { PIO::Interrupt::enable() }; @@ -1413,13 +1416,20 @@ impl<'d, PIO: Instance> Pio<'d, PIO> { // other way. pub struct State { users: AtomicU8, - used_pins: AtomicU64, + used_pins: Cell, } +unsafe impl Sync for State {} + fn on_pio_drop() { let state = PIO::state(); - if state.users.fetch_sub(1, Ordering::AcqRel) == 1 { - let used_pins = state.used_pins.load(Ordering::Relaxed); + let users_state = critical_section::with(|_| { + let val = state.users.load(Ordering::Acquire) - 1; + state.users.store(val, Ordering::Release); + val + }); + if users_state == 1 { + let used_pins = critical_section::with(|_| state.used_pins.get()); let null = pac::io::vals::Gpio0ctrlFuncsel::NULL as _; for i in 0..crate::gpio::BANK0_PIN_COUNT { if used_pins & (1 << i) != 0 { @@ -1444,7 +1454,7 @@ trait SealedInstance { fn state() -> &'static State { static STATE: State = State { users: AtomicU8::new(0), - used_pins: AtomicU64::new(0), + used_pins: Cell::new(0), }; &STATE diff --git a/embassy-rp/src/uart/buffered.rs b/embassy-rp/src/uart/buffered.rs index 02649ad81..fdb8ce776 100644 --- a/embassy-rp/src/uart/buffered.rs +++ b/embassy-rp/src/uart/buffered.rs @@ -1,8 +1,8 @@ //! Buffered UART driver. use core::future::Future; use core::slice; +use core::sync::atomic::{AtomicU8, Ordering}; -use atomic_polyfill::AtomicU8; use embassy_hal_internal::atomic_ring_buffer::RingBuffer; use super::*; @@ -241,7 +241,11 @@ impl BufferedUartRx { } fn get_rx_error(state: &State) -> Option { - let errs = state.rx_error.swap(0, Ordering::Relaxed); + let errs = critical_section::with(|_| { + let val = state.rx_error.load(Ordering::Relaxed); + state.rx_error.store(0, Ordering::Relaxed); + val + }); if errs & RXE_OVERRUN != 0 { Some(Error::Overrun) } else if errs & RXE_BREAK != 0 { @@ -555,7 +559,10 @@ impl interrupt::typelevel::Handler for BufferedInterr } let dr = r.uartdr().read(); if (dr.0 >> 8) != 0 { - s.rx_error.fetch_or((dr.0 >> 8) as u8, Ordering::Relaxed); + critical_section::with(|_| { + let val = s.rx_error.load(Ordering::Relaxed); + s.rx_error.store(val | ((dr.0 >> 8) as u8), Ordering::Relaxed); + }); error = true; // only fill the buffer with valid characters. the current character is fine // if the error is an overrun, but if we add it to the buffer we'll report diff --git a/embassy-rp/src/uart/mod.rs b/embassy-rp/src/uart/mod.rs index 8be87a5d2..b7b569dd5 100644 --- a/embassy-rp/src/uart/mod.rs +++ b/embassy-rp/src/uart/mod.rs @@ -1,9 +1,9 @@ //! UART driver. use core::future::poll_fn; use core::marker::PhantomData; +use core::sync::atomic::{AtomicU16, Ordering}; use core::task::Poll; -use atomic_polyfill::{AtomicU16, Ordering}; use embassy_futures::select::{Either, select}; use embassy_hal_internal::{Peri, PeripheralType}; use embassy_sync::waitqueue::AtomicWaker; @@ -456,7 +456,12 @@ impl<'d> UartRx<'d, Async> { transfer, poll_fn(|cx| { self.dma_state.rx_err_waker.register(cx.waker()); - match self.dma_state.rx_errs.swap(0, Ordering::Relaxed) { + let rx_errs = critical_section::with(|_| { + let val = self.dma_state.rx_errs.load(Ordering::Relaxed); + self.dma_state.rx_errs.store(0, Ordering::Relaxed); + val + }); + match rx_errs { 0 => Poll::Pending, e => Poll::Ready(Uartris(e as u32)), } @@ -468,7 +473,11 @@ impl<'d> UartRx<'d, Async> { Either::First(()) => { // We're here because the DMA finished, BUT if an error occurred on the LAST // byte, then we may still need to grab the error state! - Uartris(self.dma_state.rx_errs.swap(0, Ordering::Relaxed) as u32) + Uartris(critical_section::with(|_| { + let val = self.dma_state.rx_errs.load(Ordering::Relaxed); + self.dma_state.rx_errs.store(0, Ordering::Relaxed); + val + }) as u32) } Either::Second(e) => { // We're here because we errored, which means this is the error that @@ -616,7 +625,12 @@ impl<'d> UartRx<'d, Async> { transfer, poll_fn(|cx| { self.dma_state.rx_err_waker.register(cx.waker()); - match self.dma_state.rx_errs.swap(0, Ordering::Relaxed) { + let rx_errs = critical_section::with(|_| { + let val = self.dma_state.rx_errs.load(Ordering::Relaxed); + self.dma_state.rx_errs.store(0, Ordering::Relaxed); + val + }); + match rx_errs { 0 => Poll::Pending, e => Poll::Ready(Uartris(e as u32)), } @@ -629,7 +643,11 @@ impl<'d> UartRx<'d, Async> { Either::First(()) => { // We're here because the DMA finished, BUT if an error occurred on the LAST // byte, then we may still need to grab the error state! - Uartris(self.dma_state.rx_errs.swap(0, Ordering::Relaxed) as u32) + Uartris(critical_section::with(|_| { + let val = self.dma_state.rx_errs.load(Ordering::Relaxed); + self.dma_state.rx_errs.store(0, Ordering::Relaxed); + val + }) as u32) } Either::Second(e) => { // We're here because we errored, which means this is the error that -- cgit From f8ec795741663f559295327911a51559c526ba8f Mon Sep 17 00:00:00 2001 From: Bjorn Beishline <75190918+BjornTheProgrammer@users.noreply.github.com> Date: Tue, 25 Nov 2025 11:37:22 -0700 Subject: Update CHANGELOG.md --- embassy-rp/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/embassy-rp/CHANGELOG.md b/embassy-rp/CHANGELOG.md index fa8609bbf..7480b729f 100644 --- a/embassy-rp/CHANGELOG.md +++ b/embassy-rp/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add fix #4822 in PIO onewire. Change to disable the state machine before setting y register ([#4824](https://github.com/embassy-rs/embassy/pull/4824)) - Add PIO::Ws2812 color order support - Add TX-only, no SCK SPI support +- Remove atomic-polyfill with critical-section instead ([#4948](https://github.com/embassy-rs/embassy/pull/4948)) ## 0.8.0 - 2025-08-26 @@ -116,4 +117,3 @@ Small release fixing a few gnarly bugs, upgrading is strongly recommended. - rename the Channel trait to Slice and the PwmPin to PwmChannel - i2c: Fix race condition that appears on fast repeated transfers. - Add a basic "read to break" function - -- cgit From edd8878f8cbd15a56a6c845a2a8772a016e24d4b Mon Sep 17 00:00:00 2001 From: Bjorn Beishline <75190918+BjornTheProgrammer@users.noreply.github.com> Date: Tue, 25 Nov 2025 12:05:52 -0700 Subject: Use two AtomicU32 instead --- embassy-rp/src/pio/mod.rs | 56 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 10 deletions(-) diff --git a/embassy-rp/src/pio/mod.rs b/embassy-rp/src/pio/mod.rs index fd0b4c072..1c370fdfc 100644 --- a/embassy-rp/src/pio/mod.rs +++ b/embassy-rp/src/pio/mod.rs @@ -1,9 +1,8 @@ //! PIO driver. -use core::cell::Cell; use core::future::Future; use core::marker::PhantomData; use core::pin::Pin as FuturePin; -use core::sync::atomic::{AtomicU8, Ordering, compiler_fence}; +use core::sync::atomic::{AtomicU8, AtomicU32, Ordering, compiler_fence}; use core::task::{Context, Poll}; use embassy_hal_internal::{Peri, PeripheralType}; @@ -1233,9 +1232,12 @@ impl<'d, PIO: Instance> Common<'d, PIO> { }); // we can be relaxed about this because we're &mut here and nothing is cached critical_section::with(|_| { - let val = PIO::state().used_pins.get(); - PIO::state().used_pins.set(val | 1 << pin.pin_bank()); + let val = PIO::state().used_pins.load(Ordering::Relaxed); + PIO::state() + .used_pins + .store(val | 1 << pin.pin_bank(), Ordering::Relaxed); }); + Pin { pin: pin.into(), pio: PhantomData::default(), @@ -1372,7 +1374,7 @@ impl<'d, PIO: Instance> Pio<'d, PIO> { /// Create a new instance of a PIO peripheral. pub fn new(_pio: Peri<'d, PIO>, _irq: impl Binding>) -> Self { PIO::state().users.store(5, Ordering::Release); - critical_section::with(|_| PIO::state().used_pins.set(0)); + PIO::state().used_pins.store(0, Ordering::Release); PIO::Interrupt::unpend(); unsafe { PIO::Interrupt::enable() }; @@ -1407,6 +1409,42 @@ impl<'d, PIO: Instance> Pio<'d, PIO> { } } +struct AtomicU64 { + upper_32: AtomicU32, + lower_32: AtomicU32, +} + +impl AtomicU64 { + const fn new(val: u64) -> Self { + let upper_32 = (val >> 32) as u32; + let lower_32 = val as u32; + + Self { + upper_32: AtomicU32::new(upper_32), + lower_32: AtomicU32::new(lower_32), + } + } + + fn load(&self, order: Ordering) -> u64 { + let (upper, lower) = critical_section::with(|_| (self.upper_32.load(order), self.lower_32.load(order))); + + let upper = (upper as u64) << 32; + let lower = lower as u64; + + upper | lower + } + + fn store(&self, val: u64, order: Ordering) { + let upper_32 = (val >> 32) as u32; + let lower_32 = val as u32; + + critical_section::with(|_| { + self.upper_32.store(upper_32, order); + self.lower_32.store(lower_32, order); + }); + } +} + /// Representation of the PIO state keeping a record of which pins are assigned to /// each PIO. // make_pio_pin notionally takes ownership of the pin it is given, but the wrapped pin @@ -1416,11 +1454,9 @@ impl<'d, PIO: Instance> Pio<'d, PIO> { // other way. pub struct State { users: AtomicU8, - used_pins: Cell, + used_pins: AtomicU64, } -unsafe impl Sync for State {} - fn on_pio_drop() { let state = PIO::state(); let users_state = critical_section::with(|_| { @@ -1429,7 +1465,7 @@ fn on_pio_drop() { val }); if users_state == 1 { - let used_pins = critical_section::with(|_| state.used_pins.get()); + let used_pins = state.used_pins.load(Ordering::Relaxed); let null = pac::io::vals::Gpio0ctrlFuncsel::NULL as _; for i in 0..crate::gpio::BANK0_PIN_COUNT { if used_pins & (1 << i) != 0 { @@ -1454,7 +1490,7 @@ trait SealedInstance { fn state() -> &'static State { static STATE: State = State { users: AtomicU8::new(0), - used_pins: Cell::new(0), + used_pins: AtomicU64::new(0), }; &STATE -- cgit