Interrupt not returning!

Go To Last Post

14 posts / 0 new

Author

Message

nxp

Level: Rookie

Joined: Fri. Sep 8, 2006

Posts: 47 View posts

Location: Malta

Posted by nxp: Tue. Sep 19, 2006 - 11:15 PM

Hello all!

After finally getting my first project to run well [timer with interrupt as clock divider to a counting variable and then output of the counted value to port D], I decided to get into something more advanced.

Right now I'm experimenting with ADC and PWM. Trying to read the adc value through channel 3 and using timer 2 to generate PWM on an AtMega8 chip. The OCR2 value is updated in the main program, while the ADC complete interrupt just sets a flag variable for the main program to know it has to update OCR2.

But it doesn't work!

I tried to debug using the AVRSTUDIO simulator. The configuration registers are all loaded as in my code and both the timer and the ADC start. The ADC runs until the end of the first conversion and it seems to enter the ISR, where it sets the flag variable, and then stops at the IRET instruction!

The timer keeps running but execution never goes from IRET to the main code, and in the disassembly window there is no address associated to the IRET, as shown below. [would the address be stored in an SFR somwhere?]

0000003C:   9518        RETI                     Interrupt return

Any ideas on what is happening? I am using WinAvr to compile my code, with the KamAvr gui. I have no knowledge of AVR assembly [but I have done assembly on other chips], so my interpretation of the code might not be exact!

Here is the assembly code, if you would like the source ask me [trying not to post too long].

+00000013:   2411        CLR     R1               Clear Register
+00000014:   BE1F        OUT     0x3F,R1          Out to I/O location
+00000015:   E5CF        LDI     R28,0x5F         Load immediate
+00000016:   E0D4        LDI     R29,0x04         Load immediate
+00000017:   BFDE        OUT     0x3E,R29         Out to I/O location
+00000018:   BFCD        OUT     0x3D,R28         Out to I/O location
+00000019:   E010        LDI     R17,0x00         Load immediate
+0000001A:   E6A0        LDI     R26,0x60         Load immediate
+0000001B:   E0B0        LDI     R27,0x00         Load immediate
+0000001C:   EBE0        LDI     R30,0xB0         Load immediate
+0000001D:   E0F0        LDI     R31,0x00         Load immediate
+0000001E:   C002        RJMP    PC+0x0003        Relative jump
+0000001F:   9005        LPM     R0,Z+            Load program memory and postincrement
+00000020:   920D        ST      X+,R0            Store indirect and postincrement
+00000021:   36A0        CPI     R26,0x60         Compare with immediate
+00000022:   07B1        CPC     R27,R17          Compare with carry
+00000023:   F7D9        BRNE    PC-0x04          Branch if not equal
+00000024:   E010        LDI     R17,0x00         Load immediate
+00000025:   E6A0        LDI     R26,0x60         Load immediate
+00000026:   E0B0        LDI     R27,0x00         Load immediate
+00000027:   C001        RJMP    PC+0x0002        Relative jump
+00000028:   921D        ST      X+,R1            Store indirect and postincrement
+00000029:   36A1        CPI     R26,0x61         Compare with immediate
+0000002A:   07B1        CPC     R27,R17          Compare with carry
+0000002B:   F7E1        BRNE    PC-0x03          Branch if not equal
+0000002C:   C01C        RJMP    PC+0x001D        Relative jump
+0000002D:   CFD2        RJMP    PC-0x002D        Relative jump
+0000002E:   921F        PUSH    R1               Push register on stack
+0000002F:   920F        PUSH    R0               Push register on stack
+00000030:   B60F        IN      R0,0x3F          In from I/O location
+00000031:   920F        PUSH    R0               Push register on stack
+00000032:   2411        CLR     R1               Clear Register
+00000033:   938F        PUSH    R24              Push register on stack
+00000034:   EF8F        SER     R24              Set Register
+00000035:   93800060    STS     0x0060,R24       Store direct to data space
+00000037:   918F        POP     R24              Pop register from stack
+00000038:   900F        POP     R0               Pop register from stack
+00000039:   BE0F        OUT     0x3F,R0          Out to I/O location
+0000003A:   900F        POP     R0               Pop register from stack
+0000003B:   901F        POP     R1               Pop register from stack
+0000003C:   9518        RETI                     Interrupt return
+0000003D:   BC13        OUT     0x23,R1          Out to I/O location
+0000003E:   E68A        LDI     R24,0x6A         Load immediate
+0000003F:   BD85        OUT     0x25,R24         Out to I/O location

