This is the Atmel AVR204 BCDADD Bug
The AVR204 Atmel Application Note is in error, routine
BCDADD.
This text is long and difficult, if you want to understand it, please
print it.
The BCDADD routine below uses a traditional DAA operation, common in other processors as 8051 and even 8086 family, to adjust the after-math add packed bcd routine, to a real decimal numbers.
In the 8051 Atmel books, the DAA operation is described as the following, but obviously it apply to ANY DAA performed by a instruction or by a software routine:
========
DAA adjusts the eight-bit value, resulting from
the earlier addition of two variables (each in Packed-BCD format), producing two
four-bit digits. Any Add or Addc operation may have been used to perform the
addition.
If result bits 4 through 0 are greater than nine (xxx1010-xxx1111) [rule#1], or if the H flag (in AVR) is one [rule#2], six is added to the result producing the proper BCD digit in the low-order nibble [action#1]. this internal addition sets the carry flag IF a carry-out of the low order four-bit field propagates [action#2], but it does not clear the carry flag otherwise.
If the carry flag is now set [rule#3] (here is where AVR204
fails), or if the four high-order bits now exceed nine (1010xxxx-1111xxxx)
[rule$4], these high-order bits are incremented by six [action#3], producing the
proper BCD digit in the high-order nibble. Again, this sets the carry flag if
these is a carry-out of the high-order bits [action#4], but does not clear the
carry. The carry flag thus indicate if the sum of the original two BCD
variables is greater than 100 [action#5 = action#3 OR action#4], allowing
multiple precision decimal addition.
========
The AVR204 just ignore the possible carry flag created at the second paragraph action#2. The third paragraph rule#3 does not observe a possible action#2, thus not doing the action#3 based on rule#3, not adding the correspondent $60, also failing action#4 and action#5.
It results in wrong values.
Lets exercise two cases through the BCDAdd routine:
Adding
$75 to $84 (ok), and $75 to $85 (fail).
First
$75+$84.
--------------------------------------
BCDadd:
ldi tmpadd,6
; BCD1: 0111-0101
; BCD2: 1000-0100
add BCD1,BCD2
; BCD1: 0111-0101 ($75)
; + 1000-0100 ($84)
;
= 1111-1001 ($F9)
; Carry bit: OFF (no byte overflow)
; H Flag: OFF (no
carry from low order 4 bits to high order 4 bits)
clr BCD2
brcc add_0
; Carry bit is OFF, so branch is executed, BCD2 still = 0
;
Entry Carry is OFF.
ldi BCD2,1
add_0:
brhs add_1
; Half Carry bit is OFF, so branch is not executed.
add BCD1,tmpadd
; Checking Rule#1 - if 4 low order bits are equal or lower
than 9
; This is a tricky way to check it. First add 6, if H flag sets, it
is
; because the original 4 bits were higher than 9, so Action#1 is in
;
effect. If the H flag does not go on, it means the original 4 low
; bits
were 9 or lower, so Action#1 does not apply and 6 should
; be removed from
low order bits of BCD1, as you can see in
; the next instructions.
;
;
Checking Rule#1
; BCD1: 1111-1001 ($F9)
; + 0000-0110
($06)
; = 1111-1111 ($FF)
brhs add_2
; Half Carry bit is off (no overflow from low to high order
bits)
; branch does not execute because Action#1 does not apply,
; thus, 6
should be subtracted from BCD1, leaving it as original.
; Here the failure
starts. The previous routine does not save the
; possible carry from the
addition (Action#2), so Rule#3 (below)
; will not be totally verified.
subi BCD1,6
; BCD1: 1111-1111 ($FF)
; - 0000-0110
($06)
; = 1111-1001 ($F9)
; Returning BCD1 as original, since Action#1 does not
apply.
; It also skips the possibility of Action#2.
rjmp add_2
; Skip the next add, BCD1 is ok with its own
value.
add_1: add BCD1,tmpadd
add_2: swap tmpadd
; TMPADD is swapped, nibbles reversed, from 0000-0110
($6)
; to 0110-0000 ($60), so now it will works on the high order bits.
;
Tmpadd = $60
add BCD1,tmpadd
; will do the same trick, add $60 and check Carry to see if it
; was equal or lower than $9X, checking Rule#3.
; BCD1: 1111-1001
($F9)
; + 0110-0000 ($60)
; = 1-0101-1001 ($159)
;
Carry bit ON (byte overflow)
; H flag Off (no overflow from 4 low bits to 4
high bits)
brcs add_4
; Again the trick, as the carry does go on, it means the
previous
; high 4 bits of BCD1 was higher than 9, in true it was $F
(1111)
; so it generated the carry. Rule#3 checks valid, Action#3
occurs,
; it will keeps the previous adding of $60 to BCD1.
; Branch to
ADD_4 is valid, BCD1 stays $59 with Carry Set
sbrs BCD2,0
subi BCD1,$60
add_3: ret
add_4: ldi BCD2,1
ret
;The routine ends with BCD1 as $59 and BCD2=1 means carry
set,
;final result is $159. - Everything is ok, 75+84=159.
; BCD1: 0111-0101
; BCD2: 1000-0101
add BCD1,BCD2
; BCD1: 0111-0101 ($75)
; + 1000-0101 ($85)
; = 1111-1010
($FA)
; Carry bit: OFF (No byte overflow)
; H Flag: OFF (no carry from
low order 4 bits to high order 4 bits)
clr BCD2
brcc add_0
; Carry bit is OFF, so "Entry Carry" is OFF.
; Branch IS executed
ldi BCD2,1
add_0: brhs add_1
; BCD2 = 0
; Half Carry bit is OFF, so branch is not executed.
add BCD1,tmpadd
; Checking Rule#1 - if 4 low order bits are higher than 9.
; This is a
tricky way to check it. First add 6, if H flag sets, it is
; because the
original 4 bits were higher than 9, so Action#1 is in
; effect. If the H
flag does not go on, it means the original 4 low
; bits were 9 or lower, so
Action#1 does not apply and 6 should
; be removed from low order bits of
BCD1.
; Lets see how it happens:
;
; Checking Rule#1
; BCD1:
1111-1010 ($FA)
; + 0000-0110 ($06)
; = 1-0000-0000
($00)
; H flag is ON
; Carry is ON <==== HERE IS THE BUG
; This
Carry Set (Action#2) is not saved to be checked at
; Rule#3 later on.
; In
fact this Carry SET already satisfies Rule#3, and should
; force to Add $6
to the high order 4 bits, but it is forgoten.
brhs add_2
; Half Carry bit is ON (overflow from low to high order bits)
; branch
does execute because Action#1 does apply,
; thus, BCD1 should keep the
previous addition of 6.
; Here the failure starts. The previous routine does
not save the
; possible carry from the addition (Action#2), so Rule#3
(below)
; will not be totally verified.
; Branch takes effect to
ADD_2
subi BCD1,6
rjmp add_2
add_1:
add BCD1,tmpadd
add_2: swap tmpadd
; TMPADD is swapped, nibbles reversed, from 0000-0110 ($6)
; to
0110-0000 ($60), so now it will works on the high order bits.
; Tmpadd =
$60
add BCD1,tmpadd
; will do the same trick, add $60 and check Carry to see if it
; was
equal or lower than $9X, checking Rule#3.
; BCD1: 0000-0000
($00)
; + 0110-0000 ($60)
; = 0110-0000 ($60)
; Carry bit
OFF (No byte overflow)
; H flag Off (no overflow from 4 low bits to 4 high
bits)
brcs add_4
; Again the trick, as the carry does NOT go on, it means the previous
;
high 4 bits of BCD1 was 9 or lower, in true it was $0 (0000)
; so it does not
generated the carry.
; Rule#3 checks invalid, Action#3 does not occurs, and
it SHOULD.
; It fails becaseu the Carry Set at Action#2 above fails to save
it.
; it should NOT remove the previous adding of $60 to BCD1.
; as that
Carry was not saved, this jump evaluate wrong and
; will subtract the $60
incorrectly.
; Branch to ADD_4 does NOT happens
sbrs BCD2,0
subi BCD1,$60
; BCD1: 0110-0000 ($60)
; - 0110-0000
($60)
; = 0000-0000 ($00) <=== INVALID
; Carry bit OFF (No byte
overflow)
; H flag Off (no overflow from 4 low bits to 4 high bits)
add_3: ret
; The Routine Ends bugged. It returns BCD1 = 00, it should be $60,
;
BCD2 still zero, reporting no carry, wrong again.
; The correct result of
75+85=160, but it returns 00.
add_4: ldi BCD2,1
ret
===============
This failure happens whenever the Packed BCD ADD results in values starting in $160 ($60 plus carry) up to $166, it depends on having the carry bit set by the first addition of value $6 to the four low bits in Action#1.
The fix appears below, I hope Atmel takes the correct measurements to announce this error and post the below fix to the Application Note AVR204.
Proper credit for the find out and bug fix suggestion will be very well appreciated.
Wagner Lipnharski
http://www.ustr.net
Suggestion #1
FIX for the BCDadd
Routine:
;***************************************************************************
;*
;*
"BCDadd" - 2-digit packed BCD addition
;* Fixed by Wagner Lipnharski
- wagner@ustr.net -
Mar/2002
;*
;* This subroutine adds the two unsigned 2-digit BCD
numbers
;* "BCD1" and "BCD2". The result is returned in "BCD1", and the
overflow
;* carry in "BCD2".
;*
;* Number of words :23|21
(with|without BCD2 as carry at exit)
;* Number of cycles :??/??
(Min/Max)
;* Low registers used :None
;* High registers used :2
(BCD1,BCD2)
;* T Flag used as a temporary internal Carry
Flag
;* On Return, Carry Flag represents the real valid
status of the Add.
;* Variable "tmpadd" eliminated, using BCD2 instead.
;* If the caller routine can consider the real Carry Bit as the
actual
; carry, instead of the BCD2, then two instructions can be
eliminated;
; the CLR BCD2 and LDI BCD2,1 near the end.
;***************************************************************************
;***** Subroutine Register Variables
.def BCD1 =r16 ;BCD input value #1
.def BCD2 =r17 ;BCD input value
#2
;***** Code
BCDAdd: CLT ;FIX BUG - Clear Temp
Carry
add BCD1,BCD2 ;add the numbers
binary
ldi BCD2,6 ;value to be added later
brcc add_0 ;if carry not clear
Set ;Save Intermediary Carry
add_0: brhs add_1 ;if half carry not set
add
BCD1,BCD2 ; add 6 to LSD
brcc add_5 ;FIX
BUG is carry clear?
Set ;FIX BUG If not Save
it (Fix Bug)
add_5: brhs add_2 ; if half carry not
set (LSD <= 9)
subi BCD1,6 ; restore
value
rjmp add_2 ;else
add_1: add BCD1,BCD2
; add 6 to LSD
add_2: swap BCD2
add BCD1,BCD2 ;add
6 to MSD
brcs add_4 ;if carry not set (MSD <=
9)
brts add_4 ;FIX BUG
subi BCD1,$60 ; restore value
clr BCD2 ; clear secondary BCD
carry
add_3: ret ;else
add_4: ldi BCD2,1 ;
set secondary BCD carry
sec ; set Carry Flag for
multi-byte bcd add
ret
--------------------------------------------------------