Correct timing by timer + overflow count

[igendel]

Sounds like it is rocket science to you.   

Kind as you are, that still doesn't answer the question - what's the point of comparing to 128.
Under what circumstances, in the code you wrote, will the OF flag be set AND timerval will be >= 128?

[Added: to prevent further insults, please be aware that this is just "part 1" of the question]

[igendel]

Ok, sorry for the constant editing, I think I finally figured it out.
Still, a code like this, without that comparison, is clearer and will probably give more accurate results:

Code: [Select]

  cli();
  ovfl = _overflows;
  timerval = TCNT0;
  if (TIFR0 & (1<<TOV0)) {
    ovfl++;
    timerval = TCNT0;
  }
  sei();
  return ( (ovfl << 8) | timerval ); // return result is quite accurate, no time wasted on comparison

[ralphd]

Ok, sorry for the constant editing, I think I finally figured it out.
Still, a code like this, without that comparison, is clearer and will probably give more accurate results:

Code: [Select]

  cli();
  ovfl = _overflows;
  timerval = TCNT0;
  if (TIFR0 & (1<<TOV0)) {
    ovfl++;
    timerval = TCNT0;
  }
  sei();
  return ( (ovfl << 8) | timerval ); // return result is quite accurate, no time wasted on comparison

That way is worse because it adds jitter when the overflow flag is set.
And although avoiding the comparison to 128 is a wash (maybe 1 extra cycle) vs reloading TCNT0 on the 8-bit AVR, on other MCUs the IO read lantency can be slower than the comparison.

p.s. I think I went pretty easy on you after you falsely accused me of posting buggy code.  Suggesting you are not as smart as me isn't an insult if it is true. :-)

[Chris C]

That way is worse because it adds jitter when the overflow flag is set.
And although avoiding the comparison to 128 is a wash (maybe 1 extra cycle) vs reloading TCNT0 on the 8-bit AVR, on other MCUs the IO read lantency can be slower than the comparison.

Then again, on some MCUs the "disable all interrupts" facility does not disable those at the very highest priority.  That priority is reserved for hard real-time tasks, like NTSC generation for instance.  And if that priority is used, not only is jitter reduction in get_ticks() pointless; but you may run into a situation where the timer interrupt flag is set and TCNT0>=128, yet hasn't rolled over a full second time.

Introducing hardware-specific and unusual scenarios does complicate things a bit, and I don't blame [igendel] for questioning why you did it that way.

[igendel]

p.s. I think I went pretty easy on you after you falsely accused me of posting buggy code.  Suggesting you are not as smart as me isn't an insult if it is true. :-)

Yeah, sorry about that. I guess I was inclined to doubt you, since your very first response in this thread was irrelevant to the question, and then you falsely accused me of not reading carefully.

[ralphd]

p.s. I think I went pretty easy on you after you falsely accused me of posting buggy code.  Suggesting you are not as smart as me isn't an insult if it is true. :-)

Yeah, sorry about that. I guess I was inclined to doubt you, since your very first response in this thread was irrelevant to the question, and then you falsely accused me of not reading carefully.

If you are serious, then I'll be adding you to my ignore preferences.  You ended your initial post with, "So, is there a better way? "
My first response was a better way to do correct timing.  You respond saying my solution doesn't solve the problem, and then when you realize it avoids the problem altogether while providing a glitch-free timer, you say it was irrelevant to the question.  Kinda reminds me of the ex-wife, "I don't care what I said, that's not what I meant!" :-)

And since you admitted you had to re-read my blog post and code to understand how it really works, then you really didn't read it carefully the first time.

[igendel]

You ended your initial post with, "So, is there a better way? "

A better way => to overcome the specific overflow-while-reading issue, that I detailed so clearly throughout the post.

Please do add me to your ignore list. It appears both of us will benefit from it.

[C]

Hi guys

I may be wrong, but I see some problems with the general idea.

First
 remember that C and other high level languages can reorder the machine code they can produce.
