bugAVR C Runtime Library - Bugs: bug #44140, wdt_disable() macro clobbers...

 
 

bug #44140: wdt_disable() macro clobbers prescaller bits and can cause unexpected resets

Submitted by:  None
Submitted on:  Sat 31 Jan 2015 05:05:09 PM UTC  
 
Category: LibrarySeverity: 3 - Normal
Priority: 5 - NormalItem Group: None
Status: FixedPercent Complete: 100%
Assigned to: Pitchumani <pitchumani>Originator Email: -unavailable-
Open/Closed: OpenRelease: Any
Fixed Release: None

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Wed 06 May 2015 11:40:00 AM UTC, comment #4:

Remaks to

https://savannah.nongnu.org/bugs/download.php?file_id=33934

  • adds "memory" clobber to all
  • use constraint "d" with ORI reg
  • use constraint "n" with SFR_MEM_ADDR
  • use "n" with some non-address compile time constants
  • use SFR_MEM_ADDR if used in LDS / STS
  • use SFR_IO_ADDR if used in IN / OUT

This is completely untested. In particular, the right fix for the latter 2 might be to use LDS/STS instead of IN/OUT and not adjusting operands.

Georg-Johann Lay <gjlayde>
Mon 09 Mar 2015 06:37:07 AM UTC, comment #3:

committed.
http://svn.savannah.nongnu.org/viewvc?view=rev&root=avr-libc&revision=2469

Pitchumani <pitchumani>
Project MemberIn charge of this item.
Thu 05 Mar 2015 02:56:20 PM UTC, comment #2:

This new code resolves the unintentional watchdog reset problem. Thanks!

Anonymous
Thu 05 Mar 2015 11:35:37 AM UTC, comment #1:

Hi Josh,

Below is the modified wdt_disable function. I have included wdr instruction before modifying watch dog. Also avoided overwriting prescaler bits.

Generated assembly for tiny84a it should be like:

Is this change ok?

Manually verified the generated assembly for all devices that has wdt.

Pitchumani <pitchumani>
Project MemberIn charge of this item.
Sat 31 Jan 2015 05:05:09 PM UTC, original submission:

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

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #33934:  wdt-2.diff added by gjlayde (8KiB - text/x-patch - Fix several issues.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by gjlayde (Updated the item)
  • -unavailable- added by pitchumani (Posted a comment)
  • -unavailable- added by None (Submitted the item)
  •  

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 06 May 2015 11:38:35 AM UTCgjlaydeAttached File-=>Added wdt-2.diff, #33934
    Mon 09 Mar 2015 06:37:07 AM UTCpitchumaniStatusIn Progress=>Fixed
    Thu 05 Mar 2015 11:35:37 AM UTCpitchumaniStatusNone=>In Progress
      Assigned toNone=>pitchumani

    Back to the top


    Powered by Savane 3.1-cleanup1