The impossible interrupt from Timer 2, ATmega324P

Go To Last Post
7 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Here's the code - it's a somewhat simplified version of what I'm working on, in particular the actions inside the while loop have been removed because they don't add to the mystery or change the behavior of the code.

The question is, how can pulse_count be 2?

The code starts in the debugger, I give it some keyboard input causing INT1 to fire and clock in the PS/2 scan codes. Timer 2 is started in CTC mode to detect when idle time has elapsed since the last code input and when it expires TIMER2_COMPA_vect is reached. The first thing that happens is that interrupts are disabled...

pulse_count has been initialized to 0 in main before interrupts are enabled. Then in the ISR it gets incremented... but it should be impossible for it to be incremented again because of cli() and also TIMSK2 = 0 - but on breaking the debugger always shows the value of pulse_count to be 2.

This is not a huge problem because the codes will be handled there and it is easy to do nothing if the codes have been handled. But it's a mystery.

More information - I thought it may have been timer2 expiring too early for the incoming codes to keep TCNT2 below OCIE2A - but all scan codes I expect to see are being received. Timer 2 is firing when I think it should - after the last INT0 - and no earlier. But the strange thing is, if I reset recvClock to -1 in the Timer 2 interrupt to guard against bogus codes and get a fresh start on the next code set, no codes are received at all, which suggests Timer 2 is firing early and often :cry:

// For ATMEGA324P

#include 
#include 

void go_state_receive(void);
void flush_codes(void);

typedef enum _state {
	STATE_MIRROR,
	STATE_RECEIVE
} state;

volatile unsigned char received_code[80];
volatile unsigned char code;
volatile int pulse_count;
volatile int code_cursor;
volatile int recvClock;
volatile int parity;
volatile state stateNow;

// empty the buffer for incoming codes
void flush_codes(void)
{
	int i;

	// flush codes!
	for(i = 0; i < 80; i++)
		received_code[i] = 0x00;
}

// setup and loop - idle loop handles different tasks depending on state
int main()
{
	// DEBUG - bicolor LED on PORTC
	DDRC |= (1 << DDC0) | (1 << DDC1);

	EIMSK &= ~(1 << INT1);				// disable interrupt on INT1
	EICRA |= (1 << ISC11);				// set ISC11
	EICRA &= ~(1 << ISC10);				// clear ISC10 (generate interrupt on falling edge)
	EIMSK |= (1 << INT1);				// enable INT1
	
	DDRA &= ~(1 << DDA1);				// BS_DATA is an input
	DDRD &= ~(1 << DDD3);				// BS_CLOCK is an input
	PORTA |= (1 << PORTA1);
	PORTD |= (1 << PORTD3);				// both have pullups

	TCCR2A = 0 | (1 << WGM21);			// set timer 2 to CTC mode
	TCCR2B = 0 | (1 << CS22) | (1 << CS21) | (1 << CS20);	// with prescaler /1024
	OCR2A = 0xff;						// interrupt at max

	flush_codes();						// clear out anything in the received codes array
		
	pulse_count = 0;
	code = parity = 0;					// initialize globals used for receiving codes
	recvClock = -1;

	stateNow = STATE_MIRROR;

	sei();								// global interrupt enable

	while(1)
	{
		if(stateNow == STATE_MIRROR)
		{
			PORTC &= ~(1 << PORTC1);	// make the LED GREEN
			PORTC |= (1 << PORTC0);
		}
		else
		{
			PORTC &= ~(1 << PORTC0);	// make the LED RED
			PORTC |= (1 << PORTC1);
		}	
	}
}

// timeout - treat any incoming codes as a complete string
ISR(TIMER2_COMPA_vect)
{
	cli();			// DISABLE ALL INTERRUPTS!
	TIMSK2 = 0;
	pulse_count++;	// <-- HOW CAN THIS EXECUTE TWICE!!!

	// return to mirroring keyboard.
	stateNow = STATE_MIRROR;
}

