Issues with HiResTimer

[Watzlavick]

I think there are two issues with the HiResTimer on the MOD5441x:

1) In delay_uSec(), I believe init_ticks() should be called with ReferenceTicks, not delayTime. I noticed this when I tried to use delay_uSec() directly instead of delay(). For cases when the delay is known in advance, it is better to use delay_uSec() and avoid the floating point math in delay().

2) There may be a race condition in readTime(). If a counter overflow occurs after the USER_ENTER_CRITICAL call but before readLow() is called, then the time will be under reported by 34 seconds (2^32 * 8 ns) because resetCount didn't get updated. Disabling interrupts is a good strategy but in this case may actually make the problem worse. A safer way to do it would be as follows:

Code: Select all

OSLock(); // Allow interrupts to occur so resetCount_ will update but prevent task switching
DWORD lowTime_1 = readLow();
DWORD highTime_1 = readHigh(); // same as reading resetCount_ but clearer
DWORD lowTime_2;
DWORD highTime_2;
do
{
   lowTime_2 = readLow();
   highTime_2 = readHigh();
} while (highTime_2 != highTime_1);
OSUnlock();
// continue calculations using lowTime_2 and highTime_2

A variation of this is to read H, L, H and see if H changes. But if H changes, then it's not clear what to set L to so it's easier to just read them at the same time. Even though there's the potential of an infinite loop, there's virtually no chance of hitting that loop more than twice. If that is a concern, just read H, L, H, L and go with it.

-Bob

[dciliske]

I'll take a look at this once my house isn't near a fire...

[Watzlavick]

Yikes-good luck! No hurry on this.
-Bob

[dciliske]

1) It would appear you are correct. I'm not sure how that got missed. Bugs happen...

2) I think you're on the right track here, but I'm almost certain that this triggers a near infinite loop (rolling over after a few hundred years...). Here's my proposed fix:

Code: Select all

    OSLock(); // Allow interrupts to occur so resetCount_ will update but prevent task switching
    DWORD lowTime = readLow();
    DWORD highTime = readHigh(); // same as reading resetCount_ but clearer
    DWORD lowTimeCheck;
    if ((lowTimeCheck = readLow()) < lowTime) { // check to see if we rolled over
        asm("    nop;");
        lowTime = lowTimeCheck;
        highTime = readHigh();
    }
    OSUnlock();

-Dan

[Watzlavick]

The idea behind the loop was to keep reading until highTime is the same twice in a row. The only way it could execute more than twice is if some other higher priority interrupt kept it away for the duration of the counter (34 sec). But your idea seems safer Just curious - what is the purpose of the nop? I've seen that elsewhere in the NB code. Is it to flush the instruction cache?

-Bob

[dciliske]

It forces a flush of the pipeline; thus if there is an interrupt that has triggered, but not yet been processed, it will fire the handler before continuing, which is desirable if we hit a rollover. Also, sometimes it's there to make other hardware happy by flushing the pipeline (errata, etc.)

-Dan