From 5dc0c80f0b0b553f0b6288d4cdb80264b67b97ba Mon Sep 17 00:00:00 2001
From: Jim McGee <madscifi@jetblackcat.com>
Date: Tue, 10 May 2011 22:36:08 -0700
Subject: [PATCH] Defer enabling of timer1_compa interrupt the end of the
 interrupt handler. Specifically, disable interrupts just before returning and
 then enable the timer interrupt if appropriate. This means that the timer
 interrupt cannot actually fire until after the RETI, so the function cannot
 be entered recursively.

---
 dda.c       |  2 ++
 dda_queue.c | 17 ++++++++++++++---
 timer.c     | 46 ++++++++++++++++++++++++++++++----------------
 timer.h     |  2 ++
 4 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/dda.c b/dda.c
index 00dfdf5..279a837 100644
--- a/dda.c
+++ b/dda.c
@@ -368,6 +368,8 @@ void dda_create(DDA *dda, TARGET *target) {
 	We set direction and enable outputs, and set the timer for the first step from the precalculated value.
 
 	We also mark this DDA as running, so other parts of the firmware know that something is happening
+
+	Called both inside and outside of interrupts.
 */
 void dda_start(DDA *dda) {
 	// called from interrupt context: keep it simple!
diff --git a/dda_queue.c b/dda_queue.c
index aa1869b..a7cbbca 100644
--- a/dda_queue.c
+++ b/dda_queue.c
@@ -130,9 +130,20 @@ void enqueue(TARGET *t) {
 	MEMORY_BARRIER();
 	SREG = save_reg;
 	
-	if (isdead)
+	if (isdead) {
+		timer1_compa_deferred_enable = 0;
 		next_move();
-	}
+		if (timer1_compa_deferred_enable) {
+			uint8_t save_reg = SREG;
+			cli();
+			CLI_SEI_BUG_MEMORY_BARRIER();
+			
+			TIMSK1 |= MASK(OCIE1A);
+			
+			MEMORY_BARRIER();
+			SREG = save_reg;
+		}
+	}	
 }
 
 /// go to the next move.
@@ -163,10 +174,10 @@ void next_move() {
 		else {
 			dda_start(current_movebuffer);
 		}
-		
 	} 
 	if (queue_empty())
 		setTimer(0);
+		
 }
 
 /// DEBUG - print queue.
diff --git a/timer.c b/timer.c
index 8c3b4eb..c3a652e 100644
--- a/timer.c
+++ b/timer.c
@@ -41,6 +41,8 @@ volatile uint8_t	clock_flag_10ms = 0;
 volatile uint8_t	clock_flag_250ms = 0;
 volatile uint8_t	clock_flag_1s = 0;
 
+volatile uint8_t	timer1_compa_deferred_enable = 0;
+
 /// comparator B is the system clock, happens every TICK_TIME
 ISR(TIMER1_COMPB_vect) {
 	// set output compare register to the next clock tick
@@ -69,27 +71,36 @@ ISR(TIMER1_COMPB_vect) {
 }
 
 #ifdef	HOST
-void timer1_compa_isr(void) __attribute__ ((hot));
-void timer1_compa_isr() {
-	// led on
-	WRITE(SCK, 1);
-
-	// disable this interrupt. if we set a new timeout, it will be re-enabled when appropriate
-	TIMSK1 &= ~MASK(OCIE1A);
-
-	// stepper tick
-	queue_step();
-
-	// led off
-	WRITE(SCK, 0);
-}
 
 /// comparator A is the step timer. It has higher priority then B.
 ISR(TIMER1_COMPA_vect) {
 	// Check if this is a real step, or just a next_step_time "overflow"
 	if (next_step_time < 65536) {
 		// step!
-		timer1_compa_isr();
+		WRITE(SCK, 1);
+
+		// disable this interrupt. if we set a new timeout, it will be re-enabled when appropriate
+		TIMSK1 &= ~MASK(OCIE1A);
+		timer1_compa_deferred_enable = 0;
+		
+		// stepper tick
+		queue_step();
+
+		// led off
+		WRITE(SCK, 0);
+		
+		// Enable the timer1_compa interrupt, if needed, 
+		// but only do it after disabling global interrupts.
+		// This will cause push any possible timer1a interrupt
+		// to the far side of the return, protecting the 
+		// stack from recursively clobbering memory.
+		
+		cli();
+		CLI_SEI_BUG_MEMORY_BARRIER();
+		
+		if (timer1_compa_deferred_enable) {
+			TIMSK1 |= MASK(OCIE1A);
+		}
 		return;
 	}
 
@@ -169,11 +180,14 @@ void setTimer(uint32_t delay)
 			OCR1A = step_start;
 		}
 
-		TIMSK1 |= MASK(OCIE1A);
+		// Defer the enabling of the timer1_CompA interrupts.
+		
+		timer1_compa_deferred_enable = 1;
 	} else {
 		// flag: move has ended
 		next_step_time = 0;
 		TIMSK1 &= ~MASK(OCIE1A);
+		timer1_compa_deferred_enable = 0;
 	}
 
 	// restore interrupt flag
diff --git a/timer.h b/timer.h
index 9edc71b..702a20a 100644
--- a/timer.h
+++ b/timer.h
@@ -15,6 +15,8 @@ extern volatile uint8_t	clock_flag_10ms;
 extern volatile uint8_t	clock_flag_250ms;
 extern volatile uint8_t	clock_flag_1s;
 
+extern volatile uint8_t	timer1_compa_deferred_enable;
+
 // If the specific bit is set, execute the following block exactly once
 // and then clear the flag.
 #define	ifclock(F)	for (;F;F=0 )