Interrupt problem (timer1 timing + 'interference')

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

Hello All,

I'm using Timer1 to generate an interrupt about every 70 clocks and it seems to work fine. However when I use another interrupt, timer0 or the ADC, it's timing goes to hell. Initially I thought that this was due to the CPU running out of oomph and taking too long to respond, but when I look a test pulse on a 'scope sometimes they appear to be early. Could (say) the timer0 interrupt also be triggering the timer1 ISR ? Am I doing something daft ? Please find attached the relevant bits of code, ideas/thoughts/(constructive)critism welcome :)

#define PS_CODE_0 1           	// Prescale OFF,1,8,64,256,1024
#define PS_CODE_1 1           	// Prescale OFF,1,8,64,256,1024
#define ADCSRA_def ((1<<ADSC) | (1<<ADATE) | (1<<ADIE) | 0x05 ) 	
#define ADCSRB_def (1<<ADLAR)	// Free running + left adjusted data

//***************** Interrupt vectors
//ISR(RESET_vect)		{ } 
//ISR(INT0_vect) 		{ } 
//ISR(PCINT0_vect) 		{ } 
//ISR(PCINT1_vect )		{ } 		
//ISR(WDT_vect) 		{ } 		
//ISR(TIM1_CAPT_vect) 	{ }
	
  ISR(TIM1_COMPA_vect) 	
  {
  	PORTA |= TEST_op;
  	PORTA &= ~TEST_op;
  	return;
  } 

//ISR(TIM1_COMPB_vect)	{ }		
//ISR(TIM1_OVF_vect) 	{ }
//ISR(TIM0_COMPA_vect) 	{ }
//ISR(TIM0_COMPB_vect)	{ }	
	
  ISR(TIM0_OVF_vect)	
  {
 	return;
  } 	

//ISR(ANA_COMP_vect) 	{ }	

  ISR(ADC_vect) 		
  { 
  	return;
  }
  	
//ISR(EE_RDY_vect) 		{ }	
//ISR(USI_STR_vect) 	{ }		
//ISR(USI_OVF_vect) 	{ }	

//*********************** Initialisation

        /* Timer 0 */
  
	TCCR0A = (1<<COM0A1 | 1<<WGM01 | 1<<WGM00);	// configure timer 0 fast PWM operation 
	TCCR0B = 0; 			// timer off
	OCR0A  = 0x00;			// start at 0 
	TIMSK0 = (1<<TOIE0); //(1<<OCIE0A | 1<<TOIE0);	// enable CTC + overflow interrupt (timer not yet running)


	/* Timer 1 */

	TCCR1A = 0; 			// configure timer 1 normal operation - no extn pins
	TCCR1B = (1<<WGM12); 	// configure timer 1 for CTC mode
	OCR1A  = 0x45;			// set compare value to def 
	TIMSK1 = (1<<OCIE1A);	// enable CTC interrupt (timer not yet running) 


	/* ADC */

	ADCSRA = ADCSRA_def; 	// set up clk + not running 
	ADCSRB = ADCSRB_def; 	// set up data right adjusted + free running
	ADMUX  = Audio;			// set input of ADC to audio

//***************** Enable in main routine


// start timer 0

	TCNT0 = 0x00;						// reset counter 
	TCCR0B |= PS_CODE_0; 				// start timer with correct prescaler

// start timer 1

	TCNT1H = 0x00;						// reset counter - MSB
	TCNT1L = 0x00;						// reset counter - LSB
	TCCR1B |= PS_CODE_1; 				// start timer with correct prescaler

// start ADC

//	ADCSRA = ((1<<ADEN) | ADCSRA_def);		// Enable ADC 	


<º))))><

I am only one lab accident away from becoming a super villain.

Last Edited: Tue. Feb 24, 2009 - 10:21 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

70 clocks ian't much time. most of this will be chewed up getting into and out of the isr. Try running the code in the avrstudio simulator and see what happens - you'll be able to time various operations and see what is happening.

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

A good idea, I'll try to simulate it.

70clks isn't a great deal of time, but then again the CPU has almost nothing to do but toggle an output line. Surely it can handle that the PICs I've used would have no trouble.

<º))))><

I am only one lab accident away from becoming a super villain.

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

Hmm... I've changed the timer1 interrupt to trigger every 192 clocks, which should be slow enough for the AVR to handle. Again it works fine until I turn on timer0, then it's timing is all aver the place ?

Ah, think I've got it... it appears that it takes AGES (well ~24 clks) just to service a empty interrupt call, and the 'early' ticks are infact just very late ones. (I'm used to PICs which are much faster at handling this) :( OK I'll have to find another way of solving this problem, if anyone has a better idea than polling the timer I'd appreciate it.

<º))))><

I am only one lab accident away from becoming a super villain.

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

