Skip to content

Commit

Permalink
bus: make AtomicDevice use a per-bus "busy" flag, not per-device.
Browse files Browse the repository at this point in the history
Needed for soundness.
  • Loading branch information
Dirbaio committed Apr 23, 2024
1 parent d02221f commit aafb7bd
Showing 1 changed file with 32 additions and 16 deletions.
48 changes: 32 additions & 16 deletions embedded-hal-bus/src/spi/atomic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,28 @@ use embedded_hal::spi::{Error, ErrorKind, ErrorType, Operation, SpiBus, SpiDevic
use super::DeviceError;
use crate::spi::shared::transaction;

/// Cell type used by [`AtomicDevice`].
///
/// To use [`AtomicDevice`], you must wrap the bus with this struct, and then
/// construct multiple [`AtomicDevice`] instances with references to it.
pub struct AtomicCell<BUS> {
bus: UnsafeCell<BUS>,
busy: portable_atomic::AtomicBool,
}

unsafe impl<BUS: Send> Send for AtomicCell<BUS> {}
unsafe impl<BUS: Send> Sync for AtomicCell<BUS> {}

impl<BUS> AtomicCell<BUS> {
/// Create a new `AtomicCell`
pub fn new(bus: BUS) -> Self {
Self {
bus: UnsafeCell::new(bus),
busy: portable_atomic::AtomicBool::from(false),
}
}
}

/// `UnsafeCell`-based shared bus [`SpiDevice`] implementation.
///
/// This allows for sharing an [`SpiBus`], obtaining multiple [`SpiDevice`] instances,
Expand All @@ -19,10 +41,9 @@ use crate::spi::shared::transaction;
/// rules, such as the RTIC framework.
///
pub struct AtomicDevice<'a, BUS, CS, D> {
bus: &'a UnsafeCell<BUS>,
bus: &'a AtomicCell<BUS>,
cs: CS,
delay: D,
busy: portable_atomic::AtomicBool,
}

#[derive(Debug, Copy, Clone)]
Expand All @@ -40,13 +61,8 @@ pub enum AtomicError<T: Error> {
impl<'a, BUS, CS, D> AtomicDevice<'a, BUS, CS, D> {
/// Create a new [`AtomicDevice`].
#[inline]
pub fn new(bus: &'a UnsafeCell<BUS>, cs: CS, delay: D) -> Self {
Self {
bus,
cs,
delay,
busy: portable_atomic::AtomicBool::from(false),
}
pub fn new(bus: &'a AtomicCell<BUS>, cs: CS, delay: D) -> Self {
Self { bus, cs, delay }
}
}

Expand All @@ -72,18 +88,15 @@ where
/// The returned device will panic if you try to execute a transaction
/// that contains any operations of type [`Operation::DelayNs`].
#[inline]
pub fn new_no_delay(bus: &'a UnsafeCell<BUS>, cs: CS) -> Self {
pub fn new_no_delay(bus: &'a AtomicCell<BUS>, cs: CS) -> Self {
Self {
bus,
cs,
delay: super::NoDelay,
busy: portable_atomic::AtomicBool::from(false),
}
}
}

unsafe impl<'a, BUS, CS, D> Send for AtomicDevice<'a, BUS, CS, D> {}

impl<T: Error> Error for AtomicError<T> {
fn kind(&self) -> ErrorKind {
match self {
Expand All @@ -109,7 +122,8 @@ where
{
#[inline]
fn transaction(&mut self, operations: &mut [Operation<'_, Word>]) -> Result<(), Self::Error> {
self.busy
self.bus
.busy
.compare_exchange(
false,
true,
Expand All @@ -118,11 +132,13 @@ where
)
.map_err(|_| AtomicError::Busy)?;

let bus = unsafe { &mut *self.bus.get() };
let bus = unsafe { &mut *self.bus.bus.get() };

let result = transaction(operations, bus, &mut self.delay, &mut self.cs);

self.busy.store(false, core::sync::atomic::Ordering::SeqCst);
self.bus
.busy
.store(false, core::sync::atomic::Ordering::SeqCst);

result.map_err(AtomicError::Other)
}
Expand Down

0 comments on commit aafb7bd

Please sign in to comment.