diff --git a/embedded-hal-bus/CHANGELOG.md b/embedded-hal-bus/CHANGELOG.md index 04de7aa69..55013fc08 100644 --- a/embedded-hal-bus/CHANGELOG.md +++ b/embedded-hal-bus/CHANGELOG.md @@ -7,7 +7,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] -No unreleased changes +### Added +- Added a new `AtomicDevice` for I2C and SPI to enable bus sharing across multiple contexts. ## [v0.1.0] - 2023-12-28 diff --git a/embedded-hal-bus/Cargo.toml b/embedded-hal-bus/Cargo.toml index 5c89c9704..cec1945a8 100644 --- a/embedded-hal-bus/Cargo.toml +++ b/embedded-hal-bus/Cargo.toml @@ -24,6 +24,7 @@ embedded-hal = { version = "1.0.0", path = "../embedded-hal" } embedded-hal-async = { version = "1.0.0", path = "../embedded-hal-async", optional = true } critical-section = { version = "1.0" } defmt-03 = { package = "defmt", version = "0.3", optional = true } +portable-atomic = {version = "1", default-features = false} [package.metadata.docs.rs] features = ["std", "async"] diff --git a/embedded-hal-bus/src/i2c/atomic.rs b/embedded-hal-bus/src/i2c/atomic.rs new file mode 100644 index 000000000..793e827b1 --- /dev/null +++ b/embedded-hal-bus/src/i2c/atomic.rs @@ -0,0 +1,179 @@ +use embedded_hal::i2c::{Error, ErrorKind, ErrorType, I2c}; + +use crate::util::AtomicCell; + +/// Atomics-based shared bus [`I2c`] implementation. +/// +/// Sharing is implemented with a [`AtomicDevice`], which consists of an `UnsafeCell` and an `AtomicBool` "locked" flag. +/// This means it has low overhead, like [`RefCellDevice`](crate::i2c::RefCellDevice). Aditionally, it is `Send`, +/// which allows sharing a single bus across multiple threads (interrupt priority level), like [`CriticalSectionDevice`](crate::i2c::CriticalSectionDevice), +/// while not using critical sections and therefore impacting real-time performance less. +/// +/// The downside is using a simple `AtomicBool` for locking doesn't prevent two threads from trying to lock it at once. +/// For example, the main thread can be doing an I2C transaction, and an interrupt fires and tries to do another. In this +/// case, a `Busy` error is returned that must be handled somehow, usually dropping the data or trying again later. +/// +/// Note that retrying in a loop on `Busy` errors usually leads to deadlocks. In the above example, it'll prevent the +/// interrupt handler from returning, which will starve the main thread and prevent it from releasing the lock. If +/// this is an issue for your use case, you most likely should use [`CriticalSectionDevice`](crate::i2c::CriticalSectionDevice) instead. +/// +/// This primitive is particularly well-suited for applications that have external arbitration +/// rules that prevent `Busy` errors in the first place, such as the RTIC framework. +/// +/// # Examples +/// +/// Assuming there is a pressure sensor with address `0x42` on the same bus as a temperature sensor +/// with address `0x20`; [`AtomicDevice`] can be used to give access to both of these sensors +/// from a single `i2c` instance. +/// +/// ``` +/// use embedded_hal_bus::i2c; +/// use embedded_hal_bus::util::AtomicCell; +/// # use embedded_hal::i2c::{self as hali2c, SevenBitAddress, TenBitAddress, I2c, Operation, ErrorKind}; +/// # pub struct Sensor { +/// # i2c: I2C, +/// # address: u8, +/// # } +/// # impl Sensor { +/// # pub fn new(i2c: I2C, address: u8) -> Self { +/// # Self { i2c, address } +/// # } +/// # } +/// # type PressureSensor = Sensor; +/// # type TemperatureSensor = Sensor; +/// # pub struct I2c0; +/// # #[derive(Debug, Copy, Clone, Eq, PartialEq)] +/// # pub enum Error { } +/// # impl hali2c::Error for Error { +/// # fn kind(&self) -> hali2c::ErrorKind { +/// # ErrorKind::Other +/// # } +/// # } +/// # impl hali2c::ErrorType for I2c0 { +/// # type Error = Error; +/// # } +/// # impl I2c for I2c0 { +/// # fn transaction(&mut self, address: u8, operations: &mut [Operation<'_>]) -> Result<(), Self::Error> { +/// # Ok(()) +/// # } +/// # } +/// # struct Hal; +/// # impl Hal { +/// # fn i2c(&self) -> I2c0 { +/// # I2c0 +/// # } +/// # } +/// # let hal = Hal; +/// +/// let i2c = hal.i2c(); +/// let i2c_cell = AtomicCell::new(i2c); +/// let mut temperature_sensor = TemperatureSensor::new( +/// i2c::AtomicDevice::new(&i2c_cell), +/// 0x20, +/// ); +/// let mut pressure_sensor = PressureSensor::new( +/// i2c::AtomicDevice::new(&i2c_cell), +/// 0x42, +/// ); +/// ``` +pub struct AtomicDevice<'a, T> { + bus: &'a AtomicCell, +} + +#[derive(Debug, Copy, Clone)] +/// Wrapper type for errors originating from the atomically-checked I2C bus manager. +pub enum AtomicError { + /// This error is returned if the I2C bus was already in use when an operation was attempted, + /// which indicates that the driver requirements are not being met with regard to + /// synchronization. + Busy, + + /// An I2C-related error occurred, and the internal error should be inspected. + Other(T), +} + +impl Error for AtomicError { + fn kind(&self) -> ErrorKind { + match self { + AtomicError::Other(e) => e.kind(), + _ => ErrorKind::Other, + } + } +} + +unsafe impl<'a, T> Send for AtomicDevice<'a, T> {} + +impl<'a, T> AtomicDevice<'a, T> +where + T: I2c, +{ + /// Create a new `AtomicDevice`. + #[inline] + pub fn new(bus: &'a AtomicCell) -> Self { + Self { bus } + } + + fn lock(&self, f: F) -> Result> + where + F: FnOnce(&mut T) -> Result::Error>, + { + self.bus + .busy + .compare_exchange( + false, + true, + core::sync::atomic::Ordering::SeqCst, + core::sync::atomic::Ordering::SeqCst, + ) + .map_err(|_| AtomicError::::Busy)?; + + let result = f(unsafe { &mut *self.bus.bus.get() }); + + self.bus + .busy + .store(false, core::sync::atomic::Ordering::SeqCst); + + result.map_err(AtomicError::Other) + } +} + +impl<'a, T> ErrorType for AtomicDevice<'a, T> +where + T: I2c, +{ + type Error = AtomicError; +} + +impl<'a, T> I2c for AtomicDevice<'a, T> +where + T: I2c, +{ + #[inline] + fn read(&mut self, address: u8, read: &mut [u8]) -> Result<(), Self::Error> { + self.lock(|bus| bus.read(address, read)) + } + + #[inline] + fn write(&mut self, address: u8, write: &[u8]) -> Result<(), Self::Error> { + self.lock(|bus| bus.write(address, write)) + } + + #[inline] + fn write_read( + &mut self, + address: u8, + write: &[u8], + read: &mut [u8], + ) -> Result<(), Self::Error> { + self.lock(|bus| bus.write_read(address, write, read)) + } + + #[inline] + fn transaction( + &mut self, + address: u8, + operations: &mut [embedded_hal::i2c::Operation<'_>], + ) -> Result<(), Self::Error> { + self.lock(|bus| bus.transaction(address, operations)) + } +} diff --git a/embedded-hal-bus/src/i2c/mod.rs b/embedded-hal-bus/src/i2c/mod.rs index 8eaf2882f..5295492f1 100644 --- a/embedded-hal-bus/src/i2c/mod.rs +++ b/embedded-hal-bus/src/i2c/mod.rs @@ -8,3 +8,5 @@ mod mutex; pub use mutex::*; mod critical_section; pub use self::critical_section::*; +mod atomic; +pub use atomic::*; diff --git a/embedded-hal-bus/src/lib.rs b/embedded-hal-bus/src/lib.rs index e7e75ec76..396043a31 100644 --- a/embedded-hal-bus/src/lib.rs +++ b/embedded-hal-bus/src/lib.rs @@ -18,3 +18,4 @@ use defmt_03 as defmt; pub mod i2c; pub mod spi; +pub mod util; diff --git a/embedded-hal-bus/src/spi/atomic.rs b/embedded-hal-bus/src/spi/atomic.rs new file mode 100644 index 000000000..e97ed8ae0 --- /dev/null +++ b/embedded-hal-bus/src/spi/atomic.rs @@ -0,0 +1,131 @@ +use embedded_hal::delay::DelayNs; +use embedded_hal::digital::OutputPin; +use embedded_hal::spi::{Error, ErrorKind, ErrorType, Operation, SpiBus, SpiDevice}; + +use super::DeviceError; +use crate::spi::shared::transaction; +use crate::util::AtomicCell; + +/// Atomics-based shared bus [`SpiDevice`] implementation. +/// +/// This allows for sharing an [`SpiBus`], obtaining multiple [`SpiDevice`] instances, +/// each with its own `CS` pin. +/// +/// Sharing is implemented with a [`AtomicDevice`], which consists of an `UnsafeCell` and an `AtomicBool` "locked" flag. +/// This means it has low overhead, like [`RefCellDevice`](crate::spi::RefCellDevice). Aditionally, it is `Send`, +/// which allows sharing a single bus across multiple threads (interrupt priority level), like [`CriticalSectionDevice`](crate::spi::CriticalSectionDevice), +/// while not using critical sections and therefore impacting real-time performance less. +/// +/// The downside is using a simple `AtomicBool` for locking doesn't prevent two threads from trying to lock it at once. +/// For example, the main thread can be doing a SPI transaction, and an interrupt fires and tries to do another. In this +/// case, a `Busy` error is returned that must be handled somehow, usually dropping the data or trying again later. +/// +/// Note that retrying in a loop on `Busy` errors usually leads to deadlocks. In the above example, it'll prevent the +/// interrupt handler from returning, which will starve the main thread and prevent it from releasing the lock. If +/// this is an issue for your use case, you most likely should use [`CriticalSectionDevice`](crate::spi::CriticalSectionDevice) instead. +/// +/// This primitive is particularly well-suited for applications that have external arbitration +/// rules that prevent `Busy` errors in the first place, such as the RTIC framework. +pub struct AtomicDevice<'a, BUS, CS, D> { + bus: &'a AtomicCell, + cs: CS, + delay: D, +} + +#[derive(Debug, Copy, Clone)] +/// Wrapper type for errors returned by [`AtomicDevice`]. +pub enum AtomicError { + /// This error is returned if the SPI bus was already in use when an operation was attempted, + /// which indicates that the driver requirements are not being met with regard to + /// synchronization. + Busy, + + /// An SPI-related error occurred, and the internal error should be inspected. + Other(T), +} + +impl<'a, BUS, CS, D> AtomicDevice<'a, BUS, CS, D> { + /// Create a new [`AtomicDevice`]. + #[inline] + pub fn new(bus: &'a AtomicCell, cs: CS, delay: D) -> Self { + Self { bus, cs, delay } + } +} + +impl<'a, BUS, CS> AtomicDevice<'a, BUS, CS, super::NoDelay> +where + BUS: ErrorType, + CS: OutputPin, +{ + /// Create a new [`AtomicDevice`] without support for in-transaction delays. + /// + /// **Warning**: The returned instance *technically* doesn't comply with the `SpiDevice` + /// contract, which mandates delay support. It is relatively rare for drivers to use + /// in-transaction delays, so you might still want to use this method because it's more practical. + /// + /// Note that a future version of the driver might start using delays, causing your + /// code to panic. This wouldn't be considered a breaking change from the driver side, because + /// drivers are allowed to assume `SpiDevice` implementations comply with the contract. + /// If you feel this risk outweighs the convenience of having `cargo` automatically upgrade + /// the driver crate, you might want to pin the driver's version. + /// + /// # Panics + /// + /// 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 AtomicCell, cs: CS) -> Self { + Self { + bus, + cs, + delay: super::NoDelay, + } + } +} + +impl Error for AtomicError { + fn kind(&self) -> ErrorKind { + match self { + AtomicError::Other(e) => e.kind(), + _ => ErrorKind::Other, + } + } +} + +impl<'a, BUS, CS, D> ErrorType for AtomicDevice<'a, BUS, CS, D> +where + BUS: ErrorType, + CS: OutputPin, +{ + type Error = AtomicError>; +} + +impl<'a, Word: Copy + 'static, BUS, CS, D> SpiDevice for AtomicDevice<'a, BUS, CS, D> +where + BUS: SpiBus, + CS: OutputPin, + D: DelayNs, +{ + #[inline] + fn transaction(&mut self, operations: &mut [Operation<'_, Word>]) -> Result<(), Self::Error> { + self.bus + .busy + .compare_exchange( + false, + true, + core::sync::atomic::Ordering::SeqCst, + core::sync::atomic::Ordering::SeqCst, + ) + .map_err(|_| AtomicError::Busy)?; + + let bus = unsafe { &mut *self.bus.bus.get() }; + + let result = transaction(operations, bus, &mut self.delay, &mut self.cs); + + self.bus + .busy + .store(false, core::sync::atomic::Ordering::SeqCst); + + result.map_err(AtomicError::Other) + } +} diff --git a/embedded-hal-bus/src/spi/mod.rs b/embedded-hal-bus/src/spi/mod.rs index d408bd92f..d55fa5eaf 100644 --- a/embedded-hal-bus/src/spi/mod.rs +++ b/embedded-hal-bus/src/spi/mod.rs @@ -11,8 +11,10 @@ pub use refcell::*; mod mutex; #[cfg(feature = "std")] pub use mutex::*; +mod atomic; mod critical_section; mod shared; +pub use atomic::*; pub use self::critical_section::*; diff --git a/embedded-hal-bus/src/util.rs b/embedded-hal-bus/src/util.rs new file mode 100644 index 000000000..739f4b1b3 --- /dev/null +++ b/embedded-hal-bus/src/util.rs @@ -0,0 +1,25 @@ +//! Utilities shared by all bus types. + +use core::cell::UnsafeCell; + +/// Cell type used by [`spi::AtomicDevice`](crate::spi::AtomicDevice) and [`i2c::AtomicDevice`](crate::i2c::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 { + pub(crate) bus: UnsafeCell, + pub(crate) busy: portable_atomic::AtomicBool, +} + +unsafe impl Send for AtomicCell {} +unsafe impl Sync for AtomicCell {} + +impl AtomicCell { + /// Create a new `AtomicCell` + pub fn new(bus: BUS) -> Self { + Self { + bus: UnsafeCell::new(bus), + busy: portable_atomic::AtomicBool::from(false), + } + } +}