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

NRF52: Stall on SPI Master + RTC IRQs and compile fixes for non SOFTDEVICE_PRESENT target #6908

Closed
wants to merge 4 commits into from

Conversation

drahnr
Copy link
Contributor

@drahnr drahnr commented May 15, 2018

Description

Contains a set of minor fixes to avoid a crash if IRQ of SPI master and RTC fire due to the sequence of IRQ locking.

The bug is reproducable with a certain binary (priv) and jumper.io, with real hardware this causes spurious hangs.

It also adds app_fifo./h from nordic SDK to make it build without SOFTDEVICE_PRESENT flag.

Pull request type

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

@drahnr drahnr changed the title Mbed os 5.8 [NRF52] Stall on SPI Master + RTC IRQs and compile fixes for non SOFTDEVICE_PRESENT target May 15, 2018
@drahnr drahnr changed the title [NRF52] Stall on SPI Master + RTC IRQs and compile fixes for non SOFTDEVICE_PRESENT target NRF52: Stall on SPI Master + RTC IRQs and compile fixes for non SOFTDEVICE_PRESENT target May 15, 2018
@0xc0170
Copy link
Contributor

0xc0170 commented May 15, 2018

This is targeting 5.8 as this is not applicable to master (because of the recent refactors) ?

@marcuschangarm
Copy link
Contributor

All these changes are for NRF51. Have you tried the new NRF52 implementation currently in master and to be released in 5.9?

@drahnr
Copy link
Contributor Author

drahnr commented May 15, 2018

Without those fixes and removing the SOFTDEVICE_PRESENT macro it will not compile my project, which is NRF52_DK derived.

I accidentally aplied the fix to the NRF51 fix though I meant to do that for NRF52, the fix itself is necessary for both.

I can not confirm or deny that the bug is fixed in master, I did not look into the source yet and only a certain test can reveal it which fails to build on master to many breaking changes...

@cmonr
Copy link
Contributor

cmonr commented May 31, 2018

@drahnr Looks like this slipped our attention due to release activities. The fix would have to come into master instead of directly targeting the release branch itself.

However, the last 5.8 minor release is about to be made in 12 hours, and all other PRs will be targeting 5.9. I would suggest trying to compile your project against the 5.9 branch and if the issue still exists, open a PR targeting master. Once the PR lands into the repo, we have a process for bringing the changes into the release branch.

@drahnr
Copy link
Contributor Author

drahnr commented May 31, 2018

@cmonr so this PR can essentially be closed I guess, correct? Hopefully I can create another test tomorrow / early next week.

@cmonr
Copy link
Contributor

cmonr commented May 31, 2018

@drahnr Correct.

Sounds good. Looking forward to it!

@cmonr cmonr closed this May 31, 2018
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

Successfully merging this pull request may close these issues.

None yet

4 participants