anonymous
2015-01-31 17:05:10 UTC
URL:
<http://savannah.nongnu.org/bugs/?44140>
Summary: wdt_disable() macro clobbers prescaller bits and can
cause unexpected resets
Project: AVR C Runtime Library
Submitted by: None
Submitted on: Sat 31 Jan 2015 05:05:09 PM UTC
Category: Library
Severity: 3 - Normal
Priority: 5 - Normal
Item Group: None
Status: None
Percent Complete: 100%
Assigned to: None
Originator Email: ***@joshreply.com
Open/Closed: Open
Discussion Lock: Any
Release: Any
Fixed Release: None
_______________________________________________________
Details:
The wdt_disable() macro in wdt.h overwrites the prescaler bits as an
unintended side effect when setting the watchdog change enable bit.
The relevant line in wdt_disable() macro looks like this (expanded for
clarity)...
===
out WDTCSR, _BV(_WD_CHANGE_BIT) | _BV(WDE)
===
IN safely level 1 (WatchDog fuse not set), the above code writes 0's to the
WDTCSR register's lower 3 bits, which contain a prescaler for the counter.
When changing to a lower prescaler, it is possible to trigger an unintended
reset if the new prescaler has already expired.
The data sheet specially mentions this possibility in the comments for the
example code to disable the watchdog, and uses an OR to set the relevant bits
and preserve the prescaler bits...
===
; Write logical one to WDCE and WDE
; Keep old prescaler setting to prevent unintentional Watchdog Reset
in r16, WDTCSR
orir16, (1<<WDCE)|(1<<WDE)
out WDTCSR, r16
===
The problem can also be fixed with only one extra instruction/cycle and no
extra register use by placing a watchdog reset before the assignment. Since
interrupts are already disabled, this ensures that the new prescaler will not
have already tripped. Note that the watchdog is reset when it is disabled
anyway, so this should not have any side effects.
New macro with the reset strategy would look like...
===
__asm__ __volatile__ ( \
"in __tmp_reg__, __SREG__" "\n\t" \
"cli" "\n\t" \
"wdr" "\n\r" \
"out %0, %1" "\n\t" \
"out %0, __zero_reg__" "\n\t" \
"out __SREG__,__tmp_reg__" "\n\t" \
: /* no outputs */ \
: "I" (_SFR_IO_ADDR(_WD_CONTROL_REG)), \
"r" ((uint8_t)(_BV(_WD_CHANGE_BIT) | _BV(WDE))) \
: "r0" \
)
===
I have only tested this on an ATTINY84A, but the watchdog function is the same
in every 8-bit AVR I've checked.
Let me know if you have any questions.
Thanks!
-josh
_______________________________________________________
Reply to this item at:
<http://savannah.nongnu.org/bugs/?44140>
_______________________________________________
Message sent via/by Savannah
http://savannah.nongnu.org/
<http://savannah.nongnu.org/bugs/?44140>
Summary: wdt_disable() macro clobbers prescaller bits and can
cause unexpected resets
Project: AVR C Runtime Library
Submitted by: None
Submitted on: Sat 31 Jan 2015 05:05:09 PM UTC
Category: Library
Severity: 3 - Normal
Priority: 5 - Normal
Item Group: None
Status: None
Percent Complete: 100%
Assigned to: None
Originator Email: ***@joshreply.com
Open/Closed: Open
Discussion Lock: Any
Release: Any
Fixed Release: None
_______________________________________________________
Details:
The wdt_disable() macro in wdt.h overwrites the prescaler bits as an
unintended side effect when setting the watchdog change enable bit.
The relevant line in wdt_disable() macro looks like this (expanded for
clarity)...
===
out WDTCSR, _BV(_WD_CHANGE_BIT) | _BV(WDE)
===
IN safely level 1 (WatchDog fuse not set), the above code writes 0's to the
WDTCSR register's lower 3 bits, which contain a prescaler for the counter.
When changing to a lower prescaler, it is possible to trigger an unintended
reset if the new prescaler has already expired.
The data sheet specially mentions this possibility in the comments for the
example code to disable the watchdog, and uses an OR to set the relevant bits
and preserve the prescaler bits...
===
; Write logical one to WDCE and WDE
; Keep old prescaler setting to prevent unintentional Watchdog Reset
in r16, WDTCSR
orir16, (1<<WDCE)|(1<<WDE)
out WDTCSR, r16
===
The problem can also be fixed with only one extra instruction/cycle and no
extra register use by placing a watchdog reset before the assignment. Since
interrupts are already disabled, this ensures that the new prescaler will not
have already tripped. Note that the watchdog is reset when it is disabled
anyway, so this should not have any side effects.
New macro with the reset strategy would look like...
===
__asm__ __volatile__ ( \
"in __tmp_reg__, __SREG__" "\n\t" \
"cli" "\n\t" \
"wdr" "\n\r" \
"out %0, %1" "\n\t" \
"out %0, __zero_reg__" "\n\t" \
"out __SREG__,__tmp_reg__" "\n\t" \
: /* no outputs */ \
: "I" (_SFR_IO_ADDR(_WD_CONTROL_REG)), \
"r" ((uint8_t)(_BV(_WD_CHANGE_BIT) | _BV(WDE))) \
: "r0" \
)
===
I have only tested this on an ATTINY84A, but the watchdog function is the same
in every 8-bit AVR I've checked.
Let me know if you have any questions.
Thanks!
-josh
_______________________________________________________
Reply to this item at:
<http://savannah.nongnu.org/bugs/?44140>
_______________________________________________
Message sent via/by Savannah
http://savannah.nongnu.org/