Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[kernel,libc] Fix precision timer get_ptime #2088

Merged
merged 1 commit into from
Oct 29, 2024
Merged

[kernel,libc] Fix precision timer get_ptime #2088

merged 1 commit into from
Oct 29, 2024

Conversation

ghaerr
Copy link
Owner

@ghaerr ghaerr commented Oct 29, 2024

Following @Mellvik's fantastic job at testing, using and now identifying a major bug in the precision timer get_ptime routine in Mellvik/TLVC#97, this PR fixes it.

The original precision timing calculation was found to be up to 10ms inaccurate possibly 50% of the time - whenever the PIT countdown timer decreased to a value less than its original value and wrapped across zero (i.e. a hardware timer interrupt).

The original routine should have been unit tested, instead of "eyeballing" its output using QEMU. The current fix has been unit tested using the following, as well as re-eyeballed under QEMU:

#include <stdio.h>

#define MAX_PTICK   11932U     /* PIT reload value for 10ms (100 HZ) */

#define HIGH        11900U
#define LOW         11800U

unsigned testpt(unsigned lastcount, unsigned count, unsigned jdiff)
{
    unsigned pticks;

    printf("lastcount %u, count %u, jdiff %u ", lastcount, count, jdiff);
    pticks = lastcount - count;

#if 1
    if ((int)pticks < 0) {          /* wrapped */
        pticks += MAX_PTICK;        /* = MAX_PTICK - count + lastcount */
        jdiff--;                    /* won't ever be negative */
    }
    if (jdiff < 4286)               /* < ~42.86s */
        return jdiff * (unsigned long)MAX_PTICK + pticks;
#else
    if ((int)pticks < 0)            /* wrapped */
        pticks += MAX_PTICK;        /* = MAX_PTICK - count + lastcount */
    if (jdiff < 2)                  /* < 10ms: 1..11931 */
        return pticks;
    if (jdiff < 4286)               /* < ~42.86s */
        return (jdiff - 1) * (unsigned long)MAX_PTICK + pticks;
#endif
    return 0;
}

int main(int ac, char **av)
{
    printf("= %u\n", testpt(HIGH, LOW, 0));
    //printf("= %u\n", testpt(LOW, HIGH, 0));   /* not possible */
    printf("= %u\n", testpt(HIGH, LOW, 1));
    printf("= %u\n", testpt(LOW, HIGH, 1));
    printf("= %u\n", testpt(HIGH, LOW, 2));
    printf("= %u\n", testpt(LOW, HIGH, 2));
}

For now, all code has #if 1 including both the new/fixed and original code, for possible comparison purposes. This will be removed after having been verified on real hardware.

Thank you @Mellvik for your excellent debugging work on this, well done!!!!

@ghaerr ghaerr merged commit 5809d15 into master Oct 29, 2024
2 checks passed
@ghaerr ghaerr deleted the prectimer2 branch October 29, 2024 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant