From: Jan "Yenya" Kasprzak <kas@fi.muni.cz>
Date: Fri, 7 Jun 2013 16:15:11 +0000 (+0200)
Subject: Timer handling moved to the main thread
X-Git-Url: https://www.fi.muni.cz/~kas/git//home/kas/public_html/git/?a=commitdiff_plain;h=36b0dc7707312ce8a43c63a43e253325747ff81a;p=bike-lights.git

Timer handling moved to the main thread

In order to mitigate the problem with watchdog reset, possibly caused
by timer IRQ handling taking too long, we only increment the jiffies
value in the WDT IRQ handler, and then read this value in the main
loop, compare with the previous one, and if those two are different,
we run the timer-induced operations. We can (probably) detect the
timer overrun (the difference in jiffies being greater than 1), and log
it.

The individual timer-induced operations are run in their own atomic
blocks for now, in order to be safe. The finer-grained locking is
in the TODO list :-).
---

diff --git a/firmware/main.c b/firmware/main.c
index e6f03b0..b3fcc65 100644
--- a/firmware/main.c
+++ b/firmware/main.c
@@ -1,5 +1,6 @@
 #include <avr/io.h>
 #include <util/delay.h>
+#include <util/atomic.h>
 #include <avr/sleep.h>
 #include <avr/interrupt.h>
 #include <avr/power.h>
@@ -7,6 +8,8 @@
 
 #include "lights.h"
 
+static unsigned char prev_jiffies_8;
+
 static void hw_setup()
 {
 	init_battery();
@@ -83,11 +86,40 @@ static void inline first_boot()
 	hw_setup();
 	power_down(mcusr_save & _BV(WDRF));
 
+	prev_jiffies_8 = jiffies;
+
 	sei();
 }
 
+static void inline do_timer()
+{
+	// For now, we run them all in their own atomic blocks
+	ATOMIC_BLOCK(ATOMIC_FORCEON) {
+		timer_check_buttons();
+	}
+	ATOMIC_BLOCK(ATOMIC_FORCEON) {
+		patterns_next_tick();
+	}
+	ATOMIC_BLOCK(ATOMIC_FORCEON) {
+		pwm_disable_if_not_needed();
+	}
+	ATOMIC_BLOCK(ATOMIC_FORCEON) {
+		timer_start_slow_adcs();
+	}
+	ATOMIC_BLOCK(ATOMIC_FORCEON) {
+		if ((jiffies & 0x7FF) == 0)
+			ambient_log_min_max();
+	}
+}
+
 static void inline main_loop_iteration()
 {
+	unsigned char jiffies_8; /*
+				  * we use only lower 8-bits in order to
+				  * avoid the need for locking of 16-bit
+				  * accesses.
+				  */
+
 	cli();
 	if (TIMER1_IS_ON()) {
 		set_sleep_mode(SLEEP_MODE_IDLE);
@@ -102,6 +134,20 @@ static void inline main_loop_iteration()
 	sei();
 	sleep_cpu();
 	sleep_disable();
+
+	jiffies_8 = jiffies;
+
+	if (jiffies_8 != prev_jiffies_8) { // was timer IRQ
+		if (jiffies_8 - prev_jiffies_8 > 1) { // overrun
+			log_byte(0xee);
+			log_byte(jiffies_8 - prev_jiffies_8);
+			log_flush();
+		}
+
+		prev_jiffies_8 = jiffies_8;
+
+		do_timer();
+	}
 }
 
 int main(void)
diff --git a/firmware/tmr.c b/firmware/tmr.c
index 7a31f19..77e1153 100644
--- a/firmware/tmr.c
+++ b/firmware/tmr.c
@@ -18,16 +18,7 @@ void susp_tmr()
 }
 
 ISR(WDT_vect) {
-	++jiffies;
-
-	timer_check_buttons();
-	patterns_next_tick();
-	pwm_disable_if_not_needed();
-	timer_start_slow_adcs();
-
-	if ((jiffies & 0x7FF) == 0)
-		ambient_log_min_max();
-
 	WDTCR |= _BV(WDIE); // avoid WDT reset next time
+	++jiffies;
 }