avr-libc-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[avr-libc-dev] [Patch] Generalize wdt.h by removing hardcoded device nam


From: Senthil Kumar Selvaraj
Subject: [avr-libc-dev] [Patch] Generalize wdt.h by removing hardcoded device names
Date: Mon, 6 Oct 2014 16:31:22 +0530
User-agent: Mutt/1.5.23 (2014-03-12)

The attached patch to wdt.h gets rid of the huge device specific 
conditional branches for wdt_enable and wdt_disable. The branching is
now based primarily on architecture (xmega, tiny and everything else)
and then on the legality of using the OUT instruction to write to the
watch dog control register.

For the latter, I had to make wdt_{enable/disable} an inline function
and use the _SFR_IO_REG_P macro to emit different inline assembly
fragments for OUT versus STS (as suggested by Johann in the binutils
mailing list). The compiler optimizes away the
range comparison done in _SFR_IO_REG_P, so the generated code should be
identical. I verified that it is indeed identical (at -O1 and -Os),
except for a couple of devices (atmega{8,32,64,128}a) for which the
previous conditional branch was taking the STS path when OUT is more 
optimal.

I do see bigger code at -O0 though. Is that an acceptable tradeoff?

If ok, could someone commit please? I don't have commit access.

Regards
Senthil

ChangeLog

2014-10-06  Senthil Kumar Selvaraj <address@hidden>
                        Georg-Johann Lay <address@hidden>

        * 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..0c2b021 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 (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
 
 
 /**



reply via email to

[Prev in Thread] Current Thread [Next in Thread]