Make LED blink using AVR assembly

Question

I have an ATmega328P Arduino and am trying to make the LED blink using assembly without any additional includes. The goal is to have it blink for 1 second on, then 1 second off, however, it blinks incredibly quickly and is barely perceptible. Am I doing something incorrectly?

; DDRB       = 0x04
; PORTB      = 0x05
; TCCROB     = 0x25
; TCNT0      = 0x26
; 0b00000000 = OFF
; 0b00100000 = ON

setup:
    ldi r16, 0b00100000     ; Set r16 to LED ON value
    out 0x04, r16           ; Set DDRB to output
    out 0x05, r16           ; Set PORTB to ON
    ldi r16, 0b00000101     ; Set r16 with prescaler 1024 value
    out 0x25, r16           ; Set the TCCROB to 1024
    ldi r20, 0              ; Reset r20
    ldi r21, 1              ; Hold value 1 to increment r20
loop:
    in r17, 0x26            ; Read the timer 
    cpi r17, 128            ; Check to see if the timer is halfway done (1/2 second, max 255 value)
    brbc 0, increment       ; If r17 <= 128 (max 255 val before resets to 0), invert current LED state
    rjmp loop
increment:
    adc r20, r21            ; Add r21 (1) to r20 (with carry)
    brbs 0, reset           ; If the carry branch is set, reset the register
    rjmp loop               ; Otherwise, run the clock
reset:
    ldi r20, 0              ; Reset the delay register
    cpi r16, 0b00000000     ; Check if r16 is set to LED OFF
    brbs 1, switchon        ; If it is, change it to ON
    ldi r16, 0b00000000     ; Otherwise, change it to OFF
    out 0x05, r16           ; Set the LED to the new state
    rjmp loop               ; Run the clock
switchon:
    ldi r16, 0b00100000     ; Set r16 to LED ON value
    out 0x05, r16           ; Set the LED to the new state
    rjmp loop               ; Run the clock

Answer 1

There is a bug with the logic of your program: once TCNT0 gets larger than 128, it will continue to be larger than 128 for quite some time... during which time you are continuously incrementing your r20 counter. The simplest fix would be to reset TCNT0 right after the increment label, with something like out TCNT0, zero_reg. Then you may want to compare TCNT0 with 61 instead of 128, in order to get closer to the desired blink frequency of 0.5 Hz.

Other than that, there are quite a few problems in the way you write your code. jwpat7 has already pointed out some of them, and here are more:

• You should not increment a register with adc, but with plain add. Both instructions update the carry bit, but adc also adds the previous value of the carry bit to the target register.
• Better yet, increment with inc, as it's purpose is more obvious. But then you should test the zero bit, as inc does not update the carry bit.
• There is little point in resetting r20 once it has overflowed, as it is zero at this point.

There are also quite a few ways the program could me made simpler. The simplest would probably be to configure timer 1 to deliver an interrupt every second, and toggle the LED in the ISR. If you do not want interrupts, you can configure counter 1 in much the same way, only without the “interrupt enable” bit, and check the interrupt flag in a busy loop.

Below is a slightly simplified version of your program. I tried to keep it as close as possible to yours, without using timer 1, nor interrupts, and not even the interrupt flag. The code checks for timer 0 overflowing by comparing its current value with the previous one.

.set PINB,     0x03
.set DDRB,     0x04
.set TCCR0B,   0x25
.set TCNT0,    0x26
.set LED_MASK, 0b00100000
.set PS_1024,  0b00000101

setup:
    ldi r16, PS_1024    ; Set r16 with prescaler 1024 value
    out TCCR0B, r16     ; Set the TCCROB to 1024
    ldi r16, LED_MASK   ; Set r16 to the LED bit
    out DDRB, r16       ; Set LED pin to output
    clr r18             ; Clear the saved timer
loop:
    ldi r20, 61         ; Initialize our software counter
check_timer:
    in r17, TCNT0       ; Read the timer
    cp r17, r18         ; Compare with previous value
    mov r18, r17        ; Save current value
    brsh check_timer    ; unless the timer has decreased, repeat
decrement:
    dec r20             ; decrement the software counter
    brne check_timer    ; if not zero, go back to checking the timer
toggle:
    out PINB, r16       ; toggle the LED
    rjmp loop

If we can use the timer overflow interrupt flag, then the test is simpler, and we do not need to store the previous and current values of the timer in registers:

check_timer:
    sbis TIFR0, TOV0    ; unless the timer has overflowed
    rjmp check_timer    ; check again for overflow
    sbi TIFR0, TOV0     ; otherwise clear the overflow flag

Answer 2

It appears that your intent is to set TCCROB up with a 1024x prescale, and each time it counts up to 128, increment r20, and toggle the LED each time r20 overflows to zero.

That sequence ought to take 1024/16M · 128 · 256 seconds, or about 2.1 seconds, a fairly leisurely toggle rate. If instead it's flashing a hundred or more times too fast, probably one of the comparisons is backwards.

Some of the following items might make it easier to debug the problem:

• Instead of brbc 0,..., say brcc ...
• Instead of brbs 0,..., say brcs ...
• Instead of brbs 1,..., say breq ...

That is, use proper mnemonic branch instruction names, rather than referencing SREG bits.

• If my arithmetic was correct and your branch tests were fixed, your rate would be off by a factor of two. To fix this problem, you could replace one of the more problematic tests (testing for carry clear after cpi r17, 128) with cpi r17, 0 followed by a breq, and change the prescale factor to 256 instead of 1024, which gives 256/16M · 256 · 256 seconds, or about 1.05 seconds.

• Use named constants for register names instead of 0x04, 0x05, 0x25, and 0x26.

• Instead of your lengthy toggle sequence use EOR to toggle the value. That is, replace

    cpi r16, 0b00000000     ; Check if r16 is set to LED OFF
    brbs 1, switchon        ; If it is, change it to ON
    ldi r16, 0b00000000     ; Otherwise, change it to OFF
    out 0x05, r16           ; Set the LED to the new state
    rjmp loop               ; Run the clock
switchon:
    ldi r16, 0b00100000     ; Set r16 to LED ON value
    out 0x05, r16           ; Set the LED to the new state
    rjmp loop               ; Run the clock

with

    eor   r18, r16          ; Toggle LED bit
    out   PORTB, r18        ; Set the LED to the new state
    rjmp  loop              ; Go run the clock

where r16 and r18 are initialized as follows in setup:

    ldi r16, 0b00100000     ; bit mask for LED pin
    mov r18, r16