On Sat, Oct 11, 2014 at 10:35:45AM +0200, Joerg Wunsch wrote: > As Senthil Kumar Selvaraj wrote: > > > The attached patch to wdt.h gets rid of the huge device specific > > conditional branches for wdt_enable and wdt_disable. > > That's cool. The current state of affairs used to be a continued > cause for being forgotten upon adding a new devices. > > > I do see bigger code at -O0 though. Is that an acceptable tradeoff? > > For wdt_enable/wdt_disable, yes, it is. Only wdt_reset() must be > kept as short as possible, but as this translates directly into > one assembly instruction, that's not an issue. Ok.
I've recreated the patch with s/inline/__inline__, s/always_inline/__always_inline__ and a const declaration for the wdt_enable parameter. Could someone please apply the patch? I don't have commit access. Regards Senthil ChangeLog 2014-10-06 Senthil Kumar Selvaraj <senthil_kumar.selva...@atmel.com> Georg-Johann Lay <a...@gjlay.de> * include/avr/wdt.h: Generalize implementation of wdt_enable and wdt_disable based on arch and addressability of _WD_CONTROL_REG. diff --git avr-libc/include/avr/wdt.h avr-libc/include/avr/wdt.h index 57a20c3..fc15885 100644 --- avr-libc/include/avr/wdt.h +++ avr-libc/include/avr/wdt.h @@ -132,45 +132,7 @@ */ -#if defined(__AVR_ATxmega16A4__) \ -|| defined(__AVR_ATxmega16A4U__) \ -|| defined(__AVR_ATxmega16C4__) \ -|| defined(__AVR_ATxmega16D4__) \ -|| defined(__AVR_ATxmega32A4__) \ -|| defined(__AVR_ATxmega32A4U__) \ -|| defined(__AVR_ATxmega32C4__) \ -|| defined(__AVR_ATxmega32D4__) \ -|| defined(__AVR_ATxmega64A1U__) \ -|| defined(__AVR_ATxmega64A3__) \ -|| defined(__AVR_ATxmega64A3U__) \ -|| defined(__AVR_ATxmega64A4U__) \ -|| defined(__AVR_ATxmega64B1__) \ -|| defined(__AVR_ATxmega64B3__) \ -|| defined(__AVR_ATxmega64C3__) \ -|| defined(__AVR_ATxmega64D3__) \ -|| defined(__AVR_ATxmega64D4__) \ -|| defined(__AVR_ATxmega128A1__) \ -|| defined(__AVR_ATxmega128A1U__) \ -|| defined(__AVR_ATxmega128A3__) \ -|| defined(__AVR_ATxmega128A3U__) \ -|| defined(__AVR_ATxmega128A4U__) \ -|| defined(__AVR_ATxmega128B1__) \ -|| defined(__AVR_ATxmega128B3__) \ -|| defined(__AVR_ATxmega128C3__) \ -|| defined(__AVR_ATxmega128D3__) \ -|| defined(__AVR_ATxmega128D4__) \ -|| defined(__AVR_ATxmega192A3__) \ -|| defined(__AVR_ATxmega192A3U__) \ -|| defined(__AVR_ATxmega192C3__) \ -|| defined(__AVR_ATxmega192D3__) \ -|| defined(__AVR_ATxmega256A3__) \ -|| defined(__AVR_ATxmega256A3U__) \ -|| defined(__AVR_ATxmega256C3__) \ -|| defined(__AVR_ATxmega256D3__) \ -|| defined(__AVR_ATxmega256A3B__) \ -|| defined(__AVR_ATxmega256A3BU__) \ -|| defined(__AVR_ATxmega384C3__) \ -|| defined(__AVR_ATxmega384D3__) +#if defined(__AVR_XMEGA__) /* wdt_enable(timeout) for xmega devices @@ -225,175 +187,7 @@ __asm__ __volatile__ ( \ : "r0" \ ); -#elif defined(__AVR_AT90CAN32__) \ -|| defined(__AVR_AT90CAN64__) \ -|| defined(__AVR_AT90CAN128__) \ -|| defined(__AVR_AT90PWM1__) \ -|| defined(__AVR_AT90PWM2__) \ -|| defined(__AVR_AT90PWM216__) \ -|| defined(__AVR_AT90PWM2B__) \ -|| defined(__AVR_AT90PWM3__) \ -|| defined(__AVR_AT90PWM316__) \ -|| defined(__AVR_AT90PWM3B__) \ -|| defined(__AVR_AT90PWM161__) \ -|| defined(__AVR_AT90PWM81__) \ -|| defined(__AVR_AT90USB1286__) \ -|| defined(__AVR_AT90USB1287__) \ -|| defined(__AVR_AT90USB162__) \ -|| defined(__AVR_AT90USB646__) \ -|| defined(__AVR_AT90USB647__) \ -|| defined(__AVR_AT90USB82__) \ -|| defined(__AVR_ATmega128A__) \ -|| defined(__AVR_ATmega1280__) \ -|| defined(__AVR_ATmega1281__) \ -|| defined(__AVR_ATmega1284__) \ -|| defined(__AVR_ATmega1284P__) \ -|| defined(__AVR_ATmega128RFA1__) \ -|| defined(__AVR_ATmega1284RFR2__) \ -|| defined(__AVR_ATmega128RFR2__) \ -|| defined(__AVR_ATmega164__) \ -|| defined(__AVR_ATmega164A__) \ -|| defined(__AVR_ATmega164P__) \ -|| defined(__AVR_ATmega164PA__) \ -|| defined(__AVR_ATmega165__) \ -|| defined(__AVR_ATmega165A__) \ -|| defined(__AVR_ATmega165P__) \ -|| defined(__AVR_ATmega165PA__) \ -|| defined(__AVR_ATmega168__) \ -|| defined(__AVR_ATmega168A__) \ -|| defined(__AVR_ATmega168P__) \ -|| defined(__AVR_ATmega168PA__) \ -|| defined(__AVR_ATmega169__) \ -|| defined(__AVR_ATmega169A__) \ -|| defined(__AVR_ATmega169P__) \ -|| defined(__AVR_ATmega169PA__) \ -|| defined(__AVR_ATmega16HVA__) \ -|| defined(__AVR_ATmega16HVA2__) \ -|| defined(__AVR_ATmega16HVB__) \ -|| defined(__AVR_ATmega16HVBREVB__) \ -|| defined(__AVR_ATmega16M1__) \ -|| defined(__AVR_ATmega16U2__) \ -|| defined(__AVR_ATmega16U4__) \ -|| defined(__AVR_ATmega2560__) \ -|| defined(__AVR_ATmega2561__) \ -|| defined(__AVR_ATmega2564RFR2__) \ -|| defined(__AVR_ATmega256RFR2__) \ -|| defined(__AVR_ATmega32A__) \ -|| defined(__AVR_ATmega324__) \ -|| defined(__AVR_ATmega324A__) \ -|| defined(__AVR_ATmega324P__) \ -|| defined(__AVR_ATmega324PA__) \ -|| defined(__AVR_ATmega325__) \ -|| defined(__AVR_ATmega325A__) \ -|| defined(__AVR_ATmega325P__) \ -|| defined(__AVR_ATmega325PA__) \ -|| defined(__AVR_ATmega3250__) \ -|| defined(__AVR_ATmega3250A__) \ -|| defined(__AVR_ATmega3250P__) \ -|| defined(__AVR_ATmega3250PA__) \ -|| defined(__AVR_ATmega328__) \ -|| defined(__AVR_ATmega328P__) \ -|| defined(__AVR_ATmega329__) \ -|| defined(__AVR_ATmega329A__) \ -|| defined(__AVR_ATmega329P__) \ -|| defined(__AVR_ATmega329PA__) \ -|| defined(__AVR_ATmega3290__) \ -|| defined(__AVR_ATmega3290A__) \ -|| defined(__AVR_ATmega3290P__) \ -|| defined(__AVR_ATmega3290PA__) \ -|| defined(__AVR_ATmega32C1__) \ -|| defined(__AVR_ATmega32HVB__) \ -|| defined(__AVR_ATmega32HVBREVB__) \ -|| defined(__AVR_ATmega32M1__) \ -|| defined(__AVR_ATmega32U2__) \ -|| defined(__AVR_ATmega32U4__) \ -|| defined(__AVR_ATmega32U6__) \ -|| defined(__AVR_ATmega406__) \ -|| defined(__AVR_ATmega48__) \ -|| defined(__AVR_ATmega48A__) \ -|| defined(__AVR_ATmega48P__) \ -|| defined(__AVR_ATmega48PA__) \ -|| defined(__AVR_ATmega644RFR2__) \ -|| defined(__AVR_ATmega64RFR2__) \ -|| defined(__AVR_ATmega64A__) \ -|| defined(__AVR_ATmega640__) \ -|| defined(__AVR_ATmega644__) \ -|| defined(__AVR_ATmega644A__) \ -|| defined(__AVR_ATmega644P__) \ -|| defined(__AVR_ATmega644PA__) \ -|| defined(__AVR_ATmega645__) \ -|| defined(__AVR_ATmega645A__) \ -|| defined(__AVR_ATmega645P__) \ -|| defined(__AVR_ATmega6450__) \ -|| defined(__AVR_ATmega6450A__) \ -|| defined(__AVR_ATmega6450P__) \ -|| defined(__AVR_ATmega649__) \ -|| defined(__AVR_ATmega649A__) \ -|| defined(__AVR_ATmega6490__) \ -|| defined(__AVR_ATmega6490A__) \ -|| defined(__AVR_ATmega6490P__) \ -|| defined(__AVR_ATmega649P__) \ -|| defined(__AVR_ATmega64C1__) \ -|| defined(__AVR_ATmega64HVE__) \ -|| defined(__AVR_ATmega64M1__) \ -|| defined(__AVR_ATmega8A__) \ -|| defined(__AVR_ATmega88__) \ -|| defined(__AVR_ATmega88A__) \ -|| defined(__AVR_ATmega88P__) \ -|| defined(__AVR_ATmega88PA__) \ -|| defined(__AVR_ATmega8HVA__) \ -|| defined(__AVR_ATmega8U2__) \ -|| defined(__AVR_ATtiny48__) \ -|| defined(__AVR_ATtiny88__) \ -|| defined(__AVR_ATtiny87__) \ -|| defined(__AVR_ATtiny167__) \ -|| defined(__AVR_AT90SCR100__) \ -|| defined(__AVR_ATA6285__) \ -|| defined(__AVR_ATA6286__) \ -|| defined(__AVR_ATA6289__) \ -|| defined(__AVR_ATA5272__) \ -|| defined(__AVR_ATA5505__) \ -|| defined(__AVR_ATA5790__) \ -|| defined(__AVR_ATA5795__) - -/* Use STS instruction. */ - -#define wdt_enable(value) \ -__asm__ __volatile__ ( \ - "in __tmp_reg__,__SREG__" "\n\t" \ - "cli" "\n\t" \ - "wdr" "\n\t" \ - "sts %0,%1" "\n\t" \ - "out __SREG__,__tmp_reg__" "\n\t" \ - "sts %0,%2" "\n\t" \ - : /* no outputs */ \ - : "M" (_SFR_MEM_ADDR(_WD_CONTROL_REG)), \ - "r" ((uint8_t)(_BV(_WD_CHANGE_BIT) | _BV(WDE))), \ - "r" ((uint8_t) ((value & 0x08 ? _WD_PS3_MASK : 0x00) | \ - _BV(WDE) | (value & 0x07)) ) \ - : "r0" \ -) - -#define wdt_disable() \ -__asm__ __volatile__ ( \ - "in __tmp_reg__, __SREG__" "\n\t" \ - "cli" "\n\t" \ - "sts %0, %1" "\n\t" \ - "sts %0, __zero_reg__" "\n\t" \ - "out __SREG__,__tmp_reg__" "\n\t" \ - : /* no outputs */ \ - : "M" (_SFR_MEM_ADDR(_WD_CONTROL_REG)), \ - "r" ((uint8_t)(_BV(_WD_CHANGE_BIT) | _BV(WDE))) \ - : "r0" \ -) - - -#elif defined(__AVR_ATtiny4__) \ -|| defined(__AVR_ATtiny5__) \ -|| defined(__AVR_ATtiny9__) \ -|| defined(__AVR_ATtiny10__) \ -|| defined(__AVR_ATtiny20__) \ -|| defined(__AVR_ATtiny40__) +#elif defined(__AVR_TINY__) #define wdt_enable(value) \ __asm__ __volatile__ ( \ @@ -433,48 +227,84 @@ __asm__ __volatile__ ( \ : "r16" \ ); \ }while(0) - -#else - -/* Use OUT instruction. */ - -#define wdt_enable(value) \ - __asm__ __volatile__ ( \ - "in __tmp_reg__,__SREG__" "\n\t" \ - "cli" "\n\t" \ - "wdr" "\n\t" \ - "out %0,%1" "\n\t" \ - "out __SREG__,__tmp_reg__" "\n\t" \ - "out %0,%2" \ - : /* no outputs */ \ - : "I" (_SFR_IO_ADDR(_WD_CONTROL_REG)), \ - "r" ((uint8_t)(_BV(_WD_CHANGE_BIT) | _BV(WDE))), \ - "r" ((uint8_t) ((value & 0x08 ? _WD_PS3_MASK : 0x00) | \ - _BV(WDE) | (value & 0x07)) ) \ - : "r0" \ - ) -/** - \ingroup avr_watchdog - Disable the watchdog timer, if possible. This attempts to turn off the - Enable bit in the watchdog control register. See the datasheet for - details. -*/ -#define wdt_disable() \ -__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" \ -) +#else -#endif +static __inline__ +__attribute__ ((__always_inline__)) +void wdt_enable (const uint8_t value) +{ + if (_SFR_IO_REG_P (_WD_CONTROL_REG)) + { + __asm__ __volatile__ ( + "in __tmp_reg__,__SREG__" "\n\t" + "cli" "\n\t" + "wdr" "\n\t" + "out %0, %1" "\n\t" + "out __SREG__,__tmp_reg__" "\n\t" + "out %0, %2" "\n \t" + : /* no outputs */ + : "I" (_SFR_IO_ADDR(_WD_CONTROL_REG)), + "r" ((uint8_t)(_BV(_WD_CHANGE_BIT) | _BV(WDE))), + "r" ((uint8_t) ((value & 0x08 ? _WD_PS3_MASK : 0x00) | + _BV(WDE) | (value & 0x07)) ) + : "r0" + ); + } + else + { + __asm__ __volatile__ ( + "in __tmp_reg__,__SREG__" "\n\t" + "cli" "\n\t" + "wdr" "\n\t" + "sts %0, %1" "\n\t" + "out __SREG__,__tmp_reg__" "\n\t" + "sts %0, %2" "\n \t" + : /* no outputs */ + : "M" (_SFR_MEM_ADDR(_WD_CONTROL_REG)), + "r" ((uint8_t)(_BV(_WD_CHANGE_BIT) | _BV(WDE))), + "r" ((uint8_t) ((value & 0x08 ? _WD_PS3_MASK : 0x00) | + _BV(WDE) | (value & 0x07)) ) + : "r0" + ); + } +} + +static __inline__ +__attribute__ ((__always_inline__)) +void wdt_disable (void) +{ + if (_SFR_IO_REG_P (_WD_CONTROL_REG)) + { + __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" + ); + } + else + { + __asm__ __volatile__ ( + "in __tmp_reg__, __SREG__" "\n\t" + "cli" "\n\t" + "sts %0, %1" "\n\t" + "sts %0, __zero_reg__" "\n\t" + "out __SREG__,__tmp_reg__" "\n\t" + : /* no outputs */ + : "M" (_SFR_MEM_ADDR(_WD_CONTROL_REG)), + "r" ((uint8_t)(_BV(_WD_CHANGE_BIT) | _BV(WDE))) + : "r0" + ); + } +} +#endif /** _______________________________________________ AVR-libc-dev mailing list AVR-libc-dev@nongnu.org https://lists.nongnu.org/mailman/listinfo/avr-libc-dev