rust-embedded / embedded-hal

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

SpiDevice's interface can't be used for streaming transactions #583

Open gary600 opened 6 months ago

gary600 commented 6 months ago

There are applications where, when writing data to an SPI device, it is more efficient to write the data in a single transaction rather than multiple transactions (my use case is an SPI screen that can write multiple lines in a single transaction).

However, with the current SpiDevice interface, I would need to create a slice of Operations in order to be able to write all the lines in a single transaction. Because I need a new Operation for each separate line, this requires 128 lines * 8 bytes per slice = 1KiB of additional data just for the pointers (the framebuffer itself is only 2 KiB!). The chip I am targeting only has 40KiB of memory total, so this is a substantial amount of overhead.

The interface is now stabilized because the crate is at 1.0, but is there some change that can be made to improve this situation? (e.g. an interface that takes an iterator of Operations, a closure-based interface, or something similar)

codertao commented 6 months ago

+1 ; though from a bit of research I think this might have been a design decision at some point. (the original interface seems to have been closure based, and changed in #443 ; but I haven't finished reading the history on it ) (edit: #478 seems to have a lot of discussion on this topic in it)

The lack of some way to acquire CS leads to "interesting" solutions, like in the embedded-sdmmc crate: SdCard relies on an SpiDevice, and a separate CS pin, because it needs to perform multiple and variable transactions while the CS pin is asserted.

For one project I have SPI Devices (custom SPI slave) that take a command, process it, and while processing will generate a 'busy' byte until the response is ready, and then play the response, all within a single CS assertion. That interaction isn't allowed with the current SpiDevice trait.

Rahix commented 6 months ago

The problem is that not all SPI implementations support this. Notably, I think the Linux HAL implementation runs into this problem.

The traits here in embedded-hal have to be the common denominator so that's why the decision was made to require prepared operations.

However, keep in mind that individual HAL implementations are free to add additional APIs to expose such features. You won't get a HAL generic interface from this, but when writing an application, this may not even be necessary.


If embedded-hal wants to also start supporting streaming via its traits, I guess additional traits next to SpiDevice/SpiBus could facilitate this. Something like a StreamingSpiDevice/StreamingSpiBus. Then drivers which explicitly require the streaming APIs can depend on these extended traits instead.

Dirbaio commented 6 months ago

The lack of some way to acquire CS leads to "interesting" solutions, like in the embedded-sdmmc crate: SdCard relies on an SpiDevice, and a separate CS pin, because it needs to perform multiple and variable transactions while the CS pin is asserted.

for anyone reading this: don't do this, it breaks bus sharing. issue filed.

codertao commented 6 months ago

It'd be useful if embedded-hal could expose a trait for these complex uses, so that when they are possible on a platform they can be exploited. If there was a trait or similar for 'acquire/release exclusive access to the bus', it'd provide a more standardized mechanism for lib authors to work with; and would mean the advice 'drivers should implement spidevice' could be updated.

gary600 commented 6 months ago

In the meantime, does it seem like a reasonable stop-gap for my project to handle SPI bus sharing by just passing the &'static Mutex<RefCell<SpiBus>> around? This would allow for the more complicated transactions I need. It would also require a modification of embedded-sdmmc, but a change there is necessary anyway because it's currently unsound.

adamgreig commented 6 months ago

In the meantime, does it seem like a reasonable stop-gap for my project to handle SPI bus sharing by just passing the &'static Mutex<RefCell<SpiBus>> around? This would allow for the more complicated transactions I need. It would also require a modification of embedded-sdmmc, but a change there is necessary anyway because it's currently unsound.

Yep, that's an expected way to share SpiBus while getting exclusive bus access to handle your own CS or other non-SpiDevice communications. We'd like to write it up along with other options for sharing SpiBus soon.