Setting port in main loop causes ISR to starve

Question

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. — Rev, Dec 16 '14 at 14:40

score 1 · Answer 1 · answered Jun 25 '15 at 07:04

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.

score 0 · Answer 2 · answered Dec 16 '14 at 13:49

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;
volatile uint8_t PAMask = 0;  

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

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) {
        PORTA = PAMask;
    }

}

Your example code is a valid approach. But like you said, it was pointed out in the comments that in the end it really comes down to the read-modify-write sequence being interrupted by the ISR. — Rev, Dec 16 '14 at 14:48

Setting port in main loop causes ISR to starve

2 Answers2