> I never found any reason to change the mask in foreground code other than to disable.
You must be writing simple code based on 8-bitter designs. These are 32-bitters. You might as well code for the features rather than try to emulate an older technology.
The point of always performing a "save/raise/restore" sequence is threefold:
1 - It lets you write code using the multiple execution levels of these chip. You can write pretty good "poor man's threading" using this. Simply having the background code taking as long as it likes with an interrupt-triggered "high priority thread" separate from time-critical interrupts lets you write simple, yet powerful and predictable (timing) programs.
2 - It lets you write "call safe from anywhere" functions that access data structures that have to have locking associated with them. You don't want to have to have one set of functions that you can only call from the background (because it disables interrupts) and another one you can only call from a single interrupt level (because it doesn't disable interrupts).
3 - It is "simple code that simply works and obviously works".
The product I'm working on now uses all levels:
IPL0: background (slow) code
IPL1: Time-critical logging code
IPL2: USB stack interrupts
IPL3: Ethernet
IPL4: Serial port and ADC software interrupt ADC completion
IPL5: CAN, Flash erase complete and QSPI (CAN & ADC)
IPL6: High-speed CAN
IPL7: Software Watchdog, Power monitor, Code Profiler.
Your "CLI/STI with call-depth memory" solves (2) but can't be used with code taking advantage of (1). It can't work if you're using threads either.
I think I can see some problems with it. The fact that it is using two globals is a big clue that it might be unsafe. What is protecting access to those globals? Where are the locks? You may need to disable interrupts around their accesses, except the purpose of this function...
> if (++SR_level == 1)
SR_level is a shared global variable. If the CPU doesn't execute the "++" atomically (but does a load/increment/store) then it can get incremented twice and stored once. If it does atomically increment it, then it has to reload it after the increment to test it, and it may have been accessed between those by an interrupt. The optimiser may spot the test, and so decide to "load, increment, store, test the incremented data" rather than "atomic increment, then load and test" which is slower. The "volatile" might change the order, but it may not do what you want. You really need this part in assembler so you know which hazards it really has.
The above code might work, but it doesn't OBVIOUSLY work.
What happens if there's an interrupt between that instruction and the one that finally disables the interrupt by executing the "move.w D0,SR;"?
The interrupt routine will see that "SR_LEVEL" has already been incremented, so it WON'T DISABLE THE INTERRUPTS! That interrupt routine can then be interrupted by a higher one that also calls this function and it won't disable the interrupts either! The mainline won't have a problem, but any variables shared between the interrupt routines are unsafe.
You may not code this way, but some future programmer who is modifying your code might fall for this trap.
The above hazards and faults won't matter if ALL of the interrupt service routines are running at the same IPL, or all of them separately force the IPL to "7", but that's not likely.
The whole POINT of "int oldLevel = asm_set_ipl(NEW_LEVEL) is that the saved state is on the STACK and not in a Global. That simple approach makes it safe by design. It means it can be used anywhere and just simply works simply.
Tom