Pages: [1]   Go Down
Author Topic: Bug in micros()?  (Read 1422 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 3
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I just had a look at the micros() Funktion of arduino-0022 in file wiring.c and I suppose there is an error in the atomic read sequence.

Code:
unsigned long micros() {
unsigned long m;
uint8_t oldSREG = SREG, t;

cli();
m = timer0_overflow_count;
#if defined(TCNT0)
t = TCNT0;
#elif defined(TCNT0L)
t = TCNT0L;
#else
#error TIMER 0 not defined
#endif

 
#ifdef TIFR0
if ((TIFR0 & _BV(TOV0)) && (t < 255))
m++;
#else
if ((TIFR & _BV(TOV0)) && (t < 255))
m++;
#endif

SREG = oldSREG;

return ((m << 8) + t) * (64 / clockCyclesPerMicrosecond());
}

Timer0 is configured to Fast PWM Mode. In this mode the Timer/Counter Overflow Flag (TOV0) is set each time the counter reaches 255.
Assume that TCNT0 is 254, so that t is assigned 254. Also assume that with the next cycle TCNT0 is incremented to 255 and the TOV0 flag is set. This means that (TIFR0 && _BV(TOV0)) is not zero, therefore m will be incremented => wrong result.
Either "t < 255" has to be replaced by "t < 254" or TIFR0 has to be read before reading TCNT0.

Note: This is just a theoretical case. I don't know the exact timing behaviour of AVR timer overflow interrupt generation.

EDIT: After having a second look at this topic, I assume that it is a mistake to use Fast PWM Mode instead of Normal Mode. In Fast PWM Mode the case should be handled that TCNT0 is still 255 after TOV0 ISR was serviced (i.e. an m-- would be needed in micros() for that case). But in Normal Mode everything should work fine.
« Last Edit: August 22, 2011, 02:11:26 am by matthias_g » Logged

Offline Offline
Edison Member
*
Karma: 3
Posts: 1001
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

The current implementation is based on the assumption that TOV0 is set when TCNT0 change from 255 to zero. For fast PWM mode (default configuration as set in init()) this appears not to be the case and so I think you're right.

Here's a go at a fix that should address both of your issues when in fast PWM mode:

Code:
unsigned long micros() {
unsigned long m;
uint8_t oldSREG = SREG, t;

cli();
m = timer0_overflow_count;
#if defined(TCNT0)
t = TCNT0;
#elif defined(TCNT0L)
t = TCNT0L;
#else
#error TIMER 0 not defined
#endif
        t++;

 
#ifdef TIFR0
if ((TIFR0 & _BV(TOV0)) && (t < 255))
m++;
#else
if ((TIFR & _BV(TOV0)) && (t < 255))
m++;
#endif

SREG = oldSREG;

return ((m << 8) + t) * (64 / clockCyclesPerMicrosecond());
}

Would you agree?

Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 3
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I agree. (But to make it clear: this code is only applicable for Fast PWM Mode!)

I prefer using Normal Mode instead.
Logged

Offline Offline
Edison Member
*
Karma: 3
Posts: 1001
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

The current implementation is based on the assumption that TOV0 is set when TCNT0 change from 255 to zero. For fast PWM mode (default configuration as set in init()) this appears not to be the case and so I think you're right.
I ran some additional tests and it appears that the current implementation is correct as is and no patch is required for fast PWM mode (Arduino default for timer0).

Timer overflow is set at 255 when in PWM mode 3, but at the trailing edge of the timer pulse (see AtMega328 datasheet figure 15-6). This coincides with TCNT0 incrementing from 255 to zero and so TCNT0 will be zero when TOVF0 is set (not 255 as suggested by matthias_g). Tests confirmed this behavior and also revealed issues with delay when patched as suggested above.

In short - what we have in 022 is good as is.
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 3
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I also ran some tests now (ATmega32U4): External clock source on T0 pin. Clock on rising edge. T0 used as output, toggled via debugger. Result is, that in fast PWM mode TOV0 is set to 1 when TCNT0 changes from 255 to 0, on the rising edge of the clock.
This means arduino-022 is correct whereas datasheet statement "The Timer/Counter Overflow Flag (TOV0) is set each time the counter reaches TOP." is wrong. TOV0 is set when the counter reaches 0. Thanks for support.
Logged

Pages: [1]   Go Up
Jump to: