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

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

Open
gary600 opened this issue Mar 28, 2024 · 6 comments
Open

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

gary600 opened this issue Mar 28, 2024 · 6 comments

Comments

@gary600
Copy link

gary600 commented Mar 28, 2024

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
Copy link

codertao commented Apr 2, 2024

+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
Copy link

Rahix commented Apr 2, 2024

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
Copy link
Member

Dirbaio commented Apr 2, 2024

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
Copy link

codertao commented Apr 2, 2024 via email

@gary600
Copy link
Author

gary600 commented Apr 2, 2024

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants