I sometimes enable interrupts within an ISR when the time critical part has finished, and sometimes I even do this to allow the ISR to be reentrant (useful for timer interrupts) putting time consuming parts within a locked section of code.
HOWEVER it turns out that the ATmega4809 disables interrupts within an ISR based on a separate status register, and not by doing a cli(), so executing sei() to reenable interrupts does not work.
I discovered this by careful reading of the datasheet and some experimentation after I've successfully done this in other AVRs and other microcontrollers for over a decade. There seems to be no way to reenable interrupts other than returning from the ISR (the rti instruction). This seems to be a requirement of the half-hearted (basically two level) priority interrupt scheme in the ATmega4809.
The SAM (ARM) ones in the MAKR boards some others also have multilevel interrupt priorities. I was just shocked that modern ATmega4809 was so deficient.
The issue first came up for me about 10 years ago. I had a timer interrupt for my system tick clock and would hang mulltiple tasks off of it, each one basically:
if (currentTick - lastEventTick > TIME_BETWEEN_TICKS) {
lastEventTick = currentTick;
DO THE TRIGGERED ACTION
}
However had an action that would take many milliseconds when the interrupt period was a single millisecond. This would delay the other actions, which in this application would cause spurious errors.
Of course one possibility was to pull the time consuming operation out of the ISR altogether and run it when needed from the main program loop. But as I was moving this code from another processor I didn't want to change the basic structure of the code to accomplish this (it was a BACNET driver consisting of many thousands of lines of code).
The solution was to reenable interrupts, but lock out the code for the event taking the time. The other events would still run by interrupting the event taking all the time.
What is really needed is prioritized interrupts (like in ARM microcontrollers) and have the lowest level interrupt be a timer interrupt dedicated to this long running action. We don't have the luxury with the AVRs in most Arduino boards, and the ATmega4809 basically allows a single higher level interrupt when we want a single lower level interrupt!
I think I understand what you mean. But the approach is the wrong way around. Normally interrupts are generally allowed. Only for critical things they are disabled for a short time and then enabled again. So the other way around like you do it.
For this there are ready things like. This always safely restores the original state.
It does seem like I'm doing it backwards! But it is to achieve the same goal -- disable interrupts for critical code and enable them at other times. Code in ISRs is generally considered critical and interrupts are automatically disabled during their execution, however you can find sections of an ISR that are not critical and can safely and beneficially be interruptible. Since in my earlier example there is a section of code where I want to enable interrupts but the code is not reentrant, I protect the region from reentrance with a simple semaphore-like mechanism, the lock variable.
CPUINT.STATUS ?
The Instruction set manual has a more explicit note:
RETI behaves differently in AVRe, AVRxm, and AVRxt devices. In the AVRe series of devices, the Global Interrupt Enable bit is cleared by hardware once an interrupt occurs, and this bit is set when RETI is executed. In the AVRxm and AVRxt devices, RETI will not modify the Global Interrupt Enable bit in SREG since it is not cleared by hardware while entering ISR. This bit should be modified using SEI and CLI instructions when needed
Exactly. Caught me completely off-guard. I should have known better because the "traditional" method with only the global enable bit in SREG won't work when there are multiple interrupt levels because there needs to be an enable bit for each level. These bits are in the new CPUINT.STATUS register. Sadly, for my use case, these bits are readonly.
;; do a "sneaky jump" by pushing the target address, and
;; then doing a reti instruction. This might clear the "ISR in progress"
;; flags, permitting subsequent interrupts.
ldi r24, low(ISRsON) ;; (check syntax and ordering!)
push r24
ldi r24, high(ISRsON)
push R24
reti
ISRsON: ;; more code
That kludge might actually work! Although I do have a work-around. All the lengthy operations go in loop triggered by a flag variable. Basically I turn loop into a lowest priority ISR. Since my "goal" is to be 100% interrupt driven and do nothing in loop this is pretty easy to accomplish. OF course the other solution is not to use an ATmega4809, Arduino Nano Every, but I really like that part!
Maybe calling that function (from ISR) taken push and pop all “call-used” registers and SREG.
Isn't it will unnecessary overhead added seriously?
Please check the assembly list after compilation.
I need to get up the courage and try this! Since it basically makes the remainder of the ISR a standard subroutine, cli and sei could then be used, however it would be important to leave the global interrupt enabled on return which wouldn't be necessary with the older AVRs. (I'm thinking here about maximum code portability.)
OK! I deem this "Solved". Here is my simple test program. Note that references to pin, port, and ddr access the GPIO registers directly -- I don't use the Arduino functions.
#include <Pins.h>
void setup() {
// put your setup code here, to run once:
ddr.digital_2 = 1;
ddr.digital_3 = 1;
ddr.digital_4 = 1;
TCA0.SINGLE.CMP0BUF = 0x80;
TCA0.SINGLE.INTCTRL |= 0x10;
TCA0.SINGLE.CMP1BUF = 0x80;
TCA0.SINGLE.INTCTRL |= 0x20;
}
volatile uint32_t counter;
ISR (TCA0_CMP0_vect)
{
TCA0.SINGLE.INTFLAGS = 0x10;
pin.digital_4 = 1;
}
__attribute(( naked, noinline )) static void isrIrqOn () { asm("reti"); }
ISR (TCA0_CMP1_vect)
{
static volatile bool lock = false;
static volatile uint16_t tickCount = 0;
TCA0.SINGLE.INTFLAGS = 0x20; // reset interrupt flag
pin.digital_3 = 1; // toggle with each interrupt
tickCount++;
if (tickCount >= 100) {
if (!lock) { // Not reentrant
lock = true;
port.digital_2 = 1;
// sei(); // reenable interrupts because following is lengthy
isrIrqOn(); // reenable interrupts because following is lengthy
counter = 100000L;
while (counter > 0) {
counter--;
}
// cli();
port.digital_2 = 0;
tickCount = 0;
lock = false;
}
}
}
void loop() {
// put your main code here, to run repeatedly:
}
Using the (commented out) sei/cli, which doesn't work for the 4809, I see (top to bottom, running the time-consuming code, toggle on timer interrupt, toggle on another timer interrupt:
After commenting out sei/cli and adding isrIrqOn, the toggling continues during the time consuming code, and doesn't skip a beat as can be seen in this closeup:
While the question/problem is solved, I wished for a way with minimum code impact to have the ATmega4809 be compatible. After more study and testing I concluded that just putting this at the start of any ISR that wants to use sei/cli:
This disables the global interrupt before doing the RTI. If the cli is not done first, then the isrIrqOn will enable the interrupt, and I'd rather have it be compatible with the other AVRs (sei needed to enable).
Also, while the rti instruction does not re-enable interrupts in the 4809, the generated code restores the SREG register so even if the ISR has executed cli the interrupts will be enabled on return (actually just before return).
