Skip to content

Commit

Permalink
tests/periph/timer: fix timeout computation and overflow handling
Browse files Browse the repository at this point in the history
- the timeout computation for the spurious IRQ test confused numerator
  and denominator in a fraction
- the timeout offset between timer channels was hardcoded to 5000 from
  when the timer was only tested with 1 MHz as frequency
    - This resulted in slooooow test runs when running at slow
      frequencies
- fix overflow handling in the spinning wait
    - likely this would never overflow anyway assuming that
      `timer_init()` resets the counter value, but let's not rely on
      this and just fix the bug for good
  • Loading branch information
maribu committed Dec 7, 2023
1 parent 8e67ee3 commit 197f4dc
Showing 1 changed file with 21 additions and 7 deletions.
28 changes: 21 additions & 7 deletions tests/periph/timer/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,15 @@
#define TIMER_CHANNEL_NUMOF 10U
#endif

#define CHAN_OFFSET (5000U) /* fire every 5ms */
#define CHAN_OFFSET_MS 5U /* fire channels with 5 ms offset */
/* The minimum timeout to set and still being able to clear the timer
* before it fires. This should be conservative, as wasting a few milliseconds
* in the test is less annoying than false test failures */
#define MINIMUM_TIMEOUT_MS 2
#define COOKIE (100U) /* for checking if arg is passed */
/* Setting a timer for less than two ticks may cause it to fire right away,
* e.g. when the timer was about to tick anyway */
#define MINIMUM_TICKS 2

static uint8_t fired;
static uint32_t sw_count;
Expand Down Expand Up @@ -73,6 +80,14 @@ static uword_t query_channel_numof(tim_t dev)
return TIMER_CHANNEL_NUMOF;
}

static unsigned milliseconds_to_ticks(uint32_t timer_freq, unsigned millisecs)
{
/* Use 64 bit arithmetics and to avoid overflows for high frequencies. */

Check failure on line 85 in tests/periph/timer/main.c

View workflow job for this annotation

GitHub Actions / static-tests

There is a typo: arithmetics ==> arithmetic If this is a false positive, add it to dist/tools/codespell/ignored_words.txt. You can fix this interactively by calling CODESPELL_INTERACTIVE=1 BASE_BRANCH=master ./dist/tools/codespell/check.sh
unsigned result = (uint64_t)millisecs * US_PER_MS * timer_freq / US_PER_SEC;
/* Never return less than 2 ticks */
return (result >= MINIMUM_TICKS) ? result : MINIMUM_TICKS;
}

static int test_timer(unsigned num, uint32_t timer_freq)
{
int set = 0;
Expand All @@ -99,10 +114,11 @@ static int test_timer(unsigned num, uint32_t timer_freq)

timer_stop(TIMER_DEV(num));
printf(" - timer_stop(%u): stopped\n", num);
unsigned chan_offset_ticks = milliseconds_to_ticks(timer_freq, CHAN_OFFSET_MS);

/* set each available channel */
for (unsigned i = 0; i < query_channel_numof(TIMER_DEV(num)); i++) {
unsigned timeout = ((i + 1) * CHAN_OFFSET);
unsigned timeout = ((i + 1) * chan_offset_ticks);
printf(" - timer_set(%u, %u, %u)\n ", num, i, timeout);
if (timer_set(TIMER_DEV(num), i, timeout) < 0) {
printf("ERROR: Couldn't set timeout %u for channel %u\n",
Expand Down Expand Up @@ -159,12 +175,12 @@ static int test_timer(unsigned num, uint32_t timer_freq)
printf(" - Validating no spurious IRQs are triggered:\n");
expect(0 == timer_init(TIMER_DEV(num), timer_freq, cb_not_to_be_executed, NULL));

const unsigned duration = 2ULL * US_PER_MS * US_PER_SEC / timer_freq;
const unsigned duration = milliseconds_to_ticks(timer_freq, MINIMUM_TIMEOUT_MS);
unsigned target = timer_read(TIMER_DEV(num)) + duration;
expect(0 == timer_set_absolute(TIMER_DEV(num), 0, target));
expect(0 == timer_clear(TIMER_DEV(num), 0));
atomic_store_u8(&fired, 0);
while (timer_read(TIMER_DEV(num)) < target) {
while ((target - timer_read(TIMER_DEV(num))) <= duration) {
/* busy waiting for the timer to reach it timeout. Timer must not fire,
* it was cleared */
}
Expand All @@ -176,10 +192,8 @@ static int test_timer(unsigned num, uint32_t timer_freq)
/* checking again to make sure that any IRQ pending bit that may just was
* mask doesn't trigger a timer IRQ on the next set */
target = timer_read(TIMER_DEV(num)) + duration;
timer_set_absolute(TIMER_DEV(num), 0, target);
timer_clear(TIMER_DEV(num), 0);

while (timer_read(TIMER_DEV(num)) < target) {
while ((target - timer_read(TIMER_DEV(num))) <= duration) {
/* busy waiting for the timer to reach it timeout. Timer must not fire,
* it was cleared */
}
Expand Down

0 comments on commit 197f4dc

Please sign in to comment.