Thanks,
nxp

Tags:

AVR Microcontrollers, megaAVR and tinyAVR

dksmall

Level: Raving Lunatic

Joined: Mon. Apr 16, 2001

Posts: 3684 View posts

Location: Phoenix, Arizona

Posted by dksmall: Wed. Sep 20, 2006 - 12:02 AM

The dump is useless. Unless you want to sit down with a datasheet and lookup the address of every register you're using. One thing I don't see is a reference to SPL and SPH, which is the stack register. If you did initialize them we can't tell without looking up the register addresses. If you didn't initialize the stack, that's where your problem is.

Koshchi

Level: 10k+ Postman

Joined: Wed. Nov 17, 2004

Posts: 15658 View posts

Location: Vancouver, BC

Posted by Koshchi: Wed. Sep 20, 2006 - 12:03 AM

Quote:

The timer keeps running but execution never goes from IRET to the main code, and in the disassembly window there is no address associated to the IRET, as shown below. [would the address be stored in an SFR somwhere?]

The return address is stored on the stack. I don't know how a RETI instruction could not return. Even if the stack is set up incorrectly, it would have to return somewhere.

Regards,
Steve A.

The Board helps those that help themselves.

nxp

Level: Rookie

Joined: Fri. Sep 8, 2006

Posts: 47 View posts

Location: Malta

Posted by nxp: Wed. Sep 20, 2006 - 07:41 AM

dksmall wrote:

One thing I don't see is a reference to SPL and SPH, which is the stack register. If you did initialize them we can't tell without looking up the register addresses. If you didn't initialize the stack, that's where your problem is.

As I mentioned in the original post, I am coding in C, not directly in assembler. I beleive that stack pointer initialization is the task of the compiler [in this case gcc], and in fact when I used an interrupt in a previous program the compiler took care of everything and I did not have to initialize the stack.

Am I correct or is my knowledge seriously wrong?

nxp

BenG

Level: Hangaround

Joined: Wed. May 3, 2006

Posts: 152 View posts

Location: Manchester, UK

Posted by BenG: Wed. Sep 20, 2006 - 07:52 AM

nxp wrote:

As I mentioned in the original post, I am coding in C, not directly in assembler. I beleive that stack pointer initialization is the task of the compiler [in this case gcc], and in fact when I used an interrupt in a previous program the compiler took care of everything and I did not have to initialize the stack.

Am I correct or is my knowledge seriously wrong?

I think your correct - perhaps an example of the ISR code (just the ISR if the rest of the code is too long) would help. It may well be a problem in your code that is causing the ISR not to return.

Ben
-Using IAR (& ocasionally CodeVision)
0.7734
1101111011000000110111101101

nxp

Level: Rookie

Joined: Fri. Sep 8, 2006

Posts: 47 View posts

Location: Malta

Posted by nxp: Wed. Sep 20, 2006 - 08:05 AM

So here is the original code. Most of the space is comments. The ISR is as minimal as possible, just the assignment of the flag variable to return to the main code ASAP [not essential here but good to use if I have a multi-interrupt system]. What do you think?

/*
 *
 * Simple AVR demonstration.
 * Reads in ADC value and sets pwm using Timer2 according the the most
 * significant 8 bits of the conversion [or use adc in 8-bit mode].
 *
 */

#include 
#include 
#include 
#include 

//Global variables
uint8_t  ADC_end_flag = 0;

ISR (ADC_vect)
{
//Set the global flag variable to show we have just completed an ADC conversion.
	ADC_end_flag = 0xff;
}


