Add QSPI driver and example #107
Conversation
bbrown1867
commented
Dec 23, 2020
bbrown1867
commented
- Simple QSPI HAL driver that supports reading/writing (receiving/sending) using INDIRECT mode.
- Memory mapped and auto-polling mode are not supported at the moment.
- Add an example using the MT25Q flash chip on the discovery board.
- Can read and write using polling (CPU moves data in/out) or DMA.
- Note: This DMA implementation is pretty basic compared to SPI and Serial. No type-state programming. That could be added in the future.
|
@hannobraun @mvertescher any feedback on this driver? |
There was a problem hiding this comment.
Thank you for the PR, @bbrown1867!
I don't know anything about QSPI, nor do I have the hardware or time to test this. I did a quick review, and can't see anything wrong with it. Although it's a bit odd that the DMA methods are blocking (why use DMA then).
I left some comments with general nitpicks. Feel free to address those is a follow-up PR or not at all, as you prefer. I'm going to merge this now. It's been open long enough, I think.
| #![no_main] | ||
| #![no_std] | ||
|
|
||
| extern crate panic_semihosting; |
There was a problem hiding this comment.
These days it's usually better to use RTT together with cargo-embed instead, since semihosting is so slow. No big deal if it works, I'm just saying there are better options.
| w.fmode() | ||
| .bits(fmode) | ||
| .imode() | ||
| .bits(transaction.iwidth) | ||
| .admode() | ||
| .bits(transaction.awidth) | ||
| .dmode() | ||
| .bits(transaction.dwidth) | ||
| .adsize() | ||
| .bits(self.adsize) | ||
| .abmode() | ||
| .bits(QspiWidth::NONE) | ||
| .dcyc() | ||
| .bits(transaction.dummy) | ||
| .instruction() | ||
| .bits(transaction.instruction) |
There was a problem hiding this comment.
It would be nicer to group each field access into a separate statement, like this:
w.fmode().bits(fmode);
w.imode().bits(transaction.iwdidth);
// ...That's much more readable, in my opinion, as it prevents rustfmt from messing up the grouping between selecting the field and writing to the field.
There was a problem hiding this comment.
Yes I agree that is much more readable. I was trying to group the field and bits statements onto 1 line but rustfmt kept moving them - I think the issue was that I was not repeating w.
Thanks for taking a look @hannobraun ! Yes this peripheral is a bit more niche so I didn't expect a lot of feedback on it. The DMA methods are much faster than the polling ones, so even if blocking DMA could be beneficial. I agree that making the API non-blocking would be an improvement. I had tried to do this by returning the DMA handle after starting but had issues with exposing crate-private types publicly. Perhaps I need to make some kind of transfer wrapper type like |
|
Thanks for the explanation, @bbrown1867. Yes, DMA APIs can be tricky. I haven't worked on this HAL for a while, but I assume those methods are crate-private because they would allow the user to circumvent things that the API would like to guarantee. Providing a peripheral-specific wrapper is the way to go, I think. |
