Deny nested calls
I have a function to disable interrupts, but the problem is if I disable them and I call a function that also disables / enables them, they re-enable too early. Is the following logic sufficient to prevent this?
static volatile int IrqCounter = 0;
void EnableIRQ()
{
if(IrqCounter > 0)
{
IrqCounter--;
}
if(IrqCounter == 0)
{
__enable_irq();
}
}
void DisableIRQ()
{
if(IrqCounter == 0)
{
__disable_irq();
}
IrqCounter++;
}
source to share
Assuming you have a system where you cannot change the context when interrupts are disabled, then what you have is fine as long as you watch the enable () function call closely.
In the description you describe in the comments below, you plan to use these sections as part of your interrupt service routine. The main use is to block higher priority interrupts from starting for a specific part of the ISR.
Be aware that you will have to consider the stack depth of these nested ISRs, since when you enable interrupts before returning from an interrupt, interrupts will be included in the ISR.
Regarding the other answers: the lack of thread safety of enable () (due to if(IrqCounter > 0)
) doesn't matter as at any time you switch to enable () context switches due to interrupts being off. (If for some reason you don't have unambiguous disable / enable pairs, in which case you have other problems.)
The only thing I would like to do is add ASSERT to the permission instead of the runtime check, as you should never enable interrupts that you have not disabled.
void EnableIRQ()
{
ASSERT(IrqCounter != 0) //should never be 0, or we'd have an unmatched enable/disable pair
IrqCounter--; //doesn't matter that this isn't thread safe, as the enable is always called with interrupts disabled.
if(IrqCounter == 0)
{
__enable_irq();
}
}
I prefer the method you listed over method save(); disable(); restore();
as I don't like keeping track of some of the OS data every time I work with interrupts. But you need to know when you (directly or indirectly) call enable () from the ISR.
source to share
The way every operating system I know of is to store the state of the IRQ in a local variable and then restore that.
Obviously, your code has TOCTOU problems - if two threads are running at the same time, checking IrqCounter> 0, if IrqCounter == 1, then the first thread will see it as 1, the second thread sees it as 1, and both decrement the counter.
I would definitely try to arrange something like this:
int irq_state = irq_save();
irq_disable();
... do stuff with IRQ turned off ...
irq_restore(irq_state);
Now you don't have to worry about counters getting out of sync, etc.
source to share
This looks great, except that it is not thread safe.
Another common option is to query the interrupt on / off state and store it in a local variable, then disable interrupts, and then do what you want to do when interrupts are disabled, and then restore the state from the local variable.
source to share