That sounds more like a compiler issue and how many registers are saved versus the actual speed of the controller. What does the enter and exit code look like? Also, it the PIC does actually save some registers in hardware, that would make a difference.

Randy

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

The PIC has hardware to swap, and it doesn't need to back up lots of registers (as it doesn't have them :S !).

I don't know how to tell the compiler what to back up when switching to a ISR, I hope that it only bothers saving ones that are used and not all of them. How do I find this info ? Is there a way to write the interrupts in assembler to save time ?

<º))))><

I am only one lab accident away from becoming a super villain.

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

Quote:

I don't know how to tell the compiler what to back up when switching to a ISR, I hope that it only bothers saving ones that are used and not all of them. How do I find this info ? Is there a way to write the interrupts in assembler to save time ?

See the post I just made in the PIC/AVR thread in OT. The fact is that GCC's ISR may be "non optimal" and if you REALLY need the performance then follow the manual example:

http://www.gnu.org/savannah-chec...

and put the time critical ISR in as a hand crafted .S (maybe getting the compiler to first generate something from C and then work on it)

Cliff

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

Hello Colin,

thanks for the link. As I have the code/system ready to go I'd like to try replacing the ISR with just reti, even though it may look like ancient runes :S !

One little draw back is that.... I don't know how to do it... I was thinking of something like :

 ISR(TIM0_OVF_vect)	
  {	
  	asm
	(
  	reti 
	)
  }

If you can correct that for me ?

Would that also mean that the compiler still swapped all the registers ? as :

ISR(ADC_vect) 		
  { 
	//OCR0A = ADCH>>Volume; 	// TEST STUFF
  	return;
  }

still generates :

@0000013F: __vector_13
503:        { 
+0000013F:   921F        PUSH    R1               Push register on stack
+00000140:   920F        PUSH    R0               Push register on stack
+00000141:   B60F        IN      R0,0x3F          In from I/O location
+00000142:   920F        PUSH    R0               Push register on stack
+00000143:   2411        CLR     R1               Clear Register
506:        }
+00000144:   900F        POP     R0               Pop register from stack
+00000145:   BE0F        OUT     0x3F,R0          Out to I/O location
+00000146:   900F        POP     R0               Pop register from stack
+00000147:   901F        POP     R1               Pop register from stack
+00000148:   9518        RETI                     Interrupt return

<º))))><

I am only one lab accident away from becoming a super villain.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
ISR(ADC_vect, ISR_NAKED) {
	asm("reti\n");
  92:	18 95       	reti

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

Hey I thought that you were joking about 'NAKED' LOL

<º))))><

I am only one lab accident away from becoming a super villain.

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

Hey hey hey - That's better - it's now about 8 clks, which is roughly what I was expecting. I'll have to write the ISR in assembler and drop them in. I'd appreciate any more useful links you have.

Thanks so much for the help :)

<º))))><

I am only one lab accident away from becoming a super villain.

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

Just because the ISR is naked doesn't mean that you need assembler. If you are sure that what you do in the ISR will not affect any registers, you could still do it in C. For instance your timer1 ISR:

PORTA |= 1<<3; 
PORTA &= ~1<<3;

would end up compiling to

SBI porta, 0x08
CBI porta, 0xf7

You will need to put in the RETI in asm though.

Regards,
Steve A.

The Board helps those that help themselves.

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

Just to add to what Steve says - if you had used the example code I posted in that other thread:

PORTB = 0x55;

then this would have generated:

ISR(ADC_vect, ISR_NAKED) {
	PORTB = 0x55;
  92:	85 e5       	ldi	r24, 0x55	; 85
  94:	88 bb       	out	0x18, r24	; 24
	asm("reti\n");
  96:	18 95       	reti

in which case you'd need to manually provide the PUSH/POP for R24:

ISR(ADC_vect, ISR_NAKED) {
	asm("push R24\n");
	PORTB = 0x55;
	asm("pop R24\n");
	asm("reti\n");
}

which generates:

ISR(ADC_vect, ISR_NAKED) {
	asm("push R24\n");
  92:	8f 93       	push	r24
	PORTB = 0x55;
  94:	85 e5       	ldi	r24, 0x55	; 85
  96:	88 bb       	out	0x18, r24	; 24
	asm("pop R24\n");
  98:	8f 91       	pop	r24
	asm("reti\n");
  9a:	18 95       	reti

and more generally you need to inspect any generated Asm then add code to preserve anything else it may clobber, possibly including the SREG state in which case you need to push a register, then read SREG into it, then push that.

Cliff

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

A few more questions:

Is there a way of reserving some of the registers so that the C compiler never uses them ?
I'm trying :

ISR(TIM0_OVF_vect, ISR_NAKED) 
  { 
	asm("IN		R31, 0x3F	\n");	// back up STATUS FLAGS

	asm("ADIW	R29, 0x01	\n"); 	
	asm("ADD	R27, R25	\n"); 
	asm("ADC	R28, R26	\n"); 

	asm("OUT	R31, 0x3F	\n"); 	// restore STATUS FLAGS
   asm("RETI			    \n"); 
  }

Is there a neater way of doing this ?

<º))))><

I am only one lab accident away from becoming a super villain.

Last Edited: Tue. Feb 24, 2009 - 06:17 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes:

register int this_is_bound asm("r16");

However you have to be careful about which registers you use for this as the compiler uses some for its own purposes (r24 a LOT for example!)

Also the contents of libm.a and libc.a know nothing about this reservation so if you start calling any libc functionality you have no guarantee that the library code may not have been built to use the register(s) you reserved. So basically avoid library calls or check all the linked code.

BTW, the ABI for the compiler is given here:

http://www.gnu.org/savannah-chec...

Cliff

PS before the GCC police arrive should I move this thread to "GCC forum"?

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

Quote:
before the GCC police arrive should I move this thread to "GCC forum"?
Feel free to move it wherever you think is most appropriate.

It's going to all end up in assembler if I'm not careful !

<º))))><

I am only one lab accident away from becoming a super villain.

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

Quote:

It's going to all end up in assembler if I'm not careful !

No hardship - avr-as is a very powerful linking assembler.

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

Is there a neater way of adding a block of assembler code into a C program rather than line by line ? (please see a few posts above).

Personally I like assembler :) it's the mix and match that can cause problems. I just need the basic framework sorted, then I'll allocate the ISR variables into registers and that should be much faster.

Thanks for your help on this.

<º))))><

I am only one lab accident away from becoming a super villain.

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

Hmm... I'm trying :

  ISR(TIM0_OVF_vect, ISR_NAKED) 
  { 
	asm("in R31, 0x3F\n");		// back up STATUS FLAGS

	asm("adiw R29, 0x01\n"); 	

	asm("add R27, R25\n"); 		
	asm("adc R28, R26\n"); 

	asm("out R31, 0x3F\n"); 	// restore STATUS FLAGS
   asm("reti\n"); 
  }

It generates :
C:\TEMP/cc6pUQqr.s: Assembler messages:
C:\TEMP/cc6pUQqr.s:63: Error: register r24, r26, r28 or r30 required
C:\TEMP/cc6pUQqr.s:78: Error: constant value required
C:\TEMP/cc6pUQqr.s:78: Error: number must be less than 32
C:\TEMP/cc6pUQqr.s:78: Error: register name or number from 0 to 31 required

<º))))><

I am only one lab accident away from becoming a super villain.

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

Quote:
C:\TEMP/cc6pUQqr.s:63: Error: register r24, r26, r28 or r30 required

adiw requires a register pair starting at an even address, so

adiw r29

won't work, but r28 would (for register pair 28:29)

   asm("out R31, 0x3F\n");    // restore STATUS FLAGS

This needs to be

   asm("out 0x3F, R31\n");    // restore STATUS FLAGS

Also, you are clobbering all of these registers. I guarantee that it will cause severe problems. The upper three register pair are going to be used quite frequently by the compiler.

Regards,
Steve A.

The Board helps those that help themselves.

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

Quote:

Is there a neater way of adding a block of assembler code into a C program rather than line by line ? (please see a few posts above).

Yes, you put it in a .S file - like I say this is far easier than doing battle with the tortuous inline syntax.

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

I don't know how to create an .S file and link it in. I could look it up but as the ISRs are only a handful of instructions each I'll live with the messy format.

It's annoying that some of the instructions don't work on all of the registers (ADIW for one example), but that's a quirk I can live with now I'm aware of it.

Thanks to your help and suggestions I've changed all the ISRs to stripped down versions and that's freed up a lot of processing time and corrected the problem. Everything is (currently) working perfectly - whaahay !

<º))))><

I am only one lab accident away from becoming a super villain.

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

Do you use a Makefile and build at the command line or do you use Studio as an IDE for GCC? If the former then hopefully you use Mfile to create the Makefile in which using .S is simply a case of editing the code into text file with a .S (not .s!!!) extension and putting its name on the ASRC= line in the Makefile. If the latter then it's possibly even easier. You just edit the text into a file with .S extension then right click the project tree and use "add existing source file(s)..." to add it to the list of files to be built. avr-gcc then "knows" to hand off .c files to the C compiler and .S files to gas to be assembled. All the resultant .o files are then simply linked back together by avr-ld to produce the resultant .elf file.

To see an example of .S then look at this file from that example in the user manual I mentioned previously:

http://www.gnu.org/savannah-chec...

BTW, unlike when using C, the one key thing to note is that if IN or OUT (or any op that operates on IO addresses) are used you must wrap the symbol name in that _SFR_IO_ADDR() macro to account for the 0x20 address offset

Cliff