rust-embedded / embedded-hal

A Hardware Abstraction Layer (HAL) for embedded systems
Apache License 2.0
2.01k stars 202 forks source link

spi: clarify traits are for master mode. #396

Closed Dirbaio closed 2 years ago

Dirbaio commented 2 years ago

All dirvers out there assume the traitsare for master mode. However, there are some HALs out there that impl the traits for SPI in slave mode, which will break when used with drivers. (stm32f1xx-hal, stm32f4xx-hal, stm32l4xx-hal, probably more).

If we add SPI slave traits in the future, they should be a separate set of traits because the API would look quite different:

Also IMO the spi, spi_slave naming is fine even if inconsistent, since the vast majority of uses are master mode. I wouldn't name the spi module spi_master.

rust-highfive commented 2 years ago

r? @ryankurte

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

vldm commented 2 years ago

Hi there again, sorry if i annoy you with this spi slave :)

But wanting to clarify some some usage examples:

you'd want the API to give extra extra info about the transaction lengths: if you start a transfer with 256-byte buffer but the master only sends 10 bytes (sets CS low, sends 10 bytes, sets CS high), you'd want transfer to return, saying "I got only 10 bytes", instead of hanging waiting for the other 246 bytes.

Not always you need to give feedback to developer. (Real world example https://img.jdzj.com/UserDocument/2017y/wenzihui/dn/zl7968.pdf) Slave spi drived vfd display. But in it's specification it enforce: a) min CLK timing b) count of bits

And it's responsibility of master to make sure that it transfer needed amount of bits, so slave just wait for FINISH(LAT in case of VFD) signal in order to understand that transfer is finished.

My thoughts is that BUS configuration code are very common for slave and master, and BUS usage code (transfer/read implementation) are almost same for them.

But there is lack of cancellation with feedback. Example of use cases, that need cancellation:

All of them are important on master too, not only on slave.

But SpiDevice is a trait that implement SS and concrete usage scenario, and there can be used to define master/slave logic.

Dirbaio commented 2 years ago

oops I had this accidentally marked as draft

bors[bot] commented 2 years ago

Build succeeded: