🐛 Fix Teensy 4.x stepper timing (#28169)

thinkyhead · thinkyhead · commit e4463975eb5c · 2025-11-20T15:29:50.000-06:00
🧑‍💻 Timer general cleanup
🩹 Teensy 4.x timer mods
diff --git a/Marlin/src/HAL/AVR/timers.h b/Marlin/src/HAL/AVR/timers.h
@@ -109,8 +109,8 @@ FORCE_INLINE void HAL_timer_start(const uint8_t timer_num, const uint32_t) {
  * (otherwise, characters will be lost due to UART overflow).
  * Then: Stepper, Endstops, Temperature, and -finally- all others.
  */
-#define HAL_timer_isr_prologue(T) NOOP
-#define HAL_timer_isr_epilogue(T) NOOP
+inline void HAL_timer_isr_prologue(const uint8_t) {}
+inline void HAL_timer_isr_epilogue(const uint8_t) {}
 
 #ifndef HAL_STEP_TIMER_ISR
 
diff --git a/Marlin/src/HAL/DUE/timers.h b/Marlin/src/HAL/DUE/timers.h
@@ -126,4 +126,4 @@ FORCE_INLINE static void HAL_timer_isr_prologue(const uint8_t timer_num) {
   pConfig->pTimerRegs->TC_CHANNEL[pConfig->channel].TC_SR;
 }
 
-#define HAL_timer_isr_epilogue(T) NOOP
+inline void HAL_timer_isr_epilogue(const uint8_t) {}
diff --git a/Marlin/src/HAL/ESP32/timers.h b/Marlin/src/HAL/ESP32/timers.h
@@ -136,5 +136,5 @@ void HAL_timer_enable_interrupt(const uint8_t timer_num);
 void HAL_timer_disable_interrupt(const uint8_t timer_num);
 bool HAL_timer_interrupt_enabled(const uint8_t timer_num);
 
-#define HAL_timer_isr_prologue(T) NOOP
-#define HAL_timer_isr_epilogue(T) NOOP
+inline void HAL_timer_isr_prologue(const uint8_t) {}
+inline void HAL_timer_isr_epilogue(const uint8_t) {}
diff --git a/Marlin/src/HAL/LINUX/timers.h b/Marlin/src/HAL/LINUX/timers.h
@@ -92,5 +92,5 @@ void HAL_timer_enable_interrupt(const uint8_t timer_num);
 void HAL_timer_disable_interrupt(const uint8_t timer_num);
 bool HAL_timer_interrupt_enabled(const uint8_t timer_num);
 
-#define HAL_timer_isr_prologue(T) NOOP
-#define HAL_timer_isr_epilogue(T) NOOP
+inline void HAL_timer_isr_prologue(const uint8_t) {}
+inline void HAL_timer_isr_epilogue(const uint8_t) {}
diff --git a/Marlin/src/HAL/LPC1768/timers.h b/Marlin/src/HAL/LPC1768/timers.h
@@ -170,4 +170,4 @@ FORCE_INLINE static void HAL_timer_isr_prologue(const uint8_t timer_num) {
   }
 }
 
-#define HAL_timer_isr_epilogue(T) NOOP
+inline void HAL_timer_isr_epilogue(const uint8_t) {}
diff --git a/Marlin/src/HAL/NATIVE_SIM/timers.h b/Marlin/src/HAL/NATIVE_SIM/timers.h
@@ -87,5 +87,5 @@ void HAL_timer_enable_interrupt(const uint8_t timer_num);
 void HAL_timer_disable_interrupt(const uint8_t timer_num);
 bool HAL_timer_interrupt_enabled(const uint8_t timer_num);
 
-#define HAL_timer_isr_prologue(T) NOOP
-#define HAL_timer_isr_epilogue(T) NOOP
+inline void HAL_timer_isr_prologue(const uint8_t) {}
+inline void HAL_timer_isr_epilogue(const uint8_t) {}
diff --git a/Marlin/src/HAL/SAMD51/timers.h b/Marlin/src/HAL/SAMD51/timers.h
@@ -140,4 +140,4 @@ FORCE_INLINE static void HAL_timer_isr_prologue(const uint8_t timer_num) {
   }
 }
 
-#define HAL_timer_isr_epilogue(timer_num)
+inline void HAL_timer_isr_epilogue(const uint8_t) {}
diff --git a/Marlin/src/HAL/STM32/timers.h b/Marlin/src/HAL/STM32/timers.h
@@ -116,5 +116,5 @@ FORCE_INLINE static void HAL_timer_set_compare(const uint8_t timer_num, const ha
   }
 }
 
-#define HAL_timer_isr_prologue(T) NOOP
-#define HAL_timer_isr_epilogue(T) NOOP
+inline void HAL_timer_isr_prologue(const uint8_t) {}
+inline void HAL_timer_isr_epilogue(const uint8_t) {}
diff --git a/Marlin/src/HAL/STM32F1/timers.h b/Marlin/src/HAL/STM32F1/timers.h
@@ -188,7 +188,7 @@ FORCE_INLINE static void HAL_timer_isr_prologue(const uint8_t timer_num) {
   }
 }
 
-#define HAL_timer_isr_epilogue(T) NOOP
+inline void HAL_timer_isr_epilogue(const uint8_t) {}
 
 // No command is available in framework to turn off ARPE bit, which is turned on by default in libmaple.
 // Needed here to reset ARPE=0 for stepper timer
diff --git a/Marlin/src/HAL/TEENSY31_32/timers.h b/Marlin/src/HAL/TEENSY31_32/timers.h
@@ -74,10 +74,10 @@ typedef uint32_t hal_timer_t;
 #define DISABLE_TEMPERATURE_INTERRUPT() HAL_timer_disable_interrupt(MF_TIMER_TEMP)
 
 #ifndef HAL_STEP_TIMER_ISR
-  #define HAL_STEP_TIMER_ISR() extern "C" void ftm0_isr() //void TC3_Handler()
+  #define HAL_STEP_TIMER_ISR() extern "C" void ftm0_isr()
 #endif
 #ifndef HAL_TEMP_TIMER_ISR
-  #define HAL_TEMP_TIMER_ISR() extern "C" void ftm1_isr() //void TC4_Handler()
+  #define HAL_TEMP_TIMER_ISR() extern "C" void ftm1_isr()
 #endif
 
 void HAL_timer_start(const uint8_t timer_num, const uint32_t frequency);
@@ -110,4 +110,4 @@ void HAL_timer_disable_interrupt(const uint8_t timer_num);
 bool HAL_timer_interrupt_enabled(const uint8_t timer_num);
 
 void HAL_timer_isr_prologue(const uint8_t timer_num);
-#define HAL_timer_isr_epilogue(T) NOOP
+inline void HAL_timer_isr_epilogue(const uint8_t) {}
diff --git a/Marlin/src/HAL/TEENSY35_36/timers.h b/Marlin/src/HAL/TEENSY35_36/timers.h
@@ -109,4 +109,4 @@ void HAL_timer_disable_interrupt(const uint8_t timer_num);
 bool HAL_timer_interrupt_enabled(const uint8_t timer_num);
 
 void HAL_timer_isr_prologue(const uint8_t timer_num);
