From 97770aba670d3ba35e5c04581855d74a7baf3db3 Mon Sep 17 00:00:00 2001
From: Alistair Francis <alistair@alistair23.me>
Date: Thu, 4 Jul 2024 20:15:32 +1000
Subject: [PATCH] NonArduino: Tock: Improve the reliability of delays

Using the Tock libtocksync_alarm_delay_ms() syscall has too much
overhead, both in terms of extra time but also jitter. This means it's
not a reliable source for short but accurate delays, such as the kind
used in LoRaWAN.

This patch instead uses a busy loop for short (less then 5 second)
delays. This might have some impact on performance and power as we are
busy running in a loop, but overall we end up with a much more accurate
time and working LoRaWAN.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 examples/NonArduino/Tock/libtockHal.h | 67 +++++++++++++++++++++++++--
 1 file changed, 63 insertions(+), 4 deletions(-)

diff --git a/examples/NonArduino/Tock/libtockHal.h b/examples/NonArduino/Tock/libtockHal.h
index 7ae8bf2a..c21904da 100644
--- a/examples/NonArduino/Tock/libtockHal.h
+++ b/examples/NonArduino/Tock/libtockHal.h
@@ -58,6 +58,11 @@ typedef void (*gpioIrqFn)(void);
 gpioIrqFn gpio_funcs[4] = { NULL, NULL, NULL, NULL};
 uint32_t frequency = 0;
 
+/*
+ * Note that this is the CPU frequency, not the alarm frequency
+ */
+#define CPU_FREQUENCY 48000000
+
 /*
  * Get the the timer frequency in Hz.
  */
@@ -83,6 +88,30 @@ static void lora_phy_gpio_Callback (int gpioPin,
     }
 }
 
+/*
+ * Busy loop for a specified number of nop iterations.
+ *
+ * The Tock `libtocksync_alarm_delay_ms()` functions have too
+ * high of an overhead and not enough accuracy to use for
+ * short (less then 5 seconds) delays with LoRaWAN.
+ *
+ * So instead we just busy loop to ensure we meet timing
+ * requirements.
+ *
+ * This works great when running a single application, but might
+ * cause issues if running multiple applications. A potential fix
+ * to this would be to update the iterations based on time difference.
+ *
+ * For now though this provides us with a lot more accuracy and depending
+ * on the scheduler chosen isn't an issue anyway.
+ */
+static inline void busy_loop_delay(uint32_t iterations)
+{
+    for (int i = 0; i < iterations; i++) {
+        asm("nop");
+    }
+}
+
 class TockHal : public RadioLibHal {
   public:
     // default constructor - initializes the base HAL and any needed private members
@@ -157,19 +186,49 @@ class TockHal : public RadioLibHal {
     }
 
     void delay(unsigned long ms) override {
+      uint32_t delay, loops;
+
 #if !defined(RADIOLIB_CLOCK_DRIFT_MS)
-      libtocksync_alarm_delay_ms(ms);
+      delay = ms;
 #else
-      libtocksync_alarm_delay_ms(ms * 1000 / (1000 + RADIOLIB_CLOCK_DRIFT_MS));
+      delay = ms * 1000 / (1000 + RADIOLIB_CLOCK_DRIFT_MS);
 #endif
+      if (delay < 5 * 1000) {
+        /*
+         * The busy_loop_delay() loop is 5 instructions,
+         * so we divide by 5.
+         */
+        loops = (CPU_FREQUENCY / 5000) * delay;
+
+        if (loops == 0) {
+          return;
+        }
+
+        busy_loop_delay(loops);
+      } else {
+        libtocksync_alarm_delay_ms(delay);
+      }
     }
 
     void delayMicroseconds(unsigned long us) override {
+      uint32_t delay, loops;
+
 #if !defined(RADIOLIB_CLOCK_DRIFT_MS)
-      libtocksync_alarm_delay_ms(us / 1000);
+      delay = us;
 #else
-      libtocksync_alarm_delay_ms((us * 1000 / (1000 + RADIOLIB_CLOCK_DRIFT_MS)) / 1000);
+      delay = us * 1000 / (1000 + RADIOLIB_CLOCK_DRIFT_MS);
 #endif
+        /*
+         * The busy_loop_delay() loop is 5 instructions,
+         * so we divide by 5.
+         */
+      loops = (CPU_FREQUENCY / 5000000) * delay;
+
+      if (loops == 0) {
+        return;
+      }
+
+      busy_loop_delay(loops);
     }
 
     unsigned long millis() override {