Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spi: add Read and separate-buffers Transfer #287

Merged
merged 1 commit into from
Nov 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
dynamically.
- Added `Debug` to all spi mode types.
- Add impls of all traits for references (`&T` or `&mut T` depending on the trait) when `T` implements the trait.
- SPI: Added blocking `Read` trait and `Read` transactional operation
- SPI: Added blocking `Transfer` trait with separate buffers (single-buffer `Transfer` has been renamed `TransferInplace`)

### Changed
- Swap PWM channel arguments to references
Expand Down
60 changes: 54 additions & 6 deletions src/spi/blocking.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,65 @@
//! Blocking SPI API

/// Blocking transfer
/// Blocking transfer with separate buffers
pub trait Transfer<W = u8> {
/// Error type
type Error: crate::spi::Error;

/// Writes and reads simultaneously. `write` is written to the slave on MOSI and
/// words received on MISO are stored in `read`.
///
/// It is allowed for `read` and `write` to have different lengths, even zero length.
/// The transfer runs for `max(read.len(), write.len())` words. If `read` is shorter,
/// 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

has this always been the case or have we implicitly been assuming read.len() == write.len()? and is this sensible to represent for more complex implementors (ie. using DMA, linux)?

we have Transactional for building sequences, if you need to do varying length things, it would seem to me to be simpler (or maybe, less surprising to demand the same lengths here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have we implicitly been assuming read.len() == write.len()?

Before this PR, only the single-buffer Transfer (now TransferInplace) existed. Single-buffer already enforced read len = write len.

The idea for the runs for max(read.len(), write.len()) comes from the nRF SPI DMA, which automatically does this. It's somewhat handy, for example to do "write command byte, read big data" you can make the write buf small (1 byte) and only need memory for a big read buf (1 + N bytes).

I agree it's not that useful if we have TransferInplace or Transactional, and it could be annoying to support using stm32 DMA for example...

If we require same-length for both bufs, do we mandate that impls panic on length mismatch, or return an error?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was discussed in today's REWG meeting. TLDR: seems people are OK with differing lengths.

Also the possibility of allowing impls to support it or not was discussed, but IMO it defeats the point since drivers wouldn't be able to use it.

fn transfer(&mut self, read: &mut [W], write: &[W]) -> Result<(), Self::Error>;
}

impl<T: Transfer<W>, W> Transfer<W> for &mut T {
type Error = T::Error;

fn transfer(&mut self, read: &mut [W], write: &[W]) -> Result<(), Self::Error> {
T::transfer(self, read, write)
}
}

/// Blocking transfer with single buffer (in-place)
pub trait TransferInplace<W = u8> {
/// Error type
type Error: crate::spi::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.
fn transfer(&mut self, words: &mut [W]) -> Result<(), Self::Error>;
fn transfer_inplace(&mut self, words: &mut [W]) -> Result<(), Self::Error>;
ryankurte marked this conversation as resolved.
Show resolved Hide resolved
}

impl<T: Transfer<W>, W> Transfer<W> for &mut T {
impl<T: TransferInplace<W>, W> TransferInplace<W> for &mut T {
type Error = T::Error;

fn transfer(&mut self, words: &mut [W]) -> Result<(), Self::Error> {
T::transfer(self, words)
fn transfer_inplace(&mut self, words: &mut [W]) -> Result<(), Self::Error> {
T::transfer_inplace(self, words)
}
}

/// Blocking read
pub trait Read<W = u8> {
/// Error type
type Error: crate::spi::Error;

/// Reads `words` from the slave.
///
/// The word value sent on MOSI during reading is implementation-defined,
/// typically `0x00`, `0xFF`, or configurable.
fn read(&mut self, words: &mut [W]) -> Result<(), Self::Error>;
}

impl<T: Read<W>, W> Read<W> for &mut T {
type Error = T::Error;

fn read(&mut self, words: &mut [W]) -> Result<(), Self::Error> {
T::read(self, words)
}
}

Expand Down Expand Up @@ -63,10 +107,14 @@ impl<T: WriteIter<W>, W> WriteIter<W> for &mut T {
/// This allows composition of SPI operations into a single bus transaction
#[derive(Debug, PartialEq)]
pub enum Operation<'a, W: 'static = u8> {
/// Read data into the provided buffer.
Read(&'a mut [W]),
/// Write data from the provided buffer, discarding read data
Write(&'a [W]),
/// Write data out while reading data into the provided buffer
Transfer(&'a mut [W]),
Transfer(&'a mut [W], &'a [W]),
/// Write data out while reading data into the provided buffer
TransferInplace(&'a mut [W]),
}

/// Transactional trait allows multiple actions to be executed
Expand Down