The comment in the CM4F’s port.c xPortSysTickHandler() function says that “all interrupts must be unmasked”…
/* The SysTick runs at the lowest interrupt priority, so when this interrupt
executes all interrupts must be unmasked. There is therefore no need to
save and then restore the interrupt mask value as its value is already
known. */
However what this function does in the beginning is to actually MASK (disable) all interrupts (so it can perform its code atomically) by raising BASEPRI. I believe that comment need to be updated s/all interrupts must be unmasked/all interrupts must be masked to have the comment match the actual code.
I believe that the intent of that comment is that at the end of the ISR, we know we will be unmasking all the interrupts, so we don’t need to save the initial interrupt status to restore it at the end. This will allow for a small improvement in code efficiency.
We actual DON’T mask all the interrupts, but only those with a low enough priority to be able to interact with the FreeRTOS API.
It appears the current comment is (wrongly) explaining why it doesn’t need to bother with a standard practice – namely saving the original interrupt mask for restoring after the critical section.
There real reason we don’t need to restore the original value of BASEPRI is not because the systick always uses the lowest interrupt priority but because the NVIC won’t allow lower-priority interrupts when an ISR is active (when the CPU is in handler mode) regardless of the value in BASEPRI. And when the ISR ends, it will restore the original interrupt mask (BASEPRI) for us anyway (when changing to normal thread mode).
I believe that the intent of that comment is that at the end of the ISR, we know we will be unmasking all the interrupts, so we don’t need to save the initial interrupt status to restore it at the end.
My concern is around the fact that the comment says it is “unmasking” (enabling) interrupts right before(!) executing portDISABLE_INTERRUPTS(), which actually disables interrupts. I’m not arguing what the code does, it’s just that I think the placement of this comment makes things more confusing then they need to be. I think the simple edit that I suggested in the OP would help.
We actual DON’T mask all the interrupts, but only those with a low enough priority to be able to interact with the FreeRTOS API.
Good point, you are correct. This would be another opportunity to further clarify the comment
I don’t think so. The comment is stating what is already true, prior to the code in the ISR. “All interrupts must be unmasked” is a statement of precondition, not a statement of work to do.
It’s one reason why we should all avoid passive voice in our comments!
@jefftenney I see your point. The finer details of the English language together with the non-trivialities of Cortex-M interrupt handling made me interpret this differently. Anyways, thanks for chiming in!