void
sysinit_mega8 (void)
{
/*
Timer 2 is an 8-bit counter with PWM capability. We will run it in PWM mode with
the value obtained from the ADC as a pulse width. Also a 10-bit prescaler is
available. Output of the ADC is left justified by setting the ADLAR  bit in the
ADMUX register.
We will use timer 2 in Fast PWM mode, that is WGM21-20 must be set to "11". Also
COM21-20 will be set to "10" so that the OC2 pin [PWM output] is set when the
timer reaches the maximum value, and is cleared when the timer reaches the
OCR2 value. Thus the duty cycle is directly proportional to OCR2.
The frequency will be 11.0592MHz/(256*prescale). We will use a prescale of 8,
for a frequency of 5.4KHz.
*/
//Set initial PWM value to 0
	OCR2 = 0b00000000;
//WGM21, WGM20 and COM21 should be set
    TCCR2 = 0b01101010;

//Setup output ports. OC2 pin is PB3
//Port B will have only pin 3 as output.
	DDRB = 0b00001000;
	PORTB = 0b00000000;

/*
For the ADC we will use a prescale value of 64, to get a clock of 172.8KHz.
Also we will use channel 0 for the input. We will use Vcc as the Vref and at the
moment we will not try using a noise canceller.
*/
//REFS1-0 must be "01" to use Avcc as the reference. ADLAR is "1" to left adjust
//the result, and MUX3-0 must be "0011" to chose channel 3.
	ADMUX = 0b01100011;
//Enable ADC, do not start conversion, do not enable free running mode, do not
//interrupt right now, enable ADC interrupt, select divider of 64, "110"
	ADCSRA = 0b10001110;

//Set sleep mode to "ADC noise reduction".
//set_sleep_mode (SLEEP_MODE_ADC);

//Enable Interrupts
    sei ();
}

int
main (void)
{

    sysinit_mega8 ();
    
//Start first conversion.
	ADCSRA |= 0b01000000;

//Loop forever, the timer interrupt is doing the rest
    for (;;)
//		sleep_mode();
		if (ADC_end_flag == 0xff)
		{
			ADC_end_flag = 0x00;
//Write to OCR2, that is set the desired pulse width.
//Note that we will run the ADC with the output register left justified, to take
//only the most significant eight bits [by reading only ADCH, ignoring ADCL].
			OCR2 = ADCH;
//Re-start converter
			ADCSRA |= 0b01000000;
		}
	
	return (0);
}

Jepael

Level: Raving Lunatic

Joined: Mon. May 24, 2004

Posts: 6270 View posts

Location: Tampere, Finland

Posted by Jepael: Wed. Sep 20, 2006 - 08:06 AM

May we see the C code? I think the problem is not at assembly level.

At least the the interrupt function will be helpful.

- Jani

BenG

Level: Hangaround

Joined: Wed. May 3, 2006

Posts: 152 View posts

Location: Manchester, UK

Posted by BenG: Wed. Sep 20, 2006 - 08:32 AM

I've run your code in AVR Studio Simulator and it works fine for me.

What you need to remeber Anlog input is not supported in the simulator. After running the code into the infinite for() loop. I set ADIF (bit 4 - ADCSRA register) and the ISR is called, executes and returns to the main body. In the simulator what happens after the line:

ADC_end_flag = 0xff;

- i.e. after pressing F11?

Ben
-Using IAR (& ocasionally CodeVision)
0.7734
1101111011000000110111101101

nxp

Level: Rookie

Joined: Fri. Sep 8, 2006

Posts: 47 View posts

Location: Malta

Posted by nxp: Wed. Sep 20, 2006 - 10:25 AM

Guess what?

The simulation is working! I am now using GCC from within AvrStudio, whereas before I was compiling with winavr and only taking the hex file for simulation. With everything in avrstudio the pwm duty cycle on the output pin came out correct, simulating the file from winavr gave no change on the output pin.

Definitely there was a software/compiler issue. Even the hex files produced are not identical, so that might have been the problem.

Also I am thinking of a possible hardware problem. I will check that out again.

Thanks a lot for the help Ben!

nxp

Level: Rookie

Joined: Fri. Sep 8, 2006

