1
0
mirror of https://github.com/RIOT-OS/RIOT.git synced 2024-12-29 04:50:03 +01:00

tests/periph_timer: fix style and use of volatile

To synchronize communication via shared memory between ISR context and
thread context it is a common misconception that `volatile` is
sufficient. This is however is not the cause and the cause of many
subtle data race bugs. This fixes the issue.
This commit is contained in:
Marian Buschsieweke 2022-11-24 13:44:44 +01:00
parent f1a8e1f636
commit e7ee53e6ed
No known key found for this signature in database
GPG Key ID: CB8E3238CE715A94

View File

@ -22,6 +22,7 @@
#include <stdint.h> #include <stdint.h>
#include <stdlib.h> #include <stdlib.h>
#include "atomic_utils.h"
#include "clk.h" #include "clk.h"
#include "periph/timer.h" #include "periph/timer.h"
@ -36,10 +37,10 @@
#define CHAN_OFFSET (5000U) /* fire every 5ms */ #define CHAN_OFFSET (5000U) /* fire every 5ms */
#define COOKIE (100U) /* for checking if arg is passed */ #define COOKIE (100U) /* for checking if arg is passed */
static volatile int fired; static uint8_t fired;
static volatile uint32_t sw_count; static uint32_t sw_count;
static volatile uint32_t timeouts[MAX_CHANNELS]; static uint32_t timeouts[MAX_CHANNELS];
static volatile unsigned args[MAX_CHANNELS]; static unsigned args[MAX_CHANNELS];
static void cb(void *arg, int chan) static void cb(void *arg, int chan)
{ {
@ -53,8 +54,9 @@ static int test_timer(unsigned num)
int set = 0; int set = 0;
/* reset state */ /* reset state */
sw_count = 0; atomic_store_u32(&sw_count, 0);
fired = 0; atomic_store_u8(&fired, 0);
for (unsigned i = 0; i < MAX_CHANNELS; i++) { for (unsigned i = 0; i < MAX_CHANNELS; i++) {
timeouts[i] = 0; timeouts[i] = 0;
args[i] = UINT_MAX; args[i] = UINT_MAX;
@ -68,8 +70,10 @@ static int test_timer(unsigned num)
else { else {
printf("TIMER_%u: initialization successful\n", num); printf("TIMER_%u: initialization successful\n", num);
} }
timer_stop(TIMER_DEV(num)); timer_stop(TIMER_DEV(num));
printf("TIMER_%u: stopped\n", num); printf("TIMER_%u: stopped\n", num);
/* set each available channel */ /* set each available channel */
for (unsigned i = 0; i < MAX_CHANNELS; i++) { for (unsigned i = 0; i < MAX_CHANNELS; i++) {
unsigned timeout = ((i + 1) * CHAN_OFFSET); unsigned timeout = ((i + 1) * CHAN_OFFSET);
@ -81,19 +85,23 @@ static int test_timer(unsigned num)
printf("TIMER_%u: set channel %u to %u\n", num, i, timeout); printf("TIMER_%u: set channel %u to %u\n", num, i, timeout);
} }
} }
if (set == 0) { if (set == 0) {
printf("TIMER_%u: ERROR setting any channel\n\n", num); printf("TIMER_%u: ERROR setting any channel\n\n", num);
return 0; return 0;
} }
/* start the timer */ /* start the timer */
printf("TIMER_%u: starting\n", num); printf("TIMER_%u: starting\n", num);
timer_start(TIMER_DEV(num)); timer_start(TIMER_DEV(num));
/* wait for all channels to fire */ /* wait for all channels to fire */
do { do {
++sw_count; semi_atomic_fetch_add_u32(&sw_count, 1);
} while (fired != set); } while (atomic_load_u8(&fired) != set);
/* collect results */ /* collect results */
for (int i = 0; i < fired; i++) { for (int i = 0; i < set; i++) {
if (args[i] != ((COOKIE * num) + i)) { if (args[i] != ((COOKIE * num) + i)) {
printf("TIMER_%u: ERROR callback argument mismatch\n\n", num); printf("TIMER_%u: ERROR callback argument mismatch\n\n", num);
return 0; return 0;
@ -101,12 +109,14 @@ static int test_timer(unsigned num)
printf("TIMER_%u: channel %i fired at SW count %8u", printf("TIMER_%u: channel %i fired at SW count %8u",
num, i, (unsigned)timeouts[i]); num, i, (unsigned)timeouts[i]);
if (i == 0) { if (i == 0) {
printf(" - init: %8u\n", (unsigned)timeouts[i]); printf(" - init: %8" PRIu32 "\n", atomic_load_u32(&timeouts[i]));
} }
else { else {
printf(" - diff: %8u\n", (unsigned)(timeouts[i] - timeouts[i - 1])); printf(" - diff: %8" PRIu32 "\n",
atomic_load_u32(&timeouts[i]) - atomic_load_u32(&timeouts[i - 1]));
} }
} }
return 1; return 1;
} }