From e7ee53e6edfaca803b671c0ce4ecd391abda1205 Mon Sep 17 00:00:00 2001
From: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
Date: Thu, 24 Nov 2022 13:44:44 +0100
Subject: [PATCH] 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.
---
 tests/periph_timer/main.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

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