There are some problems with your code. I will point them out in order of which I think they may be the actual problem here. The ones at the bottom are just best-practice tips.
Initalise PORTC
You never initialise PORTC
. The value of PORTC
is unknown on a Power-On Reset. You will have to set it to some value in your initial
routine:
movlw 0x01
movwf PORTC
This sets it to have one LED on, which is probably what you want. After that, with RLF
, you will start rotating this bit to light other LEDs.
It may be that this is the idea behind these lines in blink
:
movlw B'00000001'
rlf PORTC,F
However, the first line doesn't have any effect: it stores 0b00000001
in W, but W is never used. RLF
takes register PORTC
, rotates it and stores it back in PORTC
- W isn't used. Also, you execute movlw B'00000001'
every time you loop through blink
. Even if this would correct, than the value of PORTC
would never change because its reset all the time: you have to initialise the port in the initial
routine, and change it in the blink
routine.
Call and goto
Blink ;flip the LED on or off
movlw B'00000001'
rlf PORTC,F
call Delay
call Blink
There are three different types of instructions to jump in code: GOTO
, CALL
and RETURN
.
With GOTO
, you simply jump in the code - it's as easy as that.
With CALL
, you implement GOTO
but also push the current program counter on the stack. This stack is a LIFO (last in first out) memory module in which in this case program locations can be stored.
With RETURN
, you pop the last element from the stack and jump to that location. Essentially you jump to the instruction after the last executed CALL
. This is what you use in the beginning where you call to Initial
and then return and call to Blink
.
The stack of this chip has eight levels, as described in section 2.3.2 of the datasheet. That means you can push a maximum of eight program locations on this stack. After that, the first index is overwritten (the stack is implemented as a circular buffer). This essentially means that it is then not possible to go back to the first CALL
instruction, and that the controller will jump to another position. This may cause many unexplainable problems in more advanced software.
With the code I quoted above, you continuously call Blink
, but there is no RETURN
instruction. This means you will keep pushing stuff to the stack without popping it. The stack is being overwritten all the time. Since you don't use RETURN
there, it doesn't matter so much. But in this situation you should really use GOTO
instead of CALL
:
Blink ;flip the LED on or off
movlw B'00000001'
rlf PORTC,F
call Delay
goto Blink
Since GOTO
doesn't use the stack, this is no problem.
Org 0 and 4
You've put lots of stuff on org 0
. This is not good practice. As can be read in section 2.1 of the datasheet, this chip has an interrupt vector at org 4
. This means that when an interrupt occurs, the program counter will jump to location 4, almost directly after 0. This is why we normally implement just a GOTO
instruction on org 0
. Something like this:
org 0
goto start
org 4
retfie ; return from interrupt (alternatively you could have
; an interrupt handler here)
start:
; your main code...
Explicit radices
As I already mentioned in the comments: in the top of your program you stated radix hex
(and this is also the default). Because of this you can use EQU 20
to select register 0x20
. However, it's much clearer when you just use EQU 0x20
or EQU 20h
. When others read your code, they don't have to search for the radix specification or the assembler's default.
Delay generator
Maybe you know already about this, but you may find this interesting: http://www.piclist.com/techref/piclist/codegen/delay.htm
This is a delay generator which will generate a delay of a specific time for you. Your delay routine is perfect as far as I can see now, but if you're ever looking for something that calculates it exactly, here you go!