Posts: 47 View posts

Location: Malta

#10

Posted by nxp: Wed. Sep 20, 2006 - 11:04 AM

Ok, found another little bug, this time in my hardware. There was a dry solder joint, so the adc input was not being connected well to the pot I was using.

Now it all works ok!

lfmorrison

Level: Raving Lunatic

Joined: Wed. Dec 8, 2004

Posts: 4643 View posts

Location: Nova Scotia, Canada

#11

Posted by lfmorrison: Wed. Sep 20, 2006 - 11:20 AM

Quote:

Even the hex files produced are not identical, so that might have been the problem.

There are at least two possible reasons why the hex files weren't identical. (I think possibility #2 is the actual cause for the difference in your hex files, and I also think it gives some insight into the reason why you were seeing trouble in the first place.)

1) The makefile you were using while you were compiling from "WinAVR" (Programmer's Notepad??) was specifying the wrong MCU type.

2) The Programmer's Notepad makefile and the AVR Studio makefile are specifying two different optimization levels. The template makefile that comes with WinAVR specifies an optimization level of -Os (roughly equivalent to "optimize for size"), whereas the default setting in AVR Studio is -O0 (turns off all optimizations).

With the optimizer turned on, the compiler will look at this fragment of code:

for (;;) 
      if (ADC_end_flag == 0xff) 
      { 
         ADC_end_flag = 0x00; 
         OCR2 = ADCH; 
         ADCSRA |= 0b01000000; 
      }

and it will notice that there are no explicit function calls, etc. which could possibly cause the variable ADC_end_flag to change state. Therefore, it doesn't bother fetching the variable every time through the loop. Rather, it will fetch the variable once, just before the first time through the loop, and it will use this cached value for all subsequent iterations through the loop.

With the unoptimized version, the compiler will actually go through the motions of re-loading the variable's value each time and thus it will capture the change of state as soon as the interrupt is serviced.

The compiler doesn't know about "context switches", so it can't tell that the ISR function can temporarily take over control of the CPU and modify ADC_end_flag's state. It needs to be "reminded" that it is possible for ADC_end_flag to change states asynchronously by attaching the volatile type qualifier to the variable's declaration.

nxp

Level: Rookie

Joined: Fri. Sep 8, 2006

Posts: 47 View posts

Location: Malta

#12

Posted by nxp: Wed. Sep 20, 2006 - 12:15 PM

Hmm, I think you are right!

I completely forgot that variables used in multiple contexts should be volatile! In fact the optimization when using WinAvr was for size while I did not change the AvrStudio settings. I should not have done this mistake, since several months ago I wasted several hours trying to debug code in my DSP course, until the lecturer pointed out a simlar issue with volatiles. Hopefully I will not do it again!

As for makefile settings, what tool is recommended to generate a correct makefile for gcc? Currently I'm not using Programmer's Notepad just because I could not find any such tool within it. Instead I use KamAVR which is a simple GUI/project manager with a settings panel to build a makefile. I'd rather not fool around with makefiles manually at the moment since I'm still rather green with gcc and AVR's.

Great thanks. :)

nxp

lfmorrison

Level: Raving Lunatic

Joined: Wed. Dec 8, 2004

Posts: 4643 View posts

Location: Nova Scotia, Canada

#13

Posted by lfmorrison: Wed. Sep 20, 2006 - 12:28 PM

nxp wrote:

As for makefile settings, what tool is recommended to generate a correct makefile for gcc?

WinAVR includes a tool called MFile for makefile management. It should be listed in your Start Menu, under the WinAVR program group.

dksmall

Level: Raving Lunatic

Joined: Mon. Apr 16, 2001

Posts: 3684 View posts

Location: Phoenix, Arizona

#14

Posted by dksmall: Wed. Sep 20, 2006 - 04:02 PM

nxp wrote:

As I mentioned in the original post, I am coding in C, not directly in assembler.
nxp

Argg you stuck the Winavr comment between the 2 code snippets, I missed it, sorry about that. Looks like you got it under control though.

Jump To

Search form

Interrupt not returning!

Tags: