spi: allow impls returning early, add flush. Fixes #264

Author: Dirbaio

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - The Minimum Supported Rust Version (MSRV) is now 1.54.0
 - `spi`: unify all traits into `SpiReadBus`, `SpiWriteBus` and `SpiBus` (read-write).
 - `spi`: Add `SpiDevice` trait to represent a single device in a (possibly shared) bus, with managed chip-select (CS) pin.
+- `spi`: Clarify that implementations are allowed to return before operations are finished, add `flush` to wait until finished.
 
 ## [v1.0.0-alpha.7] - 2022-02-09
 

src/spi/blocking.rs

@@ -140,6 +140,23 @@
 //!
 //! HALs **must not** add infrastructure for sharing at the [`SpiBus`] level. User code owning a [`SpiBus`] must have the guarantee
 //! of exclusive access.
+//!
+//! # Flushing
+//!
+//! To improve performance, Bus implementations are allowed to return before the operation is finished, i.e. when the bus is still not
+//! idle. You must call [`flush`](SpiBusFlush::flush) to wait for operations to actually finish, for example before deasserting CS in a
+//! [`SpiDevice`] implementation, or before deinitializing the hardware SPI peripheral.
+//!
+//! For example, for [`write`](SpiBusWrite::write) operations, it is common for hardware SPI peripherals to have a small
+//! FIFO buffer, usually 1-4 bytes. Software writes data to the FIFO, and the peripheral sends it on MOSI at its own pace,
+//! at the specified SPI frequency. It is allowed for an implementation of [`write`](SpiBusWrite::write) to return as soon
+//! as all the data has been written to the FIFO, before it is actually sent. Calling [`flush`](SpiBusFlush::flush) would
+//! wait until all the bits have actually been sent, the FIFO is empty, and the bus is idle.
+//!
+//! This still applies to other operations such as [`read`](SpiBusRead::read) or [`transfer`](SpiBus::transfer). It is less obvious
+//! why, because these methods can't return before receiving all the read data. However it's still technically possible
+//! for them to return before the bus is idle. For example, assuming SPI mode 0, the last bit is sampled on the first (rising) edge
+//! of SCK, at which point a method could return, but the second (falling) SCK edge still has to happen before the bus is idle.
 
 use core::fmt::Debug;
 
@@ -162,6 +179,7 @@ pub trait SpiDevice: ErrorType {
     /// - Locks the bus
     /// - Asserts the CS (Chip Select) pin.
     /// - Calls `f` with an exclusive reference to the bus, which can then be used to do transfers against the device.
+    /// - [Flushes](SpiBusFlush::flush) the bus.
     /// - Deasserts the CS pin.
     /// - Unlocks the bus.
     ///
@@ -232,12 +250,29 @@ impl<T: SpiDevice> SpiDevice for &mut T {
     }
 }
 
+/// Flush support for SPI bus
+pub trait SpiBusFlush: ErrorType {
+    /// Blocks until all operations have completed and the bus is idle.
+    ///
+    /// See the [module-level documentation](self) for important usage information.
+    fn flush(&mut self) -> Result<(), Self::Error>;
+}
+
+impl<T: SpiBusFlush> SpiBusFlush for &mut T {
+    fn flush(&mut self) -> Result<(), Self::Error> {
+        T::flush(self)
+    }
+}
+
 /// Read-only SPI bus
-pub trait SpiBusRead<Word: Copy = u8>: ErrorType {
+pub trait SpiBusRead<Word: Copy = u8>: SpiBusFlush {
     /// Reads `words` from the slave.
     ///
     /// The word value sent on MOSI during reading is implementation-defined,
     /// typically `0x00`, `0xFF`, or configurable.
+    ///
+    /// Implementations are allowed to return before the operation is
+    /// complete. See the [module-level documentation](self) for detials.
     fn read(&mut self, words: &mut [Word]) -> Result<(), Self::Error>;
 }
 
@@ -248,8 +283,11 @@ impl<T: SpiBusRead<Word>, Word: Copy> SpiBusRead<Word> for &mut T {
 }
 
 /// Write-only SPI bus
-pub trait SpiBusWrite<Word: Copy = u8>: ErrorType {
+pub trait SpiBusWrite<Word: Copy = u8>: SpiBusFlush {
     /// Writes `words` to the slave, ignoring all the incoming words
+    ///
+    /// Implementations are allowed to return before the operation is
+    /// complete. See the [module-level documentation](self) for detials.
     fn write(&mut self, words: &[Word]) -> Result<(), Self::Error>;
 }
 
@@ -273,11 +311,17 @@ pub trait SpiBus<Word: Copy = u8>: SpiBusRead<Word> + SpiBusWrite<Word> {
     /// incoming words after `read` has been filled will be discarded. If `write` is shorter,
     /// the value of words sent in MOSI after all `write` has been sent is implementation-defined,
     /// typically `0x00`, `0xFF`, or configurable.
+    ///
+    /// Implementations are allowed to return before the operation is
+    /// complete. See the [module-level documentation](self) for detials.
     fn transfer(&mut self, read: &mut [Word], write: &[Word]) -> Result<(), Self::Error>;
 
     /// Writes and reads simultaneously. The contents of `words` are
     /// written to the slave, and the received words are stored into the same
     /// `words` buffer, overwriting it.
+    ///
+    /// Implementations are allowed to return before the operation is
+    /// complete. See the [module-level documentation](self) for detials.
     fn transfer_in_place(&mut self, words: &mut [Word]) -> Result<(), Self::Error>;
 }
 
@@ -331,15 +375,15 @@ impl<BUS, CS> ExclusiveDevice<BUS, CS> {
 
 impl<BUS, CS> ErrorType for ExclusiveDevice<BUS, CS>
 where
-    BUS: ErrorType,
+    BUS: SpiBusFlush,
     CS: OutputPin,
 {
     type Error = ExclusiveDeviceError<BUS::Error, CS::Error>;
 }
 
 impl<BUS, CS> SpiDevice for ExclusiveDevice<BUS, CS>
 where
-    BUS: ErrorType,
+    BUS: SpiBusFlush,
     CS: OutputPin,
 {
     type Bus = BUS;
@@ -352,10 +396,12 @@ where
 
         let f_res = f(&mut self.bus);
 
-        // If the closure fails, it's important to still deassert CS.
+        // On failure, it's important to still flush and deassert CS.
+        let flush_res = self.bus.flush();
         let cs_res = self.cs.set_high();
 
         let f_res = f_res.map_err(ExclusiveDeviceError::Spi)?;
+        flush_res.map_err(ExclusiveDeviceError::Spi)?;
         cs_res.map_err(ExclusiveDeviceError::Cs)?;
 
         Ok(f_res)