// interrupt from keyboard
ISR(INT1_vect)
{
	if(stateNow != STATE_RECEIVE)
		go_state_receive();

	switch(recvClock)
	{
		case -1:	// start bit must be 0
			break;
		case 0: case 1: case 2: case 3: case 4: case 5: case 6: case 7: // data bits
			if(PINA & (1 << PINA1))
			{
				code |= (0x01 << recvClock);
				parity++;
			}
			break;
		case 8: // parity bit
			if(PINA & (1 << PINA1))
				parity++;
			break;
		case 9:	// stop bit must be 1
			if(PINA & (1 << PINA1))
			{
				if(parity % 2)	// parity must be odd
				{
					// code is good.
					received_code[code_cursor++] = code;
				}
			}
			break;
		case 10: 
			TCNT2 = 0x00;
			code = parity = 0;	// reset code and parity for the incoming code
			recvClock = -1;
			break;
	}		
	recvClock++;
}

void go_state_receive(void)
{
	// reset counter 1 and enable interrupt on compare match
	TCNT2 = 0;
	TIMSK2 |= (1 << OCIE2A);

	// set receiving state
	stateNow = STATE_RECEIVE;
}

Attachment(s): 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

1) You don't need cli() in an ISR. That happens automatically when the AVR jumps via the interrupt vector

2) more importantly the return from an ISR() is a RETI instruction. This does the same as sei();RET. THAT is why a second 2_COMPA interrupt can occur. If your intention really is to stop that interrupt then reset it's IE bit in the TCCR register. Or if you want all interrupt sources to be disabled either clear ALL their IE bits or set a flag then do a cli() when you see this in main() - though there'll be some latency in that obviously.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for looking at this hair-tearing problem. Good information about the RETI and the auto cli() and sei() effect - I had read about the RETI but was no longer trusting it and the futility of cli() had not occurred to me but I stuck it in to try it when TIMSK=0 had no effect.

The INT1 routine, clocking in data bits on the PS/2 interface, should be allowed to happen any time. When it doesn't happen for long enough Timer 2 fires - either it has n valid codes and the codes have stopped coming (time to process valid codes), or else it got at least one spurious clock and Timer 2 is a rescue procedure to reset it to a known state for the next batch of valid codes.

TIMSK2=0 MUST stop the second Timer 2 interrupt - it's the interrupt mask register. If no interrupts are enabled yet the ISR is jumped to, something strange is happening and it's killing my project. I'm going to have to start a blank project and experiment with Timer 2 to see if I can make just one interrupt fire. Maybe there's some interaction between INT1 and Timer 2? A start might be to create a new array and have each interrupt routine write "1" or "2" into it in the order that they are firing so I have a better idea of just what is happening in what order.* But when the ISR disables itself (and I have also tried manually clearing TIFR2 at that time, i.e. setting OCF2A) I can't see any sane way it should fire again.

*Have done this now - Timer 2 interrupt is firing as soon as the interrupt is enabled in go_state_receive() called by INT1 ISR as well as when the CTC condition is met. But why? Is there some problem with enabling one interrupt in another's ISR?

Last Edited: Sun. Jul 29, 2007 - 12:12 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

I had read about the RETI but was no longer trusting it

Interesting. You'd think that a problem in an oft-used and "important" instruction like RETI would have shown up before in the hundreds of millions of AVRs in operation.

Are there any other instructions "over 30" and therefore not to be trusted?

Lee

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

More likely my judgement is not to be trusted after 15 hours at the keyboard. I know I've done something wrong here and I hope whoever can point it out gets a good laugh at my expense :wink:

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Just tried setting the clock back to 1MHz (re-enabled internal /8 divisor fuse) and now there is only one interrupt as expected! It's not a solution because at 1MHz not enough instructions can be processed per clock edge... :(

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

aeberbach wrote:
*Have done this now - Timer 2 interrupt is firing as soon as the interrupt is enabled in go_state_receive() called by INT1 ISR as well as when the CTC condition is met. But why? Is there some problem with enabling one interrupt in another's ISR?
Your OCF2A flag is already set when you enable the interrupt. Your timer is already running, and a compare match has already occurred by the time you get to go_state_receive(). But because the interrupt was not enabled, no one has cleared your irq flag (which the irq would take care of), and as soon as you exit the int1 irq, you go directly to the timer2 comapre irq. Either leave the timer off until needed, or clear the irq flag before you enable the irq.

But I could be wrong.