GIE getting cleared somehow

[aek]

Salvo 4.2.1-rc0 is now online and incorporates these changes (and some other fixes as well -- see http://www.pumpkininc.com/salvo/lite/msp430/4/versions.txt).

------------------

[This message has been edited by aek (edited April 29, 2008).]

[Dave Hohl]

I was pondering this as I was lying in bed last night, and I think I figured out what is happening. The scheduler gets called by the main loop and prepares to enter a critical section. Interrupts are enabled. The first statement of OSDisableHook gets executed, and stores a 1 in the static variable s. The timer interrupt occurs before the second statement -- which disables interrupts -- can execute. The ISR calls an OS service, which enters a critical section by saving the current GIE state (0) into s, overwriting the previously stored value. When the critical section is done it leaves interrupts disabled. Then, after the ISR exits, the scheduler executes the second line of OSDisableHook, disabling interrupts. But when it later calls OSEnableHook the value of s is zero, so interrupts never get re-enabled.

Typically critical section code would use an auto variable to save the state of the GIE. In your case, since it needs to be accessed by two separate functions, you cannot do this. I have not figured out a way to get around this yet. In the past I have used a counter to count nesting of interrupt disables, but I am not sure if that would work in this situation.

[aek]

That's a very plausible explanation ...

The reason why we missed that here is because of two things:

1) Most examples are with just a single ISR calling OSTimer(), and since OSTimer() is only called once in any Salvo application, it does not call or need any of the interrupt hooks, and

2) For more sophisticated examples, we "go" directly to the "forget handling GIE, control the individual interrupt enable bits instead" approach, which has GIE initially enabled and lets the interrupt-handlers re-enable GIE upon exit from an interrupt, quite separate from the atomic bit clear/set that the individual control brings ...

So, the question becomes one of whether it's possible to have a good GIE-clearing default hook that works in all situations ...

------------------

[aek]

OK, here's the solution:

code:

---

istate_t s;
void OSDisableHook(void)
{
  asm("push.w  SR    ");
  asm("dint          ");
  asm("pop     R15   ");
  asm("movx.w  R15,&s");
}

void OSEnableHook(void)
{
  asm("movx.w  &s,R15");
  asm("mov.w   R15,SR");
}

---

For MSP430X, of course. movx.w -> mov.w for MSP430.

1) This issue has been nagging me for a while -- I appreciate your bringing it to my attention.

2) I'm not crazy about this solution, but it is better than blind control of GIE, as that can result in nested interrupts, which most users likely want to avoid.

3) In-line assembly is generally a no-no, but these routines are just asm() "wrappers", so there should not be any side effects. Ideally one should code this as an assembly module in the future ...

------------------

[Dave Hohl]

Thanks for the quick solution. I don't think you need to use assembly language, though. I tried the following, and it seemed to work:

code:

---

static istate_t s;
void OSDisableHook(void)
{
    istate_t t = __get_interrupt_state();
    __disable_interrupt();
    s = t;
}

void OSEnableHook(void)
{
    __set_interrupt_state(s);
}

---

Another approach might be to use macros instead of functions. The Disable macro could open a block and declare s as an auto variable, and the Enable macro would then close the block. This would work as long as both disable and enable are called from within same function at the same level.

[This message has been edited by Dave Hohl (edited April 29, 2008).]

[This message has been edited by aek (edited April 29, 2008).]

[aek]

Yes, that's more elegant.

We used to use macros -- but there are some issues with them that get ugly. Also, there are places (e.g. in the scheduler) where there are multiple calls to them (in the "windows" where interrupts can/should be enabled to improve responsiveness).

I like your solution, though.

------------------

[aek]

Interestingly, Rowley's compiler handles this automatically in their __disable_interrupt(), such that the disable hook is much simpler:

code:

---

void OSDisableHook(void)
{
  s = __disable_interrupt();
}

---

Internally they don't bother to push SP onto the stack (just save it in R15, dint, etc.), but the net effect is the same ...

------------------

[This message has been edited by aek (edited April 29, 2008).]

[aek]

Advanced Topic:

The primary reason why we moved from macros in Salvo v3 to user hooks (functions) in Salvo 4 is that with hooks, users can change (optimize, really) the interrupt control even in Salvo library builds, not just in source-code builds.

Note that for a Salvo Pro user, the hooks can be replaced by macros (defined in salvocfg.h) which may in some cases result in smaller and faster code ...

------------------