spi: add Read and separate-buffers Transfer

[Dirbaio]

Depends on #286

• Added spi::Read for only reading
• Renamed Transfer to TransferInplace
• Added a new Transfer, for reading+writing simultaneously with different buffers.

Open question

• Should Transactional gain Read, Transfer (not-inplace) support? yes, done

[rust-highfive]

r? @therealprof

(rust-highfive has picked a reviewer for you, use r? to override)

[eldruin]

Other than my comments below, looks good to me, thanks!
Closes #286

[eldruin]

Looks good to me, thanks!
Any objections @rust-embedded/hal ?

[ryankurte]

thanks for the PR! i'm not convinced all of these are improvements. have left a couple of comments, though it's possible i missed some discussion of this already?

[burrbull]

Merge conflicts

[lachlansneff]

Any chance of this getting into 1.0?

[Dirbaio]

Rebased. Now that Default is gone, the diff is much smaller :D

[eldruin]

Other than my nitpicks, looks good to me, thanks!
@ryankurte do you have any further concerns about implementation-defined fill values?

[ryankurte]

hey generally seems good, one question about the updated docs and, though it appears to be minor changes, i would quite like to see a couple of impls updated to support this to make sure we're not missing anything.

[ryankurte]

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?

[Dirbaio]

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?

[Dirbaio]

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.

[eldruin]

@Dirbaio This is good to go! Could you resolve the conflict?

[Dirbaio]

@eldruin rebased

[eldruin]

Ach, a new conflict appeared due to the default u8 PR, could you rebase again @Dirbaio ?

[Dirbaio]

nooooo 🤣

[Dirbaio]

@eldruin rebased

[eldruin]

Alright, thanks!
bors merge

[bors]

Build succeeded:

• ci-linux (1.46.0, x86_64-unknown-linux-gnu)
• ci-linux (stable, thumbv6m-none-eabi)
• ci-linux (stable, thumbv7m-none-eabi)
• ci-linux (stable, x86_64-unknown-linux-gnu)
• ci-linux-test (1.46.0, x86_64-unknown-linux-gnu)
• ci-linux-test (stable)
• fmt