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

SPI errors will be persistent #13

Open
austinglaser opened this Issue Feb 27, 2018 · 0 comments

Comments

Projects
None yet
1 participant
@austinglaser

austinglaser commented Feb 27, 2018

My reading of the current error detection code in the SPI module seems to suggest that if an error occurs it will never be cleared. The code in question:

                fn read(&mut self) -> nb::Result<u8, Error> {
                    let sr = self.spi.sr.read();

                    Err(if sr.ovr().bit_is_set() {
                        nb::Error::Other(Error::Overrun)
                    } else if sr.modf().bit_is_set() {
                        nb::Error::Other(Error::ModeFault)
                    } else if sr.crcerr().bit_is_set() {
                        nb::Error::Other(Error::Crc)
                    } else if sr.rxne().bit_is_set() {
                        // NOTE(read_volatile) read only 1 byte (the svd2rust API only allows
                        // reading a half-word)
                        return Ok(unsafe {
                            ptr::read_volatile(&self.spi.dr as *const _ as *const u8)
                        });
                    } else {
                        nb::Error::WouldBlock
                    })
                }

                fn send(&mut self, byte: u8) -> nb::Result<(), Error> {
                    let sr = self.spi.sr.read();

                    Err(if sr.ovr().bit_is_set() {
                        nb::Error::Other(Error::Overrun)
                    } else if sr.modf().bit_is_set() {
                        nb::Error::Other(Error::ModeFault)
                    } else if sr.crcerr().bit_is_set() {
                        nb::Error::Other(Error::Crc)
                    } else if sr.txe().bit_is_set() {
                        // NOTE(write_volatile) see note above
                        unsafe { ptr::write_volatile(&self.spi.dr as *const _ as *mut u8, byte) }
                        return Ok(());
                    } else {
                        nb::Error::WouldBlock
                    })
                }

The reference manual (section 30.5.11, pp 974) states of the overrun flag:

Clearing the OVR bit is done by a read access to the SPI_DR register followed by a read access to the SPI_SR register

Of the mode fault flag:

Use the following software sequence to clear the MODF bit:

  1. Make a read or write access to the SPIx_CR1 register while the MODF bit is set.
  2. Then write to the SPIx_CR1 register.

The procedure for clearing a CRC error is not explictly listed in the SPI section, but it's listed as an rc_w0 bit. Section 2.1, pp 46, says:

Software can read as well as clear this bit by writing 0. Writing '1' has no effect on the bit value.

Because of the way the module is coded, I suspect (though I haven't tested my assumption) that if one of these errors is set, the SPI driver will become completely nonfunctional -- the bit will never be cleared, and so the SPIx_DR register will never again be accessed. Furthermore, the programmer cannot even re-initialize the driver, since the constructor consumes the SPI control block and pins.

The question then becomes how to expose error-clearing functionality -- especially in a platform independent way. I see several options, each with some tradeoffs:

  1. Clear errors automatically on detection.
    • This is probably the most ergonomic and presents the fewest barriers to portability.
    • On the other hand, it provides the programmer the least flexibility
  2. Provide a function for manually clearing all errors
    • Still pretty portable, since it doesn't leak details of what errors you're clearing to the programmer -- could be added to the SPI trait, or could be exposed as a separate trait
    • A possible stumbling block for users
  3. Provide functions for clearing each error specifically
    • Less portable, less ergonomic, more room for programmer error

Of these, I think I favor option 1 or 2. The only advantage I could see to 2 over 1 would be if there's additional action the user of the SPI driver needs to take between detecting an error and clearing it in the SPI driver -- for instance, resetting an external device. This use case is a bit of a stretch.

I'd love to take a crack at implementing this myself -- but I'd like to get some feedback as to what you'd accept in a pull request before I do any actual work.

@austinglaser austinglaser referenced a pull request that will close this issue Feb 27, 2018

Open

SPI: Clear error flags when detected #14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment