Issue with global variable while making 32-bit counter

Question

I am trying to do quadrature decoding using atmel xmega avr microcontroller. Xmega has only 16-bit counters. And in addition I have used up all the available timers.

Now to make 32-bit counter I have used one 16-bit counter and in its over/under flow interrupt I have increment/decrement a 16-bit global variable, so that by combining them we can make 32-bit counter.

ISR(timer_16bit)
{

   if(quad_enc_mov_forward)
    {
      timer_over_flow++;
    }

   else if (quad_enc_mov_backward)
    {
      timer_over_flow--;
    }
}

so far it is working fine. But I need to use this 32-bit value in various tasks running parallel. I'm trying to read 32-bit values as below

uint32_t current_count = timer_over_flow;
         current_count = current_count << 16;
         current_count = current_count + timer_16bit_count;
`timer_16_bit_count` is a hardware register.

Now the problem I am facing is when I read the read timer_over_flow to current_count in the first statement and by the time I add the timer_16bit_count there may be overflow and the 16bit timer may have become zero. This may result in taking total wrong value.

And I am trying to read this 32-bit value in multiple tasks .

Is there a way to prevent this data corruption and get the working model of 32-bit value.

Details sought by different members:

1. My motor can move forward or backward and accordingly counter increments/decrements.

2. In case of ISR, before starting my motor I'm making the global variables(quad_enc_mov_forward & quad_enc_mov_backward) set so that if there is a overflow/underflow timer_over_flow will get changed accordingly.

3. Variables that are modified in the ISR are declared as volatile.

4. Multiple tasks means that I'm using RTOS Kernel with about 6 tasks (mostly 3 tasks running parallel).

5. In the XMEGA I'm directly reading TCCO_CNT register for the lower byte.

Answer 1

One solution is:

uint16_t a, b, c;
do {
    a = timer_over_flow;
    b = timer_16bit_count;
    c = timer_over_flow;
} while (a != c);
uint32_t counter = (uint32_t) a << 16 | b;

Per comment from user5329483, this must not be used with interrupts disabled, since the hardware counter fetched into b may be changing while the interrupt service routine (ISR) that modifies timer_over_flow would not run if interrupts are disabled. It is necessary that the ISR interrupt this code if a wrap occurs during it.

This gets the counters and checks whether the high word changed. If it did, this code tries again. When the loop exits, we know the low word did not wrap during the reads. (Unless there is a possibility we read the high word, then the low word wrapped, then we read the low word, then it wrapped the other way, then we read the high word. If that can happen in your system, an alternative is to add a flag that the ISR sets when the high word changes. The reader would clear the flag, read the timer words, and read the flag. If the flag is set, it tries again.)

Note that timer_over_flow, timer_16bit_count, and the flag, if used, must be volatile.

If the wrap-two-times scenario cannot happen, then you can eliminate the loop:

• Read a, b, and c as above.
• Compare b to 0x8000.
• If b has a high value, either there was no wrap, it was read before a wrap upward (0xffff to 0), or it was read after a wrap downward. Use the lower of a or c.
• Otherwise, either there was no wrap, b was read after a wrap upward, or it was read before a wrap downward. Use the larger of a or c.

Answer 2

The #1 fundamental embedded systems programming FAQ:

Any variable shared between the caller and an ISR, or between different ISRs, must be protected against race conditions. To prevent some compilers from doing incorrect optimizations, such variables should also be declared as volatile.

---

Those who don't understand the above are not qualified to write code containing ISRs. Or programs containing multiple processes or threads for that matter. Programmers who don't realize the above will always write very subtle, very hard-to-catch bugs.

Some means to protect against race conditions could be one of these:

• Temporary disabling the specific interrupt during access.
• Temporary disabling all maskable interrupts during access (crude way).
• Atomic access, verified in the machine code.
• A mutex or semaphore. On single-core MCU:s where interrupts cannot be interrupted in turn, you can use a bool as "poor man's mutex".

Answer 3

Just reading TCCO_CNT in multithreaded code is race condition if you do not handle it correctly. Check the section on reading 16bit registers in XMega manual. You should read lower byte first (this will be probably handled transparently by compiler for you). When lower byte is read, higher byte is (atomically) copied into the TEMP register. Then, reading high byte does read the TEMP register, not the counter. In this way atomic reading of 16bit value is ensured, but only if there is no access to TEMP register between low and high byte read.

Note that this TEMP register is shared between all counters, so context switch in right (wrong) moment will probably trash its content and therefore your high byte. You need to disable interrupts for this 16bit read. Because XMega will execute one instruction after the sei with interrupts disabled, the best way is probably:

cli
ld [low_byte]
sei
ld [high byte]

It disables interrupts for four CPU cycles (if I counted it correctly).

An alternative would to save shared TEMP register(s) on each context switch. It is possible (not sure if likely) that your OS already does this, but be sure to check. Even so, you need to make sure colliding access does not occur from an ISR.

This precaution should be applied to any 16bit register read in your code. Either make sure TEMP register is correctly saved/restored (or not used by multiple threads at all) or disable interrupts when reading/writing 16bit value.

Answer 4

This problem is indeed a very common and very hard one. All solutions will toit will have a caveat regarding timing constraints in the lower priority layers. To clarify this: the highest priority function in your system is the hardware counter - it's response time defines the maximum frequency that you can eventually sample. The next lower priority in your solution is the interrupt routine which tries to keep track of bit 2^16 and the lowest is your application level code which tries to read the 32-bit value. The question now is, if you can quantify the shortest time between two level changes on the A- and B- inputs of your encoder. The shortest time usually does occur not at the highest speed that your real world axis is rotating but when halting at a position: through minimal vibrations the encoder can double swing between two increments, thereby producing e.g. a falling and a rising edge on the same encoder output in short succession. Iff (if and only if) you can guarantee that your interrupt processing time is shorter (by a margin) than this minmal time you can use such a method to virtually extend the coordinate range of your encoder.