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

Fix thread_yield() on MSP430 platforms #1618

Merged
merged 4 commits into from Sep 12, 2014
Merged

Fix thread_yield() on MSP430 platforms #1618

merged 4 commits into from Sep 12, 2014

Conversation

rousselk
Copy link
Contributor

Avoid race conditions that can cause crash on MSP430 platforms, when interruptions prevent thread context switching from being correctlty done.

It fixes the biggest cause of failure for the hwtimer_wait test on MSP430 (see issue #426), the other cause being fixed by #1619.

@LudwigKnuepfer
Copy link
Member

With this PR, hwtimer_wait works on the msb-430.

@LudwigKnuepfer
Copy link
Member

Rebase to resolve the license problem.

@LudwigKnuepfer LudwigKnuepfer self-assigned this Aug 28, 2014
@LudwigKnuepfer
Copy link
Member

OK, with only this PR, the chronos does not finish the test of in time.

@rousselk
Copy link
Contributor Author

You may need #1619 too: it corrects a problem with hwtimer_spin going in an infinite loop...

@LudwigKnuepfer
Copy link
Member

chronos_success
(with #1619)

@LudwigKnuepfer
Copy link
Member

But this really is about #1617, right?

@LudwigKnuepfer
Copy link
Member

Hm, no, with only #1617 it does still fail.

@rousselk
Copy link
Contributor Author

@LudwigOrtmann Actually, #1617 only allows us to manage correctly timers with less than 32 significative bits. The PR that actually reduce the MSP430 timer to 16 significative bits is #1619: the former prepares the terrain to the latter, which is the bug buster. You need all of #1617, #1618 and #1619 to get the MSP430 platforms robust.

But I see that your tests are now passing; great!

that is: before the newly selected thread's context is totally restored
of thread_yield(), i.e.: just after SR(R2) was pushed but before
the rest of the suspended thread's context was pushed!
@rousselk
Copy link
Contributor Author

Rebased on master.

{
__restore_context_isr();

/* we want to enable if appropriate IRQs *just after*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"if"?

@rousselk
Copy link
Contributor Author

rousselk commented Sep 9, 2014

@LudwigOrtmann If you're happy with this PR, maybe could we reassign it to OlegHahm?
If I'm not mistaken, he was interested in MSP430 port of RIOT...

@OlegHahm
Copy link
Member

I am definitely and back from vacation. :)

@OlegHahm
Copy link
Member

Looks good test/hwtimer_wait seems to always work with this on TelosB. Nice work. ACK

OlegHahm added a commit that referenced this pull request Sep 12, 2014
Fix thread_yield() on MSP430 platforms
@OlegHahm OlegHahm merged commit fa8c70b into RIOT-OS:master Sep 12, 2014
@rousselk rousselk deleted the msp430-fix-thread-yield branch September 16, 2014 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Impact: major The PR changes a significant part of the code base. It should be reviewed carefully Platform: MSP Platform: This PR/issue effects MSP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants