Assembly for loop repeating 2 times when it's only suppose to do it once I'm programming an Arduino Nano so I belive that this question should be qualified for this SE.

I am trying to dim up an LED using a for loop in AVR assembly.

My problem is that the cycle is repeated twice before ending, while it is suppose to end after the first cycle.

A cycle is when the red_loop is repeated 256 times (to dim the led up). At that point, the value should overflow causing the variable to go back to 0. The code should stop there, but mysteriously, it does another cycle and then it stops.

Does anyone know why?

Here is my code : 

.org 0x000

ldi r16, 0b111
out ddrb, r16

ldi r16, 0xff
out portb, r16

ldi r16, 0

; ff00 pin
; ff01 value
; ff02 led

ldi r16, 0b000
sts $ff00, r16

ldi r16, 0
sts $ff01, r16

red:
    ldi r16, 0b001
    sts $ff00, r16

red_loop:
    rcall pwm

    lds r16, $ff01
    inc r16
    sts $ff01, r16
    cpi r16, 0
    brne red_loop

end:
    rjmp end

pwm:
    ldi r24, 190

pwm_pre_loop:
    ldi r17, 0

pwm_loop:
    lds r16, $ff01
    cp r17, r16
    brge off

    cp r17, r16
    brlo on

pwm_loop_check:
    inc r17
    cpi r17, 0
    brne pwm_loop

    inc r24

    cpi r24, 0
    brne pwm_pre_loop

    ret

on:
    lds r16, $ff00

    sbrc r16, 0
    cbi portb, 0

    sbrc r16, 1
    cbi portb, 1

    sbrc r16, 2
    cbi portb, 2

    rjmp pwm_loop_check

off:
    lds r16, $ff00

    sbrc r16, 0
    sbi portb, 0

    sbrc r16, 1
    sbi portb, 1

    sbrc r16, 2
    sbi portb, 2

    rjmp pwm_loop_check
The problem is that the register r17, which you use as a PWM timing
ramp, overflows twice every time you enter pwm_pre_loop:


when you increments it past 127, it overflows to −128
when you increment it past 255, it overflows to 0.


When using a counter the way you are using r17 here, you usually have
it overflow only once per cycle: either you consider it holds a signed
number which overflows from 127 to −128, or you consider it holds an
unsigned number which overflows from 255 to 0. I guess the latter is
more common. Your code is confused about that register's signedness:


The instruction brge (named “Branch if Greater or Equal (Signed)” in
the instruction set datasheet) tests the S (sign flag) bit of the
status register, which is useful for signed comparisons.
The instruction brlo (“Branch if Lower (Unsigned)”) tests the carry
flag, which is used for unsigned comparisons.


You should decide once on the signedness of the counter, and then use it
consistently. Since here unsigned makes more sense, you can just replace
brge by brsh (“Branch if Same or Higher (Unsigned)”).

Or better yet, remove the second test, which is redundant:

pwm_loop:
    lds r16, value
    cp r17, r16
    brsh off  ; if (r17 &gt;= r16) goto off;
    rjmp on   ; else goto on;


Note by the way that you can name your RAM variables. And you can use
some of the other available registers instead of the RAM.