From 156e4a8c0d0cfcb1ac4bd0fe1c47e4ed1efcccd2 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Tue, 23 Apr 2024 21:40:25 +0200 Subject: [PATCH] bus: expand docs for AtomicDevice a bit. --- embedded-hal-bus/src/spi/atomic.rs | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/embedded-hal-bus/src/spi/atomic.rs b/embedded-hal-bus/src/spi/atomic.rs index 135c556b..6e9eaa78 100644 --- a/embedded-hal-bus/src/spi/atomic.rs +++ b/embedded-hal-bus/src/spi/atomic.rs @@ -28,18 +28,26 @@ impl AtomicCell { } } -/// `UnsafeCell`-based shared bus [`SpiDevice`] implementation. +/// 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 `UnsafeCell`. This means it has low overhead, and, unlike [`crate::spi::RefCellDevice`], instances are `Send`, -/// so it only allows sharing across multiple threads (interrupt priority level). When attempting -/// to preempt usage of the bus, a `AtomicError::Busy` error is returned. +/// 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. /// -/// This primitive is particularly well-suited for applications that have external arbitration -/// rules, such as the RTIC framework. +/// 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, @@ -47,7 +55,7 @@ pub struct AtomicDevice<'a, BUS, CS, D> { } #[derive(Debug, Copy, Clone)] -/// Wrapper type for errors originating from the atomically-checked SPI bus manager. +/// 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