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

Recent updates appear to cause SPI communication lockup #707

Open
ryan-summers opened this issue Dec 4, 2023 · 12 comments
Open

Recent updates appear to cause SPI communication lockup #707

ryan-summers opened this issue Dec 4, 2023 · 12 comments

Comments

@ryan-summers
Copy link
Member

ryan-summers commented Dec 4, 2023

In booster, using the latest master causes our application to lock up and encounter watchdog resets. Backtraces consistently have pointed to some of the SPI functions as the root cause, and using older revisions of the f4xx-hal do not have these issues.

Below is the backtrace when I Ctrl+C during a lockup of our idle() task.

   0: stm32f4xx_hal::spi::Inner<SPI>::check_read
        at C:\Users\rsummers\.cargo\git\checkouts\stm32f4xx-hal-fe8350cc04cacf3f\4144b1d\src\spi.rs:764:16
   1: stm32f4xx_hal::spi::Spi<SPI,_,W>::read_nonblocking
        at C:\Users\rsummers\.cargo\git\checkouts\stm32f4xx-hal-fe8350cc04cacf3f\4144b1d\src\spi.rs:932:9
   2: stm32f4xx_hal::spi::Spi<SPI,_,W>::transfer_in_place
        at C:\Users\rsummers\.cargo\git\checkouts\stm32f4xx-hal-fe8350cc04cacf3f\4144b1d\src\spi.rs:945:32
   3: stm32f4xx_hal::spi::hal_02::blocking::<impl embedded_hal::blocking::spi::Transfer<u8> for stm32f4xx_hal::spi::Spi<SPI>>::transfer
        at C:\Users\rsummers\.cargo\git\checkouts\stm32f4xx-hal-fe8350cc04cacf3f\4144b1d\src\spi\hal_02.rs:61:13
   4: <w5500::bus::four_wire::FourWire<Spi,ChipSelect> as w5500::bus::Bus>::read_frame::{{closure}}
        at C:\Users\rsummers\.cargo\registry\src\index.crates.io-6f17d22bba15001f\w5500-0.4.1\src\bus\four_wire.rs:40:13
   5: <w5500::bus::four_wire::FourWire<Spi,ChipSelect> as w5500::bus::Bus>::read_frame
        at C:\Users\rsummers\.cargo\registry\src\index.crates.io-6f17d22bba15001f\w5500-0.4.1\src\bus\four_wire.rs:35:22
   6: w5500::socket::Socket::get_receive_size
        at C:\Users\rsummers\.cargo\registry\src\index.crates.io-6f17d22bba15001f\w5500-0.4.1\src\socket.rs:186:13
   7: w5500::raw_device::RawDevice<SpiBus>::read_frame
   8: booster::hardware::external_mac::<impl smoltcp::phy::Device for booster::hardware::Mac>::receive

If I revert to 2d8d882, this issue goes away. I'm not sure what's changed since then

@ryan-summers ryan-summers changed the title Recent updates appear to cause SPI communication failures Recent updates appear to cause SPI communication lockup Dec 4, 2023
@burrbull
Copy link
Member

burrbull commented Dec 4, 2023

Possibly something in #673
Try 4c16f3d, please.

@ryan-summers
Copy link
Member Author

Hmm. There's something else going on, as master now appears to work just fine. This is likely something related to hardware or register configurations, sorry for the false-positive here. If I keep seeing oddness, I'll reopen.

@ryan-summers
Copy link
Member Author

Ah intriguing. The issue appears to be caused by a difference between Release and Debug configurations. Debug is working fine, but Release is not.

I have returned with some more testing results:

  • Using our current main branch (in booster), I can confirm that both Release and Debug runs complete normally. This uses 2d8d882
  • Updating our main branch to use stm32-rs/master makes the Release build then fail to execute.

I retried 4c16f3d with our main branch and confirmed that this appears to be the first commit where this defect appears

@ryan-summers ryan-summers reopened this Dec 4, 2023
@burrbull
Copy link
Member

burrbull commented Dec 4, 2023

Try https://github.com/stm32-rs/stm32f4xx-hal/tree/spi_revert. It reverts check_read & check_send.

@ryan-summers
Copy link
Member Author

Interestingly, that branch indeed seems to be working without issue

@burrbull
Copy link
Member

burrbull commented Dec 4, 2023

Interestingly, that branch indeed seems to be working without issue

Yeah, strange.
I've added asserts in that branch. Could you check does it fault on any assert?

@ryan-summers
Copy link
Member Author

Even more strangely - the commit with the asserts does not work, but the commit immediately before does. Perhaps this is a race condition of some sort that arises because of extra processing time required by the new API, although I have not profiled things on my side extensively (I'm just looking at very high-level functionality of the app, i.e. it connects over MQTT or does not)

@ryan-summers
Copy link
Member Author

I might have time to dig deeper later on this, but I need to head home for the day now

@burrbull
Copy link
Member

burrbull commented Dec 6, 2023

@ryan-summers Try #709, please.

@ryan-summers
Copy link
Member Author

Still not working on my end, it seems to keep hitting lockup in the following functions:

stack backtrace:
   0: stm32f4xx_hal::spi::Inner<SPI>::check_read
        at C:\Users\rsummers\.cargo\git\checkouts\stm32f4xx-hal-fe8350cc04cacf3f\26f646d\src\spi.rs:766:19
   1: stm32f4xx_hal::spi::Spi<SPI,_,W>::read_nonblocking
        at C:\Users\rsummers\.cargo\git\checkouts\stm32f4xx-hal-fe8350cc04cacf3f\26f646d\src\spi.rs:933:9
   2: stm32f4xx_hal::spi::Spi<SPI,_,W>::transfer_in_place
        at C:\Users\rsummers\.cargo\git\checkouts\stm32f4xx-hal-fe8350cc04cacf3f\26f646d\src\spi.rs:946:32
   3: stm32f4xx_hal::spi::hal_02::blocking::<impl embedded_hal::blocking::spi::Transfer<u8> for stm32f4xx_hal::spi::Spi<SPI>>::transfer
        at C:\Users\rsummers\.cargo\git\checkouts\stm32f4xx-hal-fe8350cc04cacf3f\26f646d\src\spi\hal_02.rs:61:13
   4: <w5500::bus::four_wire::FourWire<Spi,ChipSelect> as w5500::bus::Bus>::read_frame::{{closure}}

@burrbull
Copy link
Member

burrbull commented Dec 7, 2023

It is strange. I was sure spi_revert and flags-test2 produce same code. 8 byte different

@burrbull burrbull mentioned this issue Dec 8, 2023
@burrbull
Copy link
Member

burrbull commented Dec 8, 2023

I've merged revert commits.

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