Using interrupts to extend a 32-bit timer to 64 bits and a pitfall to avoid

[AndrewBCN]

Hello,
When you are forced to use 64-bit variables you should be extra-careful, because as usual the devil is in the details.
Recently I implemented a 64-bit counter by using a 32-bit counter and a counter for the overflow interrupt.

Here is the overflow interrupt callback:

Code: Select all

// Interrupt Service Routine for TIM2 counter overflow / wraparound
void Timer2_Overflow_ISR(void)
{
  tim2overflowcounter++;
  // do we have to manually clear the UIF bit in TIM2 status register? No, the UIF flag is cleared automatically.
}

And the 64-bit counter is put together using these two lines of code:

Code: Select all

lsfcount = TIM2->CCR3; // read 32-bit counter compare register for Timer 2
fcount64 = (tim2overflowcounter << 32) + lsfcount; // hehe now we have a 64-bit counter

Three lines of code, so what could go wrong, right? Well, actually this wasn't working at all and I was scratching my head trying to find out what was going wrong. And then, eureka/light bulb :

I had declared tim2overflowcounter as a global variable thus:

Code: Select all

volatile uint32_t tim2overflowcounter = 0;  // counts the number of times TIM2 overflows

The problem is that shifting left 32 times a 32-bit variable basically clears it, in other words: if tim2overflowcounter is a 32-bit variable, then (tim2overflowcounter << 32) is always zero!

The correct declaration of tim2overflowcounter should be:

Code: Select all

volatile uint64_t tim2overflowcounter = 0;  // counts the number of times TIM2 overflows

Seems pretty obvious once you have solved it, but it still had me scratching my head for a few hours!

[clifford]

Two points about this solution:

First you do not need a 64 bit overflow counter - the upper 32 bits are unused. The fcount64 expression certainly needs to be 64 bit, but that can be done with a cast:

Code: Select all

fcount64 = ((uint64_t)tim2overflowcounter << 32) + lsfcount;

Second the read of the upper and lower 32 bit words is non-atomic. If the Timer2 interrupt occurs between reading the timer counter and reading the overflow you will have the timer counter from the previous rollover period and the time will appear to have leapt forward by one timer overflow period. If read again before the next overflow, it will appear the go backward again. This can result in erroneous timing and bugs that due to their infrequency and lack of deterministic reproducibility will be very hard to spot and debug.

The idiomatic solution to that is:

Code: Select all

uint64_t getTimer64()
{
    uint32_t count_lo = 0u ;
    uint32_t count_hi = 0u ;

    do
    {
        count_hi = tim2overflowcounter ;
        count_lo = TIM2->CCR3 ;

    } while( count_hi != tim2overflowcounter ) ;
   
    return ((uint64_t)count_hi << 32) | count_lo ;
}

Normally the loop runs just a single iteration, in the event of an overflow interrupt, the difference is detected and the loop iterates a second time to get the correct values. The loop should never run more than twice unless you have some preemption (interrupt or thread) that takes longer than a timer roll-over period.

[dannyf]

you got that right. a double-read is a must here.

a few other ways to get this working:
1. using a struct + union so you can read / write individual 32-bit pieces without shifting.
2. using pointers to the write to / read from the individual 32-bit pieces as well.

the code will be endian-dependent, however.