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

Is there a reason for forcing an SPI read after a SPI write? #62

Closed
therealprof opened this issue Mar 12, 2018 · 11 comments
Closed

Is there a reason for forcing an SPI read after a SPI write? #62

therealprof opened this issue Mar 12, 2018 · 11 comments

Comments

@therealprof
Copy link
Contributor

In

https://github.com/japaric/embedded-hal/blob/23ee5d244d7a4eba619ef4d1e08a38e5c05f8987/src/blocking/spi.rs#L60

there's a forced data read after the write which may block forever if there's no data clocked in on the MISO line. Is there a reason for this or just an oversight?

@austinglaser
Copy link
Contributor

During full duplex SPI communication, data is clocked in at the same time as it's clocked out.

The line before the one you've linked (block!(self.send(word.clone()))?;) will clock out word -- but it will also put another word in the hardware's receive buffer. The next line reads that data, and in doing so clears the buffer.

Without that line, the SPI block will probably end up entering an overrun error condition.

@therealprof
Copy link
Contributor Author

The line before the one you've linked (block!(self.send(word.clone()))?;) will clock out word -- but it will also put another word in the hardware's receive buffer. The next line reads that data, and in doing so clears the buffer.

Is that so? This sounds more like a half duplex variant.

Without that line, the SPI block will probably end up entering an overrun error condition.

Here's the funny thing: I'm trying to talk to a APA102C LED which is actually only using a simplex connection. On the STM32F103 it works with this implementation, on the STM32F042 it does not because it blocks on this read()...

@austinglaser
Copy link
Contributor

The line before the one you've linked (block!(self.send(word.clone()))?;) will clock out word -- but it will also put another word in the hardware's receive buffer. The next line reads that data, and in doing so clears the buffer.

Is that so? This sounds more like a half duplex variant.

The data is clocked simultaneously (hence full duplex), but to be nonblocking the spi::FullDuplex::send() function cannot wait for the received data to be available, so the spi::FullDuplex::read() function exists to do that waiting, and retrieve the data when it's ready.

Even in your LED driving case where, presumably, there's no connected MISO line, the SPI peripheral still clocks bits into the data register. It can be configured not to do this, (on the STM32F103, this is done through the BIDIMODE and BIDIOE bits in the CR1 register), but that's not what's currently done -- and it's probably the right choice the majority of the time, since it makes the implementation maximally flexible.

Since there's no electrical connection, that data is garbage -- but it's there, and the act of reading the data register is itself critical to the driver's operation, since it clears the RXNE flag in the CR1 register (STM32F103 reference manual, page 710) and thus prevents an overrun error.

Without that line, the SPI block will probably end up entering an overrun error condition.

Here's the funny thing: I'm trying to talk to a APA102C LED which is actually only using a simplex connection. On the STM32F103 it works with this implementation, on the STM32F042 it does not because it blocks on this read()...

That's interesting, and sounds to me like a bug in the nonblocking implementation for the SPI driver. I've got some APA102s lying around, and I think an STM32F042. I may be able to play around with them this weekend, and perhaps discover the root of the issue. I can't promise I'll find anything, or even end up having the time -- but if I do, I'd love to try to help debug your issue. Can you point me at the driver code? I see your stm32f042-hal repo, but I don't see SPI in there yet, presumably because it's still WIP.

It looks like the F042 has a different SPI peripheral than the F103 -- maybe there's some subtlety with regards to the RX FIFO that needs to be accounted for? But it's hard to say concretely at this point.

@therealprof
Copy link
Contributor Author

The data is clocked simultaneously (hence full duplex), but to be nonblocking the spi::FullDuplex::send() function cannot wait for the received data to be available, so the spi::FullDuplex::read() function exists to do that waiting, and retrieve the data when it's ready.

I'm using the blocking variant though...

I see your stm32f042-hal repo, but I don't see SPI in there yet, presumably because it's still WIP.

Hm, this is weird now. Just as I wanted to prepare a branch for your testing it started working. 🤔

@austinglaser
Copy link
Contributor

I'm using the blocking variant though...

But the default blocking variant is written in terms of the nonblocking one, and it's the behavior of that read (never returning a value) that's in question.

Hm, this is weird now. Just as I wanted to prepare a branch for your testing it started working. :
thinking:

Interesting indeed. Glad to hear things are working now, though!

@therealprof
Copy link
Contributor Author

@austinglaser Okay, that "working" state was me commenting out the nb::Error::WouldBlock part from the read implementation. Most of that code is copied and adapted from the stm32f103xx-hal crate where the same example application works just fine.

I've pushed the branch here: https://github.com/therealprof/stm32f042-hal/tree/features/half-assed-spi

@austinglaser
Copy link
Contributor

Cool, I'll see what I can figure out. Probably won't have time to really dig in until this weekend, though

@therealprof
Copy link
Contributor Author

No worries, will have to implement a few other HALs though to test @japaric fancy new ENC28J60 driver though. ;)

@austinglaser
Copy link
Contributor

@therealprof Alright, figured out what was wrong and opened a pull request (therealprof/stm32f042-hal#1)

@therealprof
Copy link
Contributor Author

Thanks a lot, I'll check it out and confirm.

@therealprof
Copy link
Contributor Author

Works nicely, merged into master and uploaded new version to crates.io. Thanks again!

peckpeck pushed a commit to peckpeck/embedded-hal that referenced this issue Nov 10, 2022
67: Prepare 0.4.0-alpha.1 release r=ryankurte a=eldruin

Closes rust-embedded#62 

Co-authored-by: Diego Barrios Romero <eldruin@gmail.com>
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

2 participants