A = A + 1
could have additional instructions inserted between the machine code steps needed to accomplish this. The compiler is adding additional time for change to happen which expands the time window where the wrong value could be read.

Second
  If the A from above needs more than one instruction to read/write the value, the time window expands again.

just having to use two machine code instructions for a read modify write adds a time window for a change to happen. 

Turning off interrupts can cause harm to other interrupt based tasks.

So when the change is made in interrupt code, the main code will see the change happen at same time. But if the read of A takes more than one machine code instruction, you still have a problem.

A different way to handle this would be to put a serial number on the A value.
read the serial number, read A, read the serial number.
If first serial number = second serial number then A should be good.
The serial number is changed in interrupt code and is one machine code instruction in size for read/write.

Now the value of A could be many machine code instructions in size for a read.

Here you are dealing with a hardware timer and ram value so
loop: serial_number1 = serial_number
         read hardware timer
         read ram value
         serial number2 = serial_number
         if serial number1 <> serial number2 then loop

Think on possibilities,
Any time you have interrupt code changing a value that takes more than one instruction to read you have a problem.

You have a choice of having a specific fix or a general fix.

C

 

[nctnico]

This is why you have critical sections and or mutexes. Asynchronous parallel processes are a common occurence for software engineers.

[westfw]

remember that C and other high level languages can reorder the machine code they can produce.

The opportunities for that should be somewhat limited given the number of "volatile" values in use.  The compiler can't re-order accesses to volatile values, right?

I had this exact problem with a SAMD10 implementation of a microsecond counter recently.  Obscured by rather lengthy synchronization issues and weird reset behavior, too.  ( http://www.avrfreaks.net/forum/samd10-rtc )

It IS that "re-read the counter" that really seems to get you.   I ended up with this code, that seems to work, and includes what I found to be useful debugging hooks:

Code: [Select]

uint32_t micros()
{
  uint32_t thismicros;
  CpuCriticalVar();
  CpuEnterCritical();

#if DEBUG
  rtctick_microscalled++;
#endif
  thismicros = RTC->MODE0.COUNT.reg;
  if (RTC->MODE0.INTFLAG.bit.CMP0) {
    /*
     * Check for a ms interrupt pending but not yet
     * serviced, and correct for it.
     */
    thismicros = RTC->MODE0.COUNT.reg;  //re-read
    thismicros += 1000;
  }
  thismicros += 1000*millisecondcount;
   
#if DEBUG
  if (thismicros < rtctick_lastmicros) {
    // We shouldn't get here.  But allow it to BKPT
    rtctick_backwards++;
    rtctick_trace[rtctick_tracei].current = thismicros;
    rtctick_trace[rtctick_tracei].last = rtctick_lastmicros;
    rtctick_tracei = (rtctick_tracei+1) & 7;
  }
  rtctick_lastmicros = thismicros;
#endif
   
  CpuExitCritical();
  return thismicros;
}


[mikerj]

remember that C and other high level languages can reorder the machine code they can produce.

The opportunities for that should be somewhat limited given the number of "volatile" values in use.  The compiler can't re-order accesses to volatile values, right

You'd think not, but I had this exact problem in avr-gcc.  I had to use a memory barrier to force it to work as intended.

[donotdespisethesnake]

remember that C and other high level languages can reorder the machine code they can produce.

The opportunities for that should be somewhat limited given the number of "volatile" values in use.  The compiler can't re-order accesses to volatile values, right?

volatile used to be a cheap way to get synchronization, but in fact the C spec is hazy on the subject (like many other things). At least volatile accesses can be reordered within a sequence point, and volatile can be be re-ordered with respect to non-volatile accesses. Modern gcc exploits more of this behavior, so traditional assumptions about volatile should not be relied on.

The general rule is that "a single-threaded application should see no difference in behavior" which is great for desktop apps, not great for embedded. Memory barriers can help, but you may still need proper synchronization primitives to get correct behavior.

I've struggled against gcc, and so apparently have Linux kernel team. In one case, to get the code and timing I wanted I gave up with C and used assembler. Otherwise keeping it in C I have to accept some compromise, and generally avoid "tricky" code which requires a specific sequence to work correctly. Even if code works during test, it can easily break depending on optimization when code is added or changed.

It's bad enough when I found bugs in the hardware timers, and then C compiler adds other weirdness on top. "Keep it simple" is a good approach.

Some interesting links :
http://blog.regehr.org/archives/28
https://software.intel.com/en-us/blogs/2007/11/30/volatile-almost-useless-for-multi-threaded-programming
https://lwn.net/Articles/233479/

[Jeroen3]

Volatile insists that each operation with the variables uses the variable from memory. Instead of a cached one in the registers.
Example:

Code: [Select]

int d; // d is global, otherwise it won't get a memory location
int main(void) {
volatile int vd;
vd++;
vd++;
vd++;
__DMB();
d++;
d++;
d++;
}
Will have 3 read-modify-write cycles. As compared to a non volatile one where it will only have 1 read, 3 modifies, and 1 write.
That makes optimizing much more complicated. But there might still be things to do. For example, I used a NOP instead of an DMB first. But it was inserted between the last two vd++'s. Not wat I typed in C.

Code: [Select]

    34:         vd++;
0x080002A6 9800      LDR      r0,[sp,#0x00]
0x080002A8 1C40      ADDS     r0,r0,#1
0x080002AA 9000      STR      r0,[sp,#0x00]
    35:         vd++;
0x080002AC 9800      LDR      r0,[sp,#0x00]
0x080002AE 1C40      ADDS     r0,r0,#1
0x080002B0 9000      STR      r0,[sp,#0x00]
    36:         vd++;
    37:         
0x080002B2 9800      LDR      r0,[sp,#0x00]
0x080002B4 1C40      ADDS     r0,r0,#1
0x080002B6 9000      STR      r0,[sp,#0x00]
    38:         __DMB();
    39:         
0x080002B8 F3BF8F5F  DMB.W   
    40:         d++;
    41:         d++;
0x080002BC 4903      LDR      r1,[pc,#12]  ; @0x080002CC
0x080002BE 6808      LDR      r0,[r1,#0x00]
    42:         d++;
0x080002C0 1CC0      ADDS     r0,r0,#3
0x080002C2 6008      STR      r0,[r1,#0x00]
    43: }
0x080002C4 2000      MOVS     r0,#0x00
(yes, its arm)
Registers are volatile defined in the header. Or at least they should be!

[C]

Thank you  Jeroen3 your code shows exactly part of what I was trying to get at.

Your
  vd++;
shows part what I was trying to say. This is not a new problem.
Back in the days of S-100 & VME buses where you could have many processors on the buss, it was easy to get caught.
You have three instructions for each vd++  So if you had a second processor accessing this same value, you still have a problem.
The timing could end up like this
Processor     code

Code: [Select]


1             34:         vd++;
2             34:         vd++;
1             0x080002A6 9800      LDR      r0,[sp,#0x00]
2             0x080002A6 9800      LDR      r0,[sp,#0x00]
1             0x080002A8 1C40      ADDS     r0,r0,#1
2             0x080002A8 1C40      ADDS     r0,r0,#1
1             0x080002AA 9000      STR      r0,[sp,#0x00]
2             0x080002AA 9000      STR      r0,[sp,#0x00]
Now instead of VD increasing by 2 it increased by 1 and the second was hidden with no errors or notice.
With three instructions you do not have a single buss cycle so a second processor could access the value.
The 68000 had an instruction "TAS"  Test and SET instruction. The memory buss was held so the memory read & memory Write were back to back and indivisible.

Above I used a second processor, but DMA could do this also.
The hardware could be the second processor.
If Processor #2 is interrupt code you have just bunched the three for #2 together where one of the #2's instructions are now.

The C language is bad news in a lot of ways, But the C programmer is just as bad.
 C lost the "Keep it Simple" a very long time ago.

C