34
\$\begingroup\$

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?

\$\endgroup\$
  • 7
    \$\begingroup\$ If you decided to write up your own Q&A here about this problem I can imagine the pain and suffering that was needed to discover it. \$\endgroup\$ – Vladimir Cravero Jan 31 '15 at 20:46
  • 3
    \$\begingroup\$ You bet! 12 hours on this bug. For a while, the bug would ONLY happen off site. If I brought the boards to my desktop then the bug would go away, likely because of temperature effects (my place is cold which makes the watchdog oscillator runs slightly slower relative to the system clock). It took 30+ trials to reproduce it and catch it in the act on video. \$\endgroup\$ – bigjosh Jan 31 '15 at 21:24
  • \$\begingroup\$ I can almost feel the pain. I am not an old and navigated EE but I sometimes found myself in such situations. Great catch, have a beer and keep solving problems ;) \$\endgroup\$ – Vladimir Cravero Jan 31 '15 at 23:22
41
\$\begingroup\$

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.

\$\endgroup\$
  • 7
    \$\begingroup\$ I'd also like to give you props because when I went to check the avr-libc issue list, it seems you (presumably you) submitted it there already savannah.nongnu.org/bugs/?44140 \$\endgroup\$ – vicatcu Jan 31 '15 at 23:22
  • 1
    \$\begingroup\$ p.s. "josh.com" is real... impressive \$\endgroup\$ – vicatcu Jan 31 '15 at 23:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.