Skip to content

[sival, rv_timer] Fix the systick test's ticks bounds checking#29427

Draft
luismarques wants to merge 1 commit intolowRISC:masterfrom
luismarques:fix-rv_timer_systick_test
Draft

[sival, rv_timer] Fix the systick test's ticks bounds checking#29427
luismarques wants to merge 1 commit intolowRISC:masterfrom
luismarques:fix-rv_timer_systick_test

Conversation

@luismarques
Copy link
Contributor

The chip_sw_rv_timer_systick_test test is supposed to "Verify that the timer can be configured to generate a system tick (...) within 3% of tolerance". However, the existing implementation does not correctly enforce this tolerance:

  TRY_CHECK((elapsed_millis >= (uint64_t)(kReferenceTimeMillis * 0.97)) &&
                (elapsed_millis <= (uint64_t)(kReferenceTimeMillis * 1.03)),
            "Unexpected elapsed time, expected: %u, got: %u",
            (uint32_t)kReferenceTimeMillis, (uint32_t)elapsed_millis);

Because kReferenceTimeMillis * 0.97 and kReferenceTimeMillis * 1.03 are cast to integers, their fractional parts are truncated. With kReferenceTimeMillis == 5, the truncated bounds become 4 and 5. As a result, the test effectively only checks elapsed_millis >= 4 && elapsed_millis <= 5, rather than the intended 3% tolerance.

This PR adopts a different approach, that seemed more convenient, given the lack of floating-point math. Instead of converting the observed timer counter value to milliseconds, we directly compare it with the expected lower and upper number of ticks, for each tested frequency.

The LOW_TICK_BOUND and HIGH_TICK_BOUND macros and their uses could still be refined to improve precision and rounding, but that didn't seem essential for now, particularly given the very loose bounds of the previous implementation.

@luismarques luismarques requested a review from engdoreis March 9, 2026 19:01
Signed-off-by: Luís Marques <luismarques@lowrisc.org>
@luismarques luismarques force-pushed the fix-rv_timer_systick_test branch from 69da655 to 7c8963b Compare March 9, 2026 19:06
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