-#define HAL_timer_isr_epilogue(T) NOOP
+inline void HAL_timer_isr_epilogue(const uint8_t) {}
diff --git a/Marlin/src/HAL/TEENSY40_41/timers.cpp b/Marlin/src/HAL/TEENSY40_41/timers.cpp
@@ -30,41 +30,82 @@
 
 void HAL_timer_start(const uint8_t timer_num, const uint32_t frequency) {
   switch (timer_num) {
+
+    //
+    // Step Timer – GPT1 - Compare Interrupt OCR1 - Reset Mode
+    //
     case MF_TIMER_STEP:
-      CCM_CSCMR1 &= ~CCM_CSCMR1_PERCLK_CLK_SEL; // turn off 24mhz mode
+      // 24MHz mode off – Use peripheral clock (150MHz)
+      CCM_CSCMR1 &= ~CCM_CSCMR1_PERCLK_CLK_SEL;
+      // Enable GPT1 clock gating
       CCM_CCGR1 |= CCM_CCGR1_GPT1_BUS(CCM_CCGR_ON);
 
-      GPT1_CR = 0;                   // disable timer
-      GPT1_SR = 0x3F;                // clear all prior status
+      // Disable timer, clear all status bits
+      GPT1_CR = 0;                      // Disable timer
+      GPT1_SR = 0x3F;                   // Clear all prior status
+
+      // Prescaler = 2 => 75MHz counting clock
       GPT1_PR = GPT1_TIMER_PRESCALE - 1;
-      GPT1_CR |= GPT_CR_CLKSRC(1);   //clock selection #1 (peripheral clock = 150 MHz)
-      GPT1_CR |= GPT_CR_ENMOD;       //reset count to zero before enabling
-      GPT1_CR |= GPT_CR_OM1(1);      // toggle mode
-      GPT1_OCR1 = (GPT1_TIMER_RATE / frequency) -1; // Initial compare value
-      GPT1_IR = GPT_IR_OF1IE;        // Compare3 value
-      GPT1_CR |= GPT_CR_EN;          //enable GPT2 counting at 150 MHz
-
-      OUT_WRITE(15, HIGH);
+
+      GPT1_CR = GPT_CR_CLKSRC(1)        // Clock selection #1 (peripheral clock = 150 MHz)
+              | GPT_CR_ENMOD            // Reset count to zero before enabling
+              | GPT_CR_OM2(TERN(MARLIN_DEV_MODE, 1, 0)); // 0 = edge compare, 1 = toggle
+
+      // Compare value – the number of clocks between edges
+      GPT1_OCR1 = (GPT1_TIMER_RATE / frequency) - 1;
+
+      // Enable compare‑event interrupt
+      GPT1_IR = GPT_IR_OF1IE;           // OF1 interrupt enabled
+
+      // Pull Pin 15 HIGH (logic‑high is the “idle” state)
+      TERN_(MARLIN_DEV_MODE, OUT_WRITE(15, HIGH));
+
+      // Attach and enable Stepper IRQ
+      // Note: UART priority is 16
       attachInterruptVector(IRQ_GPT1, &stepTC_Handler);
-      NVIC_SET_PRIORITY(IRQ_GPT1, 16);
+      NVIC_SET_PRIORITY(IRQ_GPT1, 16);  // Priority 16 (higher than Temp Timer)
+
+      // Start GPT1 counting at 150 MHz
+      GPT1_CR |= GPT_CR_EN;
+
       break;
+
+    //
+    // Temperature Timer – GPT2 - Compare Interrupt OCR1 - Reset Mode
+    //
     case MF_TIMER_TEMP:
-      CCM_CSCMR1 &= ~CCM_CSCMR1_PERCLK_CLK_SEL; // turn off 24mhz mode
+      // 24MHz mode off – Use peripheral clock (150MHz)
+      CCM_CSCMR1 &= ~CCM_CSCMR1_PERCLK_CLK_SEL;
+      // Enable GPT2 clock gating
       CCM_CCGR0 |= CCM_CCGR0_GPT2_BUS(CCM_CCGR_ON);
 
-      GPT2_CR = 0;                   // disable timer
-      GPT2_SR = 0x3F;                // clear all prior status
+      // Disable timer, clear all status bits
+      GPT2_CR = 0;                      // Disable timer
+      GPT2_SR = 0x3F;                   // Clear all prior status
+
+      // Prescaler = 10 => 15MHz counting clock
       GPT2_PR = GPT2_TIMER_PRESCALE - 1;
-      GPT2_CR |= GPT_CR_CLKSRC(1);   //clock selection #1 (peripheral clock = 150 MHz)
-      GPT2_CR |= GPT_CR_ENMOD;       //reset count to zero before enabling
-      GPT2_CR |= GPT_CR_OM1(1);      // toggle mode
-      GPT2_OCR1 = (GPT2_TIMER_RATE / frequency) -1; // Initial compare value
-      GPT2_IR = GPT_IR_OF1IE;        // Compare3 value
-      GPT2_CR |= GPT_CR_EN;          //enable GPT2 counting at 150 MHz
-
-      OUT_WRITE(14, HIGH);
+
+      GPT2_CR = GPT_CR_CLKSRC(1)        // Clock selection #1 (peripheral clock = 150 MHz)
+              | GPT_CR_ENMOD            // and reset count to zero before enabling
+              | GPT_CR_OM2(TERN(MARLIN_DEV_MODE, 1, 0)); // 0 = edge compare, 1 = toggle
+
+      // Compare value – the number of clocks between edges
+      GPT2_OCR1 = (GPT2_TIMER_RATE / frequency) - 1;
+
+      // Enable compare‑event interrupt
+      GPT2_IR = GPT_IR_OF1IE;           // OF1 interrupt enabled
+
+      // Pull Pin 14 HIGH (logic‑high is the “idle” state)
+      TERN_(MARLIN_DEV_MODE, OUT_WRITE(14, HIGH));
+
+      // Attach Temperature ISR
       attachInterruptVector(IRQ_GPT2, &tempTC_Handler);
-      NVIC_SET_PRIORITY(IRQ_GPT2, 32);
+      NVIC_SET_PRIORITY(IRQ_GPT2, 32);  // Priority 32 (lower than Step Timer)
+
+      // Start GPT2 counting at 150 MHz
+      GPT2_CR |= GPT_CR_EN;
+
       break;
   }
 }
@@ -82,6 +123,7 @@ void HAL_timer_disable_interrupt(const uint8_t timer_num) {
     case MF_TIMER_TEMP: NVIC_DISABLE_IRQ(IRQ_GPT2); break;
   }
 
+  // Ensure the CPU actually stops servicing the IRQ
   // We NEED memory barriers to ensure Interrupts are actually disabled!
   // ( https://dzone.com/articles/nvic-disabling-interrupts-on-arm-cortex-m-and-the )
   asm volatile("dsb");
@@ -97,8 +139,8 @@ bool HAL_timer_interrupt_enabled(const uint8_t timer_num) {
 
 void HAL_timer_isr_prologue(const uint8_t timer_num) {
   switch (timer_num) {
-    case MF_TIMER_STEP: GPT1_SR = GPT_IR_OF1IE; break; // clear OF3 bit
-    case MF_TIMER_TEMP: GPT2_SR = GPT_IR_OF1IE; break; // clear OF3 bit
+    case MF_TIMER_STEP: GPT1_SR = GPT_IR_OF1IE; break;   // clear OF1
+    case MF_TIMER_TEMP: GPT2_SR = GPT_IR_OF1IE; break;
   }
   asm volatile("dsb");
 }
diff --git a/Marlin/src/HAL/TEENSY40_41/timers.h b/Marlin/src/HAL/TEENSY40_41/timers.h
@@ -58,7 +58,7 @@ typedef uint32_t hal_timer_t;
 
 #define STEPPER_TIMER_RATE     GPT1_TIMER_RATE
 #define STEPPER_TIMER_TICKS_PER_US ((STEPPER_TIMER_RATE) / 1000000)
-#define STEPPER_TIMER_PRESCALE ((GPT_TIMER_RATE / 1000000) / STEPPER_TIMER_TICKS_PER_US)
+#define STEPPER_TIMER_PRESCALE (GPT_TIMER_RATE / STEPPER_TIMER_RATE)
 
 #define PULSE_TIMER_RATE       STEPPER_TIMER_RATE   // frequency of pulse timer
 #define PULSE_TIMER_PRESCALE   STEPPER_TIMER_PRESCALE
@@ -87,8 +87,16 @@ void HAL_timer_start(const uint8_t timer_num, const uint32_t frequency);
 
 FORCE_INLINE static void HAL_timer_set_compare(const uint8_t timer_num, const hal_timer_t compare) {
   switch (timer_num) {
-    case MF_TIMER_STEP: GPT1_OCR1 = compare - 1; break;
-    case MF_TIMER_TEMP: GPT2_OCR1 = compare - 1; break;
+    case MF_TIMER_STEP:
+      GPT1_CR |= GPT_CR_FRR;    // Free Run Mode (setting OCRx preserves CNT)
+      GPT1_OCR1 = compare - 1;
+      GPT1_CR &= ~GPT_CR_FRR;   // Reset Mode (CNT resets on trigger)
+      break;
+    case MF_TIMER_TEMP:
+      GPT2_CR |= GPT_CR_FRR;    // Free Run Mode (setting OCRx preserves CNT)
+      GPT2_OCR1 = compare - 1;
+      GPT2_CR &= ~GPT_CR_FRR;   // Reset Mode (CNT resets on trigger)
+      break;
   }
 }
 
@@ -113,5 +121,4 @@ void HAL_timer_disable_interrupt(const uint8_t timer_num);
 bool HAL_timer_interrupt_enabled(const uint8_t timer_num);
 
 void HAL_timer_isr_prologue(const uint8_t timer_num);
-//void HAL_timer_isr_epilogue(const uint8_t timer_num) {}
-#define HAL_timer_isr_epilogue(T) NOOP
+inline void HAL_timer_isr_epilogue(const uint8_t) {}
diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp
@@ -1430,11 +1430,7 @@ HAL_STEP_TIMER_ISR() {
   HAL_timer_isr_epilogue(MF_TIMER_STEP);
 }
 
-#ifdef CPU_32_BIT
-  #define STEP_MULTIPLY(A,B) MultiU32X24toH32(A, B)
-#else
-  #define STEP_MULTIPLY(A,B) MultiU24X32toH16(A, B)
-#endif
+#define STEP_MULTIPLY(A,B) TERN(CPU_32_BIT, MultiU32X24toH32, MultiU24X32toH16)(A, B)
 
 void Stepper::isr() {
 

Original file line number	Diff line number	Diff line change
`@@ -126,4 +126,4 @@ FORCE_INLINE static void HAL_timer_isr_prologue(const uint8_t timer_num) {`
`126`	`126`	`pConfig->pTimerRegs->TC_CHANNEL[pConfig->channel].TC_SR;`
`127`	`127`	`}`
`128`	`128`
`129`		`-#define HAL_timer_isr_epilogue(T) NOOP`
	`129`	`+inline void HAL_timer_isr_epilogue(const uint8_t) {}`
Original file line number	Diff line number	Diff line change
`@@ -170,4 +170,4 @@ FORCE_INLINE static void HAL_timer_isr_prologue(const uint8_t timer_num) {`
`170`	`170`	`}`
`171`	`171`	`}`
`172`	`172`
`173`		`-#define HAL_timer_isr_epilogue(T) NOOP`
	`173`	`+inline void HAL_timer_isr_epilogue(const uint8_t) {}`
Original file line number	Diff line number	Diff line change
`@@ -140,4 +140,4 @@ FORCE_INLINE static void HAL_timer_isr_prologue(const uint8_t timer_num) {`
`140`	`140`	`}`
`141`	`141`	`}`
`142`	`142`
`143`		`-#define HAL_timer_isr_epilogue(timer_num)`
	`143`	`+inline void HAL_timer_isr_epilogue(const uint8_t) {}`
Original file line number	Diff line number	Diff line change
`@@ -116,5 +116,5 @@ FORCE_INLINE static void HAL_timer_set_compare(const uint8_t timer_num, const ha`
`116`	`116`	`}`
`117`	`117`	`}`
`118`	`118`
`119`		`-#define HAL_timer_isr_prologue(T) NOOP`
`120`		`-#define HAL_timer_isr_epilogue(T) NOOP`
	`119`	`+inline void HAL_timer_isr_prologue(const uint8_t) {}`
	`120`	`+inline void HAL_timer_isr_epilogue(const uint8_t) {}`
Original file line number	Diff line number	Diff line change
`@@ -188,7 +188,7 @@ FORCE_INLINE static void HAL_timer_isr_prologue(const uint8_t timer_num) {`
`188`	`188`	`}`
`189`	`189`	`}`
`190`	`190`
`191`		`-#define HAL_timer_isr_epilogue(T) NOOP`
	`191`	`+inline void HAL_timer_isr_epilogue(const uint8_t) {}`
`192`	`192`
`193`	`193`	`// No command is available in framework to turn off ARPE bit, which is turned on by default in libmaple.`
`194`	`194`	`// Needed here to reset ARPE=0 for stepper timer`