I want to generate 1 s delay for a program using pic16f887

Hello members,

I have written an alp for blinking led..so I have written a subroutine to generate delay of 1.7 s.But I guess there is some logical error...Can anyone debug it???

Here is a subroutine:
DELAY:
MOVLW 0XFF
MOVWF 0X20
LOOP: DECFSZ 0X20,1
CALL DELAY1
RETURN
DELAY1:
MOVLW 0XFF
MOVWF 0X21
LOOP1: DECFSZ 0X21,1
CALL DELAY2
GOTO LOOP1
RETURN
DELAY2:
MOVLW 0X3F
MOVWF 0X22
LOOP2: DECFSZ 0X22,1
GOTO LOOP2
RETURN

Is this logically correct???I mean the calculation...& also have a doubt that Iit may end up as an infinite loop..help me out plz...

That code is difficult to read, here is how to make it understandable:
1. use code tags around the code, the '#' in the bar above the message window. It tells Edaboard not to reformat it so we see it as you wrote it.
2. instead of using addresses, please use names. For example, at the top of the program use:
then use the name delay_1 instead of 0x20, delay_2 instead of 0x21 and delay_3 instead of 0x22. Names make it clearer what the variable is supposed to do.
3. instead of ending variables with ",0" or ",1" use ",W" if the result goes back in the W register and ",F" if it goes back to the file register.
So the code nw looks like:

4. software delays work by accumulating the time taken to execute each instruction but without knowing how long an instruction takes it is impossible to know the length of delay. You must tell us the clock speed you are using. Also note that other things, particularly interrupts can make the delay longer than expected.

Your code is not functionally incorrect but it isn't very efficicient. A better method is to group the three delays into one, basically remove the call and return instructions, except for the last return.

Brian.