# Setting port in main loop causes ISR to starve

The example code

• initializes an asynchronous timer that is fired every second
• sets two ports as outputs (PA4, PA6 - LEDs connected)
• the timer ISR toggles pin PA4
• permanently sets pin PA6 to 1 in the main while() loop

If pin PA6 is toggled using

PORTA |= (1 << PA6);


everything works as expected. The LED on pin PA4 is toggled exactly every second. However, if the port is set like this

PORTA |= (1 << PinNumber);


the pin PA4 is only toggled very sporadically (observed up to 22 seconds between toggles). This indicates for me that for some reason the controller is either "busy" or the overflow ISR flag isn't set at all.

EDIT: If I add some other cycle intensive code like a _delay_us(10); the situation improves in that way that les and less interrupts will be missing the longer the delay. This makes sense if we assume that the pin toggle operation is at some point somehow blocking ISR entry.

Since the ISR execution should always take precedence over the code in the main loop, I don't understand why this can happen and how exactly it makes a difference if the pin number is passed as a constant or as a variable.

I understand that the compiler can probably perform some nice optimization if a constant is used, but using a variable shouldn't result in some ISR-blocking code.

This is on GCC 4.8.1.

#include "stdlib.h"
#include "avr/interrupt.h"

uint8_t PinNumber;

// asynch ISR with external clock source, fires every second
ISR (TIMER2_OVF_vect) {
PORTA ^= (1 << PA4);
}

int main(void) {
DDRA |= (1 << PA6) | (1 << PA4);    // set PA6 and PA4 as output
PinNumber = PA6;      // variable that just holds PA6 for demonstration

cli();                      // global interrupt disable during init
ASSR    |= (1<<AS2);        // Asynchronous Timer/Counter2 from external clock
TCNT2   = 0x00;
OCR2A   = 0x00;
OCR2B   = 0x00;
TCCR2A  = 0x00;
TCCR2B  = 0x05;             // set divider for one second
TIMSK2  |= (1 << TOIE2);    // enable TIMER2_OVF_vect
sei();                      // global interrupt enable

while(1) {
//PORTA |= (1 << PinNumber);    // this somehow causes the interrupt to "starve"
PORTA |= (1 << PA6);            // this works as expected
}


}

EDIT: The practical background is, that we observed the ISR not executing correctly when repeatedly changing several output ports, where the pin number is variable at runtime.

• You don't say what micro you're using, but is it possible that you have a read-modify-write problem? In other words, maybe your interrupt is running just fine, but the instruction in your main loop is inadvertently clearing PA4. Its just a guess, but possibly when you use a constant the compiler can generate code which avoids the issue (maybe by using a bit-set instruction or something similar), but can't with a variable. – brhans Dec 16 '14 at 13:29
• brhans is correct. In your main() loop, PORTA is being read, then your interrupt occurs modifying PORTA, then back in main, the OR operation is performed on the old value of PORTA and put back in PORTA losing the effect of the ISR. You need to disable the interrupt before your line in main() and then re-enable it afterwards. – Tut Dec 16 '14 at 13:45
• Oh, of course, I should have seen that. When using a constant, GCC seems to optimize it down to a single sbi instruction. When using a variable it has to "buffer" the port state before performing the OR operation. If the ISR fires between those instructions, the change from the ISR essentially gets lost. – Rev1.0 Dec 16 '14 at 14:40

As pointed out in the comments, the problem is the read-modify-write sequence being interrupted, causing the instruction in the main loop inadvertently clearing PA4.

When using a constant, GCC seems to optimize it down to a single sbi instruction. When using a variable, the port state is copied before the OR operation is performed. If the ISR fires between those instructions, the change from the ISR essentially gets lost.

Probably the memory access, which is quite slower, is holding things up. Also maybe it's better to change the port state in only one place, to avoid the read-modify-write sync mentioned in the comments. My suggestion:

#include "stdlib.h"
#include "avr/interrupt.h"

uint8_t PinMask = 1 << PA6;

// asynch ISR with external clock source, fires every second
ISR (TIMER2_OVF_vect) {
}

int main(void) {
DDRA |= (1 << PA6) | (1 << PA4);    // set PA6 and PA4 as output

cli();                      // global interrupt disable during init
ASSR    |= (1<<AS2);        // Asynchronous Timer/Counter2 from external clock
TCNT2   = 0x00;
OCR2A   = 0x00;
OCR2B   = 0x00;
TCCR2A  = 0x00;
TCCR2B  = 0x05;             // set divider for one second
TIMSK2  |= (1 << TOIE2);    // enable TIMER2_OVF_vect
sei();                      // global interrupt enable

while(1) {