From 9520962e50e625823c5719f497fc05d294c5d4fc Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 17 Feb 2022 20:37:01 +0100 Subject: [PATCH] spi: allow impls returning early, add flush. Fixes #264 --- CHANGELOG.md | 1 + src/spi/blocking.rs | 56 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c51e7bb0..1a0b047c4 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/spi/blocking.rs b/src/spi/blocking.rs index 7b4b20f43..07e83ef1d 100644 --- a/src/spi/blocking.rs +++ b/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. /// @@ -184,12 +202,29 @@ impl 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 SpiBusFlush for &mut T { + fn flush(&mut self) -> Result<(), Self::Error> { + T::flush(self) + } +} + /// Read-only SPI bus -pub trait SpiBusRead: ErrorType { +pub trait SpiBusRead: 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>; } @@ -200,8 +235,11 @@ impl, Word: Copy> SpiBusRead for &mut T { } /// Write-only SPI bus -pub trait SpiBusWrite: ErrorType { +pub trait SpiBusWrite: 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>; } @@ -225,11 +263,17 @@ pub trait SpiBus: SpiBusRead + SpiBusWrite { /// 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>; } @@ -283,7 +327,7 @@ impl ExclusiveDevice { impl ErrorType for ExclusiveDevice where - BUS: ErrorType, + BUS: SpiBusFlush, CS: OutputPin, { type Error = ExclusiveDeviceError; @@ -291,7 +335,7 @@ where impl SpiDevice for ExclusiveDevice where - BUS: ErrorType, + BUS: SpiBusFlush, CS: OutputPin, { type Bus = BUS; @@ -304,10 +348,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)