Why does my AVR reset when I call wdt_disable() to try to turn the watchdog timer off?

Question

I am having a problem where executing a disable watchdog sequence on an AVR ATtiny84A is actually resetting the chip even though the timer should have plenty of time left on it. This happens inconsistently and when running the same code on many physical parts; some reset every time, some reset sometimes, and some never.

To demonstrate the problem, I have written a simple program which...

1. Enables the watchdog with a 1 second timeout
2. Resets the watchdog
3. Flashes the white LED on for 0.1 seconds
4. Flashed the white LED off for 0.1 seconds
5. Disables the watchdog

The total time between the watchdog enable and disable is less than 0.3 seconds, yet sometimes a watchdog reset occurs when the disable sequence is executed.

Here is the code:

#define F_CPU 1000000                   // Name used by delay.h. We are running 1Mhz (default fuses)

#include <avr/io.h>
#include <util/delay.h>
#include <avr/wdt.h>


// White LED connected to pin 8 - PA5

#define WHITE_LED_PORT PORTA
#define WHITE_LED_DDR DDRA
#define WHITE_LED_BIT 5


// Red LED connected to pin 7 - PA6

#define RED_LED_PORT PORTA
#define RED_LED_DDR DDRA
#define RED_LED_BIT 6


int main(void)
{
    // Set LED pins to output mode

    RED_LED_DDR |= _BV(RED_LED_BIT);
    WHITE_LED_DDR |= _BV(WHITE_LED_BIT);


    // Are we coming out of a watchdog reset?
    //        WDRF: Watchdog Reset Flag
    //        This bit is set if a watchdog reset occurs. The bit is reset by a Power-on Reset, or by writing a
    //        logic zero to the flag

    if (MCUSR & _BV(WDRF) ) {

        // We should never get here!


        // Light the RED led to show it happened
        RED_LED_PORT |= _BV(RED_LED_BIT);

        MCUCR = 0;        // Clear the flag for next time
    }

    while(1)
    {
        // Enable a 1 second watchdog
        wdt_enable( WDTO_1S );

        wdt_reset();          // Not necessary since the enable macro does it, but just to be 100% sure

        // Flash white LED for 0.1 second just so we know it is running
        WHITE_LED_PORT |= _BV(WHITE_LED_BIT);
        _delay_ms(100);
        WHITE_LED_PORT &= ~_BV(WHITE_LED_BIT);
        _delay_ms(100);

        // Ok, when we get here, it has only been about 0.2 seconds since we reset the watchdog.

        wdt_disable();        // Turn off the watchdog with plenty of time to spare.

    }
}

At startup, the program checks to see if the previous reset was caused by a watchdog timeout, and if so it it lights the red LED and clears the watchdog reset flag to indicate that a watchdog reset happened. I believe that this code should never be executed and the red LED should never come on, yet it often does.

What is going on here?

Answer 1

There is a bug in the wdt_reset() library routine.

Here is the code...

__asm__ __volatile__ ( \
   "in __tmp_reg__, __SREG__" "\n\t" \
   "cli" "\n\t" \
   "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" \
)

The fourth line expands out to...

out _WD_CONTROL_REG, _BV(_WD_CHANGE_BIT) | _BV(WDE)

The intention of this line is to write a 1 to the WD_CHANGE_BIT, which will enable the following line to write a 0 to the watchdog enable bit (WDE). From the datasheet:

To disable an enabled Watchdog Timer, the following procedure must be followed: 1. In the same operation, write a logic one to WDCE and WDE. A logic one must be written to WDE regardless of the previous value of the WDE bit. 2. Within the next four clock cycles, in the same operation, write the WDE and WDP bits as desired, but with the WDCE bit cleared.

Unfortunately, this assignment has the side effect of also setting the bottom 3 bits of the Watchdog Control Register (WDCE) to 0's. This immediately sets the prescaler to its shortest value. If the new prescaler has already been fired at the moment this instruction executes, the processor will be reset.

Since the watchdog timer runs off a physically independent 128 kHz oscillator, it is hard to predict what the state of the new prescaler will be in relation to the running program. This accounts for the wide range of observed behaviors where the bug can be correlated with supply voltage, temperature, and manufacturing batch since all of these things can affect the speed of the watchdog oscillator and the system clock asymmetrically. This was a very hard bug to find!

Here is updated code that avoids this problem...

__asm__ __volatile__ ( \
   "in __tmp_reg__, __SREG__" "\n\t" \
   "cli" "\n\t" \
   "wdr" "\n\t" \
   "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" \
)

The extra wdr instruction resets the watchdog timer, so when the following line potentially switches to a different prescaler, it is guaranteed to not have timed out yet.

This could also be fixed by ORing the WD_CHANGE_BIT and WDE bits into the WD_CONTROL_REGISTER as suggested in the datasheets...

; Write logical one to WDCE and WDE
; Keep old prescaler setting to prevent unintentional Watchdog Reset
in r16, WDTCR
ori r16, (1<<WDCE)|(1<<WDE)
out WDTCR, r16

...but this requires more code and an extra scratch register. Since the watchdog counter is reset when it is disabled anyway, the extra reset does not clobber anything and has no unintentional side effects.