diff --git a/cpu/cc430/hwtimer_cc430.c b/cpu/cc430/hwtimer_cc430.c index 466ca500bae5dd9ecd936a580176a7b4d716ccf6..75d6f6bed668d5e99e2bfbffbe026d374060a18d 100644 --- a/cpu/cc430/hwtimer_cc430.c +++ b/cpu/cc430/hwtimer_cc430.c @@ -27,18 +27,16 @@ #define ENABLE_DEBUG (0) #include "debug.h" -static uint32_t ticks = 0; extern void (*int_handler)(int); extern void timer_unset(short timer); -extern uint16_t overflow_interrupt[HWTIMER:_MAXTIMERS+1]; -extern uint16_t timer_round; +extern volatile uint16_t overflow_interrupt[HWTIMER_MAXTIMERS+1]; +extern volatile uint16_t timer_round; void timerA_init(void) { volatile unsigned int *ccr = &TA0CCR0; volatile unsigned int *ctl = &TA0CCTL0; - ticks = 0; /* Set tick counter value to 0 */ timer_round = 0; /* Set to round 0 */ TA0CTL = TASSEL_1 + TACLR; /* Clear the timer counter, set ACLK */ TA0CTL &= ~TAIFG; /* Clear the IFG */ @@ -56,10 +54,12 @@ void timerA_init(void) interrupt(TIMER0_A0_VECTOR) __attribute__((naked)) timer0_a0_isr(void) { __enter_isr(); + if (overflow_interrupt[0] == timer_round) { timer_unset(0); int_handler(0); } + __exit_isr(); } @@ -67,18 +67,34 @@ interrupt(TIMER0_A1_VECTOR) __attribute__((naked)) timer0_a1_5_isr(void) { __enter_isr(); - short taiv = TA0IV; - short timer = taiv / 2; - /* TAIV = 0x0E means overflow */ - if (taiv == 0x0E) { + short taiv_reg = TA0IV; + if (taiv_reg == 0x0E) { + /* TAIV = 0x0E means overflow */ DEBUG("Overflow\n"); - timer_round += 1; + timer_round++; } - /* check which CCR has been hit and if the overflow counter for this timer - * has been reached */ - else if (overflow_interrupt[timer] == timer_round) { - timer_unset(timer); - int_handler(timer); + else { + short timer = taiv_reg >> 1; + /* check which CCR has been hit and if the overflow counter + for this timer has been reached (or exceeded); + there is indeed a race condition where an hwtimer + due to fire in the next round can be set after + the timer's counter has overflowed but *before* + timer_round incrementation has occured (when + interrupts are disabled for any reason), thus + effectively setting the timer one round in the past! */ + int16_t round_delta = overflow_interrupt[timer] - timer_round; + /* in order to correctly handle timer_round overflow, + we must fire the timer when, for example, + timer_round == 0 and overflow_interrupt[timer] == 65535; + to that end, we compute the difference between the two + on a 16-bit signed integer: any difference >= +32768 will + thus overload to a negative number; we should then + correctly fire "overdue" timers whenever the case */ + if (round_delta <= 0) { + timer_unset(timer); + int_handler(timer); + } } __exit_isr(); diff --git a/cpu/msp430-common/hwtimer_cpu.c b/cpu/msp430-common/hwtimer_cpu.c index de561ff24bc77f4d913bb7827e9d9f86d52bbea5..f9cd892bc8c370cd21e7836b3866d7e7800d42dc 100644 --- a/cpu/msp430-common/hwtimer_cpu.c +++ b/cpu/msp430-common/hwtimer_cpu.c @@ -22,8 +22,8 @@ See the file LICENSE in the top level directory for more details. void (*int_handler)(int); extern void timerA_init(void); -uint16_t overflow_interrupt[HWTIMER_MAXTIMERS+1]; -uint16_t timer_round; +volatile uint16_t overflow_interrupt[HWTIMER_MAXTIMERS+1]; +volatile uint16_t timer_round; #ifdef CC430 /* CC430 have "TimerA0", "TimerA1" and so on... */ @@ -52,15 +52,20 @@ static void timer_enable_interrupt(short timer) *ptr &= ~(CCIFG); } -static void timer_set_nostart(unsigned long value, short timer) +static void timer_set_nostart(uint32_t value, short timer) { volatile unsigned int *ptr = &CNT_COMP_BASE_REG + (timer); - *ptr = value; + /* ensure we won't set the timer to a "past" tick */ + if (value <= hwtimer_arch_now()) { + value = hwtimer_arch_now() + 2; + } + overflow_interrupt[timer] = (uint16_t)(value >> 16); + *ptr = (value & 0xFFFF); } -static void timer_set(unsigned long value, short timer) +static void timer_set(uint32_t value, short timer) { - DEBUG("Setting timer %u to %u overflows and %lu\n", timer, overflow_interrupt[timer], value); + DEBUG("Setting timer %u to %lu\n", timer, value); timer_set_nostart(value, timer); timer_enable_interrupt(timer); } @@ -74,7 +79,7 @@ void timer_unset(short timer) unsigned long hwtimer_arch_now() { - return ((uint32_t)timer_round << 16)+TIMER_VAL_REG; + return ((uint32_t)timer_round << 16) + TIMER_VAL_REG; } void hwtimer_arch_init(void (*handler)(int), uint32_t fcpu) @@ -106,9 +111,7 @@ void hwtimer_arch_set(unsigned long offset, short timer) void hwtimer_arch_set_absolute(unsigned long value, short timer) { - uint16_t small_value = value & 0xFFFF; - overflow_interrupt[timer] = (uint16_t)(value >> 16); - timer_set(small_value, timer); + timer_set(value, timer); } void hwtimer_arch_unset(short timer) diff --git a/cpu/msp430x16x/hwtimer_msp430.c b/cpu/msp430x16x/hwtimer_msp430.c index aa55b05662ed3386ce1411e8f3a51f70b851c8e0..8c15ad90a326d8d6732b72d5b159a20996dd7b82 100644 --- a/cpu/msp430x16x/hwtimer_msp430.c +++ b/cpu/msp430x16x/hwtimer_msp430.c @@ -27,18 +27,15 @@ #define ENABLE_DEBUG (0) #include "debug.h" -static uint32_t ticks = 0; - extern void (*int_handler)(int); extern void timer_unset(short timer); -extern uint16_t overflow_interrupt[HWTIMER_MAXTIMERS+1]; -extern uint16_t timer_round; +extern volatile uint16_t overflow_interrupt[HWTIMER_MAXTIMERS+1]; +extern volatile uint16_t timer_round; void timerA_init(void) { volatile unsigned int *ccr; volatile unsigned int *ctl; - ticks = 0; /* Set tick counter value to 0 */ timer_round = 0; /* Set to round 0 */ TACTL = TASSEL_1 + TACLR; /* Clear the timer counter, set ACLK */ TACTL &= ~TAIFG; /* Clear the IFG */ @@ -58,10 +55,12 @@ void timerA_init(void) interrupt(TIMERA0_VECTOR) __attribute__((naked)) timer_isr_ccr0(void) { __enter_isr(); + if (overflow_interrupt[0] == timer_round) { timer_unset(0); int_handler(0); } + __exit_isr(); } @@ -69,18 +68,34 @@ interrupt(TIMERA1_VECTOR) __attribute__((naked)) timer_isr(void) { __enter_isr(); - short taiv = TAIV; - short timer = taiv / 2; - /* TAIV = 0x0A means overflow */ - if (taiv == 0x0A) { + short taiv_reg = TAIV; + if (taiv_reg == 0x0A) { + /* TAIV = 0x0A means overflow */ DEBUG("Overflow\n"); - timer_round += 1; + timer_round++; } - /* check which CCR has been hit and if the overflow counter for this timer - * has been reached */ - else if (overflow_interrupt[timer] == timer_round) { - timer_unset(timer); - int_handler(timer); + else { + short timer = taiv_reg >> 1; + /* check which CCR has been hit and if the overflow counter + for this timer has been reached (or exceeded); + there is indeed a race condition where an hwtimer + due to fire in the next round can be set after + the timer's counter has overflowed but *before* + timer_round incrementation has occured (when + interrupts are disabled for any reason), thus + effectively setting the timer one round in the past! */ + int16_t round_delta = overflow_interrupt[timer] - timer_round; + /* in order to correctly handle timer_round overflow, + we must fire the timer when, for example, + timer_round == 0 and overflow_interrupt[timer] == 65535; + to that end, we compute the difference between the two + on a 16-bit signed integer: any difference >= +32768 will + thus overload to a negative number; we should then + correctly fire "overdue" timers whenever the case */ + if (round_delta <= 0) { + timer_unset(timer); + int_handler(timer); + } } __exit_isr();