Nested CRITICAL_REGION_ENTER/CRITICAL_REGION_EXIT

[RK]

Are you supposed to be able to nest critical regions using CRITICAL_REGION_ENTER and CRITICAL_REGION_EXIT? Here I mean you have a function which uses a critical region to surround some work, and you call that function from within another critical region set up by another function. So you end up calling CRITICAL_REGION_ENTER twice, then CRITICAL_REGION_EXIT() twice.

Looking at the code for CRITICAL_REGION_ENTER, it makes sense. If the softdevice isn't enabled, it just turns interrupts off, else it relies on the softdevice to have turned off the application ones. However the code for CRITICAL_REGION_EXIT doesn't make so much sense. It's here..

#define CRITICAL_REGION_EXIT()                                                              \
        if (CURRENT_INT_PRI != APP_IRQ_PRIORITY_HIGH)                                       \
        {                                                                                   \
            uint32_t ERR_CODE;                                                              \
            __enable_irq();                                                                 \
            ERR_CODE = sd_nvic_critical_region_exit(IS_NESTED_CRITICAL_REGION);             \
            if (ERR_CODE != NRF_ERROR_SOFTDEVICE_NOT_ENABLED)                               \
            {                                                                               \
                APP_ERROR_CHECK(ERR_CODE);                                                  \
            }                                                                               \
        }                                                                                   \
    }

This calls __enable_irq() directly, thus immediately turning on all interrupts so the critical region is effectively ended at this point even if you were nested and sd_nvic_critical_region_exit() was not going to disable the interrupts.

Should the code not do the inverse of the CRITICAL_REGION_ENTER() code and only call __enable_irq() in the case the softdevice isn't active? Something like

   #define CRITICAL_REGION_EXIT()   
            if (CURRENT_INT_PRI != APP_IRQ_PRIORITY_HIGH)  
            {     
                uint32_t ERR_CODE= sd_nvic_critical_region_exit(IS_NESTED_CRITICAL_REGION);
                if (ERR_CODE == NRF_ERROR_SOFTDEVICE_NOT_ENABLED) 
                { 
                    __enable_irq();  
                }
                else 
                {   
                    APP_ERROR_CHECK(ERR_CODE);  
                }  
            }   
        }

Or do you have to ensure that you never call functions which end up nesting critical regions like this?

[RK]

I finally managed to set up a test case to test this by using functions which call each other, each of which makes its own critical scope. Sort of like this

void foo( void )
{
    CRITICAL_REGION_ENTER()
    .. do some work ..
    CRITICAL_REGION_EXIT()
}

void bar( void )
{
    CRITICAL_REGION_ENTER()
    .. do some other work ..
    foo();    // call foo
    .. and do a little cleanup work .. 
}

That's not a particularly unusual pattern, when you have a protected resource it does happen you end up protecting your work and also then calling a helper function which protects the same thing.

If you have the softdevice enabled then it works. The reason for that is that the sd_nvic_critical_region_enter()/exit() functions disable/enable the individual user interrupts, they don't use __irq_disable() to disable everything, so the call to __irq_enable() makes no difference, only when the finally non-nested sd_nvic_critical_region_exit() is called are the individual interrupts re-enabled and then they fire.

However if the soft device is disabled then it does fail, but for a different reason than I expected. The first entry to the critical region disables interrupts. When you come to the second entry (this is before you exit the first one, because they are nested) sd_nvic_critical_region_enter() is called again, however interrupts are at that point disabled and emitting an SVC call with interrupts disabled is a hard fault, so it hard faults.

Since I do have this issue I'll write my own version of the wrapper which checks only the first time if the SD is enabled/disabled and then just calls it or the _irq* functions and nests properly, my library code has to work with and without a softdevice enabled and needs the nested critical regions. I need more than just calling sd_nvic_critical_region() however as if the SD is disabled, I do need to at some point call __irq_disable() to actually get the criticality.

[Joe Merten]

Looks that I could check the lsb of PRIMASK (determined by __get_PRIMASK()) to check if irq currently is enabled or not.