0
\$\begingroup\$

I am playing around with an AVR MCU. I am having trouble toggling a LED. It's being toggled faster than I have expected. It should be toggling at 500hz, but it's up to 600hz and not very stable. I have double checked, triple checked. God I even went through all the assembly code. No, I can't find the bug whatsoever.

Atmega8.elf:     file format elf32-avr

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 .text         000000ae  00000000  00000000  00000074  2**1
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  1 .bss          00000001  00800060  00800060  00000122  2**0
                  ALLOC
  2 .comment      00000030  00000000  00000000  00000122  2**0
                  CONTENTS, READONLY
  3 .debug_aranges 00000040  00000000  00000000  00000152  2**0
                  CONTENTS, READONLY, DEBUGGING
  4 .debug_info   00000131  00000000  00000000  00000192  2**0
                  CONTENTS, READONLY, DEBUGGING
  5 .debug_abbrev 000000d0  00000000  00000000  000002c3  2**0
                  CONTENTS, READONLY, DEBUGGING
  6 .debug_line   0000017e  00000000  00000000  00000393  2**0
                  CONTENTS, READONLY, DEBUGGING
  7 .debug_frame  0000005c  00000000  00000000  00000514  2**2
                  CONTENTS, READONLY, DEBUGGING
  8 .debug_str    00000157  00000000  00000000  00000570  2**0
                  CONTENTS, READONLY, DEBUGGING
  9 .debug_loc    00000049  00000000  00000000  000006c7  2**0
                  CONTENTS, READONLY, DEBUGGING
 10 .debug_ranges 00000020  00000000  00000000  00000710  2**0
                  CONTENTS, READONLY, DEBUGGING

Disassembly of section .text:

00000000 <__vectors>:
   0:   12 c0           rjmp    .+36        ; 0x26 <__ctors_end>
   2:   21 c0           rjmp    .+66        ; 0x46 <__bad_interrupt>
   4:   20 c0           rjmp    .+64        ; 0x46 <__bad_interrupt>
   6:   3a c0           rjmp    .+116       ; 0x7c <__vector_3>
   8:   1e c0           rjmp    .+60        ; 0x46 <__bad_interrupt>
   a:   1d c0           rjmp    .+58        ; 0x46 <__bad_interrupt>
   c:   1c c0           rjmp    .+56        ; 0x46 <__bad_interrupt>
   e:   1b c0           rjmp    .+54        ; 0x46 <__bad_interrupt>
  10:   1a c0           rjmp    .+52        ; 0x46 <__bad_interrupt>
  12:   19 c0           rjmp    .+50        ; 0x46 <__bad_interrupt>
  14:   18 c0           rjmp    .+48        ; 0x46 <__bad_interrupt>
  16:   17 c0           rjmp    .+46        ; 0x46 <__bad_interrupt>
  18:   16 c0           rjmp    .+44        ; 0x46 <__bad_interrupt>
  1a:   15 c0           rjmp    .+42        ; 0x46 <__bad_interrupt>
  1c:   14 c0           rjmp    .+40        ; 0x46 <__bad_interrupt>
  1e:   13 c0           rjmp    .+38        ; 0x46 <__bad_interrupt>
  20:   12 c0           rjmp    .+36        ; 0x46 <__bad_interrupt>
  22:   11 c0           rjmp    .+34        ; 0x46 <__bad_interrupt>
  24:   10 c0           rjmp    .+32        ; 0x46 <__bad_interrupt>

00000026 <__ctors_end>:
  26:   11 24           eor r1, r1
  28:   1f be           out 0x3f, r1    ; 63
  2a:   cf e5           ldi r28, 0x5F   ; 95
  2c:   d4 e0           ldi r29, 0x04   ; 4
  2e:   de bf           out 0x3e, r29   ; 62
  30:   cd bf           out 0x3d, r28   ; 61

00000032 <__do_clear_bss>:
  32:   20 e0           ldi r18, 0x00   ; 0
  34:   a0 e6           ldi r26, 0x60   ; 96
  36:   b0 e0           ldi r27, 0x00   ; 0
  38:   01 c0           rjmp    .+2         ; 0x3c <.do_clear_bss_start>

0000003a <.do_clear_bss_loop>:
  3a:   1d 92           st  X+, r1

0000003c <.do_clear_bss_start>:
  3c:   a1 36           cpi r26, 0x61   ; 97
  3e:   b2 07           cpc r27, r18
  40:   e1 f7           brne    .-8         ; 0x3a <.do_clear_bss_loop>
  42:   02 d0           rcall   .+4         ; 0x48 <main>
  44:   32 c0           rjmp    .+100       ; 0xaa <_exit>

00000046 <__bad_interrupt>:
  46:   dc cf           rjmp    .-72        ; 0x0 <__vectors>

00000048 <main>:
#include "INTERRUPT.h"


int main(void){

DDRD= 0x0f;   // LED PD0:3 to OUTPUT 
  48:   8f e0           ldi r24, 0x0F   ; 15
  4a:   81 bb           out 0x11, r24   ; 17
PORTD= 0x0f;  // Active LOW, turn them OFF
  4c:   82 bb           out 0x12, r24   ; 18

// 8000000/256=31250/(OCR2+1)31=1000hz  1/1000= 1ms
TCCR2 = (1 << CS22) | (1<<CS21) | (1<<WGM21);  
  4e:   8e e0           ldi r24, 0x0E   ; 14
  50:   85 bd           out 0x25, r24   ; 37
OCR2=30;
  52:   8e e1           ldi r24, 0x1E   ; 30
  54:   83 bd           out 0x23, r24   ; 35

TIMSK |= 1<< OCIE2; // Enable Compare Match Interrupt 
  56:   89 b7           in  r24, 0x39   ; 57
  58:   80 68           ori r24, 0x80   ; 128
  5a:   89 bf           out 0x39, r24   ; 57
sei();              // Enable Global Interrupt, SREG |= ( 1 << 7);
  5c:   78 94           sei

  while (1){
      PORTD ^= (1<<3);
  5e:   98 e0           ldi r25, 0x08   ; 8


      // 1000/200=5/2=2.5hz;
      if ( counter >=200 ) { PORTD ^= (1<<1); counter=0; }     
  60:   22 e0           ldi r18, 0x02   ; 2

TIMSK |= 1<< OCIE2; // Enable Compare Match Interrupt 
sei();              // Enable Global Interrupt, SREG |= ( 1 << 7);

  while (1){
      PORTD ^= (1<<3);
  62:   82 b3           in  r24, 0x12   ; 18
  64:   89 27           eor r24, r25
  66:   82 bb           out 0x12, r24   ; 18


      // 1000/200=5/2=2.5hz;
      if ( counter >=200 ) { PORTD ^= (1<<1); counter=0; }     
  68:   80 91 60 00     lds r24, 0x0060
  6c:   88 3c           cpi r24, 0xC8   ; 200
  6e:   c8 f3           brcs    .-14        ; 0x62 <main+0x1a>
  70:   82 b3           in  r24, 0x12   ; 18
  72:   82 27           eor r24, r18
  74:   82 bb           out 0x12, r24   ; 18
  76:   10 92 60 00     sts 0x0060, r1
  7a:   f3 cf           rjmp    .-26        ; 0x62 <main+0x1a>

0000007c <__vector_3>:
#include "INTERRUPT.h"

volatile uint8_t counter;

// 8000000/256=31250/31=1000hz
ISR (TIMER2_COMP_vect){
  7c:   1f 92           push    r1
  7e:   0f 92           push    r0
  80:   0f b6           in  r0, 0x3f    ; 63
  82:   0f 92           push    r0
  84:   11 24           eor r1, r1
  86:   8f 93           push    r24
  88:   9f 93           push    r25
    counter++;
  8a:   80 91 60 00     lds r24, 0x0060
  8e:   8f 5f           subi    r24, 0xFF   ; 255
  90:   80 93 60 00     sts 0x0060, r24
    PORTD ^= (1<<0);   // @1khz Toggle First LED
  94:   92 b3           in  r25, 0x12   ; 18
  96:   81 e0           ldi r24, 0x01   ; 1
  98:   89 27           eor r24, r25
  9a:   82 bb           out 0x12, r24   ; 18

}
  9c:   9f 91           pop r25
  9e:   8f 91           pop r24
  a0:   0f 90           pop r0
  a2:   0f be           out 0x3f, r0    ; 63
  a4:   0f 90           pop r0
  a6:   1f 90           pop r1
  a8:   18 95           reti

000000aa <_exit>:
  aa:   f8 94           cli

000000ac <__stop_program>:
  ac:   ff cf           rjmp    .-2         ; 0xac <__stop_program>
\$\endgroup\$
14
  • \$\begingroup\$ how can you tell that it is toggling at 600 Hz? \$\endgroup\$
    – jsotola
    Apr 6, 2020 at 6:37
  • \$\begingroup\$ @jsotola I am a robot from future, and my eye is so fast that I can count the flashing LED. Not really, I have a frequency counter. \$\endgroup\$
    – user83582
    Apr 6, 2020 at 6:39
  • 3
    \$\begingroup\$ From your attitude I can only assume that you are not really interested in an answer but simply want to troll around. I guess you won't get many helpful answers ;) \$\endgroup\$
    – jusaca
    Apr 6, 2020 at 7:00
  • \$\begingroup\$ Why do you have "// 8000000/256=31250/(OCR2+1)31=1000hz 1/1000= 1ms OCR2=30;" aren't you setting the interrupt to run at 500 Hz ( 2ms)? \$\endgroup\$
    – jDAQ
    Apr 6, 2020 at 7:01
  • \$\begingroup\$ @jDAQ 8000000/256=31250/(OCR2+1)31=1000hz or 1ms, not 2ms. Toggling a LED requires 1 ON and 1OFF, so 1000/2=500hz. \$\endgroup\$
    – user83582
    Apr 6, 2020 at 7:04

2 Answers 2

5
\$\begingroup\$

In the main loop the LED on PORTD is operated with read-modify-write operation. The interrupt also does a read-modify-write operation to PORTD. If the interrupt happens after when main loop has read PORTD and before writing back new PORTD value, the interrupt does happen and modify PORTD, but the main loop then continues and overwrites the modification done in the interrupt.

And then maybe on the next interrupt the LED should turn off, but because main loop left it off, it is turned on again because the port state is off, the frequency counter just sees these as extra edges and only reports the frequency based on edges. An oscilloscope would show there are short pulses happening due to the interrupts, so this explains why the result measured with frequency counter is off from what you expect - the measurement of the signal is correct, but it's not the correct square wave signal you expect.

So toggle the bits atomically. If SBI and CBI are not available, disable and enable interrupts while changing the PORTD in main loop. Or use the pin toggling hardware functionality by writing PIND.

\$\endgroup\$
6
  • \$\begingroup\$ sorry boo, wrong answer. My interrupt is used to both toggle PD0 and add a number to counter, so it does jumps to the interrupt vector. Do you even know how to read assembly, mate? :: TIMER2_COMP_vect >>> SIG_OUTPUT_COMPARE2 >>> Timer/Counter2 Compare Match >>> AT90S4434, AT90S8535, AT90CAN128, AT90CAN32, AT90CAN64, ATmega103, ATmega128, ATmega16, ATmega161, ATmega162, ATmega163, ATmega165, ATmega165P, ATmega169, ATmega169P, ATmega32, ATmega323, ATmega325, ATmega3250, ATmega3250P, ATmega329, ATmega3290, ATmega3290P, ATmega64, ATmega645, ATmega6450, ATmega649, ATmega6490, ATmega8, ATmega8535 \$\endgroup\$
    – user83582
    Apr 6, 2020 at 7:09
  • \$\begingroup\$ @Marcus Müller thank you for your feedback mate. I know there are people making friend in this platform. I am sorry for making it less friendly for you folks. Please accept my sincerest apology. \$\endgroup\$
    – user83582
    Apr 6, 2020 at 7:37
  • \$\begingroup\$ hopefully after 20 years you have stopped programming AVRs in assembly by now or did you stop 20 minutes after you had started 20 years ago? You do sound very experienced, but COMPA, COMPB and COMP are in the same update, so if you know COMPA and COMPB , you should have known COMP too, so if blaming on the updates or inconsistent between devices makes you feel better please do, especially after I have hurt your feeling already, and I am very sorry for that. \$\endgroup\$
    – user83582
    Apr 6, 2020 at 8:06
  • 1
    \$\begingroup\$ I'd say this is very likely the bug. You should never modify a GPIO port from an ISR while modifying it from the main program at the same time, that's a race condition bug. You can easily verify if this is the case by surrounding the PORTD ^= (1<<3); in main() (which you can see from the asm is not atomic) with cli(); PORTD ^= (1<<3); sei();. \$\endgroup\$
    – Lundin
    Apr 6, 2020 at 10:28
  • 1
    \$\begingroup\$ And yep you would see this bug straight away with a proper scope. It would manifest itself as short unexplained glitches instead of the expected duty cycle. Been there, done this bug myself, very hard to track down. \$\endgroup\$
    – Lundin
    Apr 6, 2020 at 10:32
0
\$\begingroup\$
/*
Fun with AVR Interrupt Example (atmega8)
Using Timer2 compare with CPU clock @ 8000000UL
Toggle a LED in various stages to demonstrate interrupt concept and potential bugs
Connected 4 LED to PD0:3;

April. 05. 2020
 */

#include <avr/io.h>
#include <avr/interrupt.h>

volatile uint8_t counter;

int main(void){

    DDRD= 0x0f;   // LED PD0:3 to OUTPUT
    PORTD= 0x0f;  // Active LOW, turn them OFF

    // 8000000/256=31250/(OCR2+1)31=1000hz  1/1000= 1ms
    TCCR2 = (1 << CS22) | (1<<CS21) | (1<<WGM21);
    OCR2=30;

    TIMSK = (1<< OCIE2); // Enable Compare Match Interrupt
    sei();              // Enable Global Interrupt, SREG |= ( 1 << 7);

    while (1){
        // Remove this line will increase PD0 toggling stability a lot
        PORTD ^= (1<<3); 

        // remove this line also will make PD0 solid stable
        if ( counter >=200 ) { PORTD ^= (1<<1); counter=0; }  

    }
}


// 8000000/256=31250/31=1000hz
ISR (TIMER2_COMP_vect){

    PORTD ^= (1<<0);
    counter++;

}

Bug: 100 extra toggles with PD0, the interrupt toggle.

      PORTD ^= (1<<3);
  62:   82 b3           in  r24, 0x12   ; 18
  64:   89 27           eor r24, r25
  66:   82 bb           out 0x12, r24   ; 18


      if ( counter >=200 ) { PORTD ^= (1<<1); counter=0; }     
  68:   80 91 60 00     lds r24, 0x0060
  6c:   88 3c           cpi r24, 0xC8   ; 200
  6e:   c8 f3           brcs    .-14        ; 0x62 <main+0x1a>
  70:   82 b3           in  r24, 0x12   ; 18
  72:   82 27           eor r24, r18
  74:   82 bb           out 0x12, r24   ; 18

  94:   92 b3           in  r25, 0x12   ; 18
  96:   81 e0           ldi r24, 0x01   ; 1
  98:   89 27           eor r24, r25
  9a:   82 bb           out 0x12, r24   ; 18

////////////////////////////////////////////////////////
r24 is "corrupted" while jumping out of the interrupt subroutine, 
and it's being written to PORTD 8-bits at a time, thus the extra toggles
and making it very unstable.

  66:   82 bb           out 0x12, r24   ; 18
  74:   82 bb           out 0x12, r24   ; 18
  9a:   82 bb           out 0x12, r24   ; 18

>>PORTD= 1001
  62:   82 b3           in  r24, 0x12   ; 18
  r24= 1001

  ### Interrupt ###
  94:   92 b3           in  r25, 0x12   ; 18
  r25= 1001

  96:   81 e0           ldi r24, 0x01   ; 1
  98:   89 27           eor r24, r25
  r24= 1000
  9a:   82 bb           out 0x12, r24   ; 18
>>PORTD=1000
  ### Interrupt ###

  r24= 1001
  64:   89 27           eor r24, r25
  r24= 0001

  66:   82 bb           out 0x12, r24   ; 18
>>PORTD=0001  ( An extra toggle for PD0)

Solution 1: this force the compiler to use toggle 1-bit instead of 8-bits instruction.

 if( PORTD & (1<<3) ) PORTD &= ~(1<<3);
 else PORTD |= (1<<3);

/////////////////////////////////////////////////////
     if( PORTD & (1<<3) ) PORTD &= ~(1<<3);
  5e:   93 9b           sbis    0x12, 3 ; 18
  60:   02 c0           rjmp    .+4         ; 0x66 <main+0x1e>
  62:   93 98           cbi 0x12, 3 ; 18
  64:   01 c0           rjmp    .+2         ; 0x68 <main+0x20>
     else PORTD |= (1<<3);
  66:   93 9a           sbi 0x12, 3 ; 18

Solution 2: Disable interrupt during the toggle

 cli();
 PORTD ^= (1<<3); 
 sei();

/////////////////////////////////////////////////////
  cli();
  60:   f8 94           cli
     PORTD ^= (1<<3); 
  62:   82 b3           in  r24, 0x12   ; 18
  64:   89 27           eor r24, r25
  66:   82 bb           out 0x12, r24   ; 18
  sei();
  68:   78 94           sei
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.