Bugged AVR204 Atmel Application Note

1 post / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

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.

=====================
Now the problem:
$75+$85.
--------------------------------------
BCDadd:
ldi tmpadd,6

; 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 - wagner@ustr.net
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

admin's test signature