Clear SPI errors on a fresh transmit #223
Conversation
| @@ -1051,21 +1051,32 @@ where | |||
| } | |||
|
|
|||
| fn send(&mut self, byte: u8) -> nb::Result<(), Error> { | |||
| let sr = self.spi.sr.read(); | |||
| // Read from DR register to clear OVR | |||
| let _ = self.spi.dr.read(); | |||
There was a problem hiding this comment.
I would feel more comfortable if we only did this read upon overrun detection. I'm curious if there's a potential race condition here of reading the DR right as we finish shifting out the current TX byte (e.g. we have received a byte for the current transfer, but haven't quite finished sending ours?). Not sure if this can really happen, but it wouldn't hurt to be careful.
| w.crcerr().clear_bit(); | ||
| } | ||
| w | ||
| }); |
There was a problem hiding this comment.
Right now, this just blindly clears any errors and doesn't indicate that they occurred in the first place. We still need to indicate that the errors happened after clearing them - it's up to the user to determine what to do after the error is detected.
There was a problem hiding this comment.
ACK. Unfortunately for that to be useful we need proper generic error types which are still stuck in limbo.
|
My plan to change this is to check errors at the END of every send, and report them then, while also clearing errors at the START of every transaction. Let me know if this works for you @ryan-summers and @therealprof |
|
Note to self: add to changelog |
Yes, that seems sensible. Also cf. stm32-rs/stm32f0xx-hal#131 and stm32-rs/stm32f0xx-hal#130 where we also discussed error handling as a tangent. |
Indeed. ;) |
269: Clear SPI errors on send to prevent races r=therealprof a=riskable I'm already aware of #223 but I tried it and... It doesn't work. I sort of went back to the drawing board on it and created my own PR that I've tested so I know it works 👍 If errors are encountered during an SPI write it'll automatically clear any bits that need clearing *then* return the error. So you get the bit cleared and the error. What more could you ask for? 😄 Co-authored-by: Riskable <[email protected]>
Previous to this, all errors on the SPI port were latching. Instead, default to clearing errors at the start of every new transaction.