Fix STM32F1 motor shocks (stepper timer issue) (#14030)

Marlin/src/HAL/HAL_STM32F1/HAL_timers_STM32F1.cpp

@@ -42,14 +42,10 @@
 // Local defines
 // --------------------------------------------------------------------------
 
-#define NUM_HARDWARE_TIMERS 4
-
-//#define PRESCALER 1
 // --------------------------------------------------------------------------
 // Types
 // --------------------------------------------------------------------------
 
-
 // --------------------------------------------------------------------------
 // Public Variables
 // --------------------------------------------------------------------------
@@ -57,19 +53,7 @@
 // --------------------------------------------------------------------------
 // Private Variables
 // --------------------------------------------------------------------------
-/* VGPV
-const tTimerConfig TimerConfig [NUM_HARDWARE_TIMERS] = {
-  { TC0, 0, TC0_IRQn, 0},  // 0 - [servo timer5]
-  { TC0, 1, TC1_IRQn, 0},  // 1
-  { TC0, 2, TC2_IRQn, 0},  // 2
-  { TC1, 0, TC3_IRQn, 2},  // 3 - stepper
-  { TC1, 1, TC4_IRQn, 15}, // 4 - temperature
-  { TC1, 2, TC5_IRQn, 0},  // 5 - [servo timer3]
-  { TC2, 0, TC6_IRQn, 0},  // 6
-  { TC2, 1, TC7_IRQn, 0},  // 7
-  { TC2, 2, TC8_IRQn, 0},  // 8
-};
-*/
+
 // --------------------------------------------------------------------------
 // Function prototypes
 // --------------------------------------------------------------------------
@@ -101,11 +85,14 @@ void HAL_timer_start(const uint8_t timer_num, const uint32_t frequency) {
     case 3: irq_num = NVIC_TIMER3; break;
     case 4: irq_num = NVIC_TIMER4; break;
     case 5: irq_num = NVIC_TIMER5; break;
+    #ifdef STM32_HIGH_DENSITY
+      // 6 & 7 are basic timers, avoid them
+      case 8: irq_num = NVIC_TIMER8_CC; break;
+    #endif
     default:
       /**
-       *  We should not get here, add Sanitycheck for timer number. Should be a general timer
-       *  since basic timers do not have CC channels.
-       *  Advanced timers should be skipped if possible too, and are not listed above.
+       * This should never happen. Add a Sanitycheck for timer number.
+       * Should be a general timer since basic timers have no CC channels.
        */
       break;
   }
@@ -118,23 +105,27 @@ void HAL_timer_start(const uint8_t timer_num, const uint32_t frequency) {
   switch (timer_num) {
     case STEP_TIMER_NUM:
       timer_pause(STEP_TIMER_DEV);
+      timer_set_mode(STEP_TIMER_DEV, STEP_TIMER_CHAN, TIMER_OUTPUT_COMPARE); // counter
       timer_set_count(STEP_TIMER_DEV, 0);
       timer_set_prescaler(STEP_TIMER_DEV, (uint16_t)(STEPPER_TIMER_PRESCALE - 1));
       timer_set_reload(STEP_TIMER_DEV, 0xFFFF);
+      timer_oc_set_mode(STEP_TIMER_DEV, STEP_TIMER_CHAN, TIMER_OC_MODE_FROZEN, TIMER_OC_NO_PRELOAD); // no output pin change
       timer_set_compare(STEP_TIMER_DEV, STEP_TIMER_CHAN, MIN(hal_timer_t(HAL_TIMER_TYPE_MAX), (STEPPER_TIMER_RATE / frequency)));
+      timer_no_ARR_preload_ARPE(STEP_TIMER_DEV); // Need to be sure no preload on ARR register
       timer_attach_interrupt(STEP_TIMER_DEV, STEP_TIMER_CHAN, stepTC_Handler);
-      nvic_irq_set_priority(irq_num, 1);
+      nvic_irq_set_priority(irq_num, STEP_TIMER_IRQ_PRIO);
       timer_generate_update(STEP_TIMER_DEV);
       timer_resume(STEP_TIMER_DEV);
       break;
     case TEMP_TIMER_NUM:
       timer_pause(TEMP_TIMER_DEV);
+      timer_set_mode(TEMP_TIMER_DEV, TEMP_TIMER_CHAN, TIMER_OUTPUT_COMPARE);
       timer_set_count(TEMP_TIMER_DEV, 0);
       timer_set_prescaler(TEMP_TIMER_DEV, (uint16_t)(TEMP_TIMER_PRESCALE - 1));
       timer_set_reload(TEMP_TIMER_DEV, 0xFFFF);
       timer_set_compare(TEMP_TIMER_DEV, TEMP_TIMER_CHAN, MIN(hal_timer_t(HAL_TIMER_TYPE_MAX), ((F_CPU / TEMP_TIMER_PRESCALE) / frequency)));
       timer_attach_interrupt(TEMP_TIMER_DEV, TEMP_TIMER_CHAN, tempTC_Handler);
-      nvic_irq_set_priority(irq_num, 2);
+      nvic_irq_set_priority(irq_num, TEMP_TIMER_IRQ_PRIO);
       timer_generate_update(TEMP_TIMER_DEV);
       timer_resume(TEMP_TIMER_DEV);
       break;
@@ -145,7 +136,6 @@ void HAL_timer_enable_interrupt(const uint8_t timer_num) {
   switch (timer_num) {
     case STEP_TIMER_NUM: ENABLE_STEPPER_DRIVER_INTERRUPT(); break;
     case TEMP_TIMER_NUM: ENABLE_TEMPERATURE_INTERRUPT(); break;
-    default: break;
   }
 }
 
@@ -153,12 +143,11 @@ void HAL_timer_disable_interrupt(const uint8_t timer_num) {
   switch (timer_num) {
     case STEP_TIMER_NUM: DISABLE_STEPPER_DRIVER_INTERRUPT(); break;
     case TEMP_TIMER_NUM: DISABLE_TEMPERATURE_INTERRUPT(); break;
-    default: break;
   }
 }
 
 static inline bool timer_irq_enabled(const timer_dev * const dev, const uint8_t interrupt) {
-  return bool(*bb_perip(&(dev->regs).adv->DIER, interrupt));
+  return bool(*bb_perip(&(dev->regs).gen->DIER, interrupt));
 }
 
 bool HAL_timer_interrupt_enabled(const uint8_t timer_num) {
@@ -208,12 +197,12 @@ timer_dev* get_timer_dev(int number) {
       case 12: return &timer12;
     #endif
     #if STM32_HAVE_TIMER(13)
-      case 13: return &timer14;
+      case 13: return &timer13;
     #endif
     #if STM32_HAVE_TIMER(14)
       case 14: return &timer14;
     #endif
-      default: return nullptr;
+    default: return nullptr;
   }
 }
 

Marlin/src/HAL/HAL_STM32F1/HAL_timers_STM32F1.h

@@ -45,7 +45,7 @@
 typedef uint16_t hal_timer_t;
 #define HAL_TIMER_TYPE_MAX 0xFFFF
 
-#define HAL_TIMER_RATE         (F_CPU)  // frequency of timers peripherals
+#define HAL_TIMER_RATE uint32_t(F_CPU)  // frequency of timers peripherals
 
 #define STEP_TIMER_CHAN 1 // Channel of the timer to use for compare and interrupts
 #define TEMP_TIMER_CHAN 1 // Channel of the timer to use for compare and interrupts
@@ -60,6 +60,9 @@ typedef uint16_t hal_timer_t;
 #define PULSE_TIMER_NUM STEP_TIMER_NUM
 #define SERVO0_TIMER_NUM 1  // SERVO0 or BLTOUCH
 
+#define STEP_TIMER_IRQ_PRIO 1
+#define TEMP_TIMER_IRQ_PRIO 2
+
 #define TEMP_TIMER_PRESCALE     1000 // prescaler for setting Temp timer, 72Khz
 #define TEMP_TIMER_FREQUENCY    1000 // temperature interrupt frequency
 
@@ -126,43 +129,38 @@ bool HAL_timer_interrupt_enabled(const uint8_t timer_num);
  */
 
 FORCE_INLINE static void HAL_timer_set_compare(const uint8_t timer_num, const hal_timer_t compare) {
-  //compare = MIN(compare, HAL_TIMER_TYPE_MAX);
   switch (timer_num) {
   case STEP_TIMER_NUM:
-    timer_set_compare(STEP_TIMER_DEV, STEP_TIMER_CHAN, compare);
-    return;
+    // NOTE: WE have set ARPE = 0, which means the Auto reload register is not preloaded
+    // and there is no need to use any compare, as in the timer mode used, setting ARR to the compare value
+    // will result in exactly the same effect, ie trigerring an interrupt, and on top, set counter to 0
+    timer_set_reload(STEP_TIMER_DEV, compare); // We reload direct ARR as needed during counting up
+    break;
   case TEMP_TIMER_NUM:
     timer_set_compare(TEMP_TIMER_DEV, TEMP_TIMER_CHAN, compare);
-    return;
-  default:
-    return;
-  }
-}
-
-FORCE_INLINE static hal_timer_t HAL_timer_get_compare(const uint8_t timer_num) {
-  switch (timer_num) {
-  case STEP_TIMER_NUM:
-    return timer_get_compare(STEP_TIMER_DEV, STEP_TIMER_CHAN);
-  case TEMP_TIMER_NUM:
-    return timer_get_compare(TEMP_TIMER_DEV, TEMP_TIMER_CHAN);
-  default:
-    return 0;
+    break;
   }
 }
 
 FORCE_INLINE static void HAL_timer_isr_prologue(const uint8_t timer_num) {
   switch (timer_num) {
   case STEP_TIMER_NUM:
-    timer_set_count(STEP_TIMER_DEV, 0);
+    // No counter to clear
     timer_generate_update(STEP_TIMER_DEV);
     return;
   case TEMP_TIMER_NUM:
     timer_set_count(TEMP_TIMER_DEV, 0);
     timer_generate_update(TEMP_TIMER_DEV);
     return;
-  default:
-    return;
   }
 }
 
 #define HAL_timer_isr_epilogue(TIMER_NUM)
+
+// 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
+FORCE_INLINE static void timer_no_ARR_preload_ARPE(timer_dev *dev) {
+  bb_peri_set_bit(&(dev->regs).gen->CR1, TIMER_CR1_ARPE_BIT, 0);
+}
+
+#define TIMER_OC_NO_PRELOAD 0 // Need to disable preload also on compare registers.

Marlin/src/module/stepper.cpp

@@ -1262,7 +1262,7 @@ void Stepper::isr() {
   // Program timer compare for the maximum period, so it does NOT
   // flag an interrupt while this ISR is running - So changes from small
   // periods to big periods are respected and the timer does not reset to 0
-  HAL_timer_set_compare(STEP_TIMER_NUM, HAL_TIMER_TYPE_MAX);
+  HAL_timer_set_compare(STEP_TIMER_NUM, hal_timer_t(HAL_TIMER_TYPE_MAX));
 
   // Count of ticks for the next ISR
   hal_timer_t next_isr_ticks = 0;