Menu

#92 portENTER_CRITICAL / portEXIT_CRITICAL nesting on MSP430

v1.0 (example)
closed-invalid
nobody
None
5
2015-02-17
2014-07-19
ilgitano
No

I've seen this particular point brought up repeatedly and marked as invalid, so my apologies bringing it up again. The issue is that portENTER_CRITICAL / portEXIT_CRITICAL cannot be used in an ISR, or any interrupt-disabled context. You have said this is designed-in behaviour, but a relatively minor change would resolve the problem.

I hope the pseudocode is understandable. I have this working and thoroughly tested in our local codebase. I also repeat myself with the actual implementation.

portBASE_TYPE usCriticalNesting = 1; // or any value > 0; 1 is best

In portENTER_CRITICAL:
    if (interrupts_currently_enabled)
    {
        // set nesting to 0 to ensure interrupts are reenabled in the final EXIT_CRITICAL,
        // rather than unexpectedly leaving interrupts disabled.
        usCriticalNesting = 0; // otherwise inherit the previous value for nesting
    }
    disable_interrupts;
    usCriticalNesting++;

In portEXIT_CRITICAL:
    usCriticalNesting--;
    if (usCriticalNesting == 0)
    {
        // set it to 1 so next time portENTER_CRITICAL is called, it is >0; then on leaving
        // a final nested portEXIT_CRITICAL it won't unreasonably enable the interrupts again.
        usCriticalNesting = 1;
        enable_interrupts;
    }

With this change, portENTER_CRITICAL/portEXIT_CRITICAL can be used anywhere, in an ISR and out, with the scheduler started or not, even completely independently of FreeRTOS; this can be used completely standalone.

The actual implementation for MSP430 is here. Of course, nothings free, there is a cost: to be portable, then the platform implementations will have to provide a low cost means to test if interrupts are enabled or disabled. For IAR/MSP430 __get_SR_register() can be used.

volatile unsigned portBASE_TYPE usCriticalNesting = 1;

#define portENTER_CRITICAL()                                                            \
{                                                                                       \
extern volatile unsigned portBASE_TYPE usCriticalNesting;                               \
                                                                                        \
    if (__get_SR_register() & __SR_GIE)                                                 \
    {                                                                                   \
        __disable_interrupt();                                                          \
        /* If interrupts are set, start at zero. We then know to reenable interrupts */ \
        /* when the count returns to zero. If interrupts are disabled, we have no    */ \
        /* way of knowing if this is a first call, or a nested call. just increment. */ \
        usCriticalNesting = 0;                                                          \
    }                                                                                   \
    usCriticalNesting++;                                                                \
}                                                                                       \

#define portEXIT_CRITICAL()                                                             \
{                                                                                       \
extern volatile unsigned portBASE_TYPE usCriticalNesting;                               \
                                                                                        \
    usCriticalNesting--;                                                                \
    if (usCriticalNesting == 0)                                                         \
    {                                                                                   \
        /* About to leave the outermost critical section; the next critical section  */ \
        /* started might be with interrupts enabled or might not, so set to 1.       */ \
        usCriticalNesting = 1;                                                          \
        __enable_interrupt();                                                           \
    }                                                                                   \
}                                                                                       \

Discussion

  • Richard Barry

    Richard Barry - 2014-07-19
    • status: open --> closed-invalid
     
  • Richard Barry

    Richard Barry - 2014-07-19

    Thank you for your suggestion. This text has been moved to the following feature request as it is not reporting a bug.

    https://sourceforge.net/p/freertos/feature-requests/84/

     

Log in to post a comment.

MongoDB Logo MongoDB