From ee04743a8522682b4e5d878c6d06fa67c38dc3de Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Fri, 24 Mar 2023 07:58:16 +0000 Subject: [PATCH 1/3] Shifting before calc_timer_interval() can cause a hang with LA --- Marlin/src/module/stepper.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index 5c0034a5f991..314141c9a093 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -2254,7 +2254,7 @@ hal_timer_t Stepper::block_phase_isr() { #if ENABLED(LIN_ADVANCE) if (la_active) { const uint32_t la_step_rate = la_advance_steps < current_block->max_adv_steps ? current_block->la_advance_rate : 0; - la_interval = calc_timer_interval((acc_step_rate + la_step_rate) >> current_block->la_scaling); + la_interval = calc_timer_interval(acc_step_rate + la_step_rate) << current_block->la_scaling; } #endif @@ -2326,7 +2326,7 @@ hal_timer_t Stepper::block_phase_isr() { const uint32_t la_step_rate = la_advance_steps > current_block->final_adv_steps ? current_block->la_advance_rate : 0; if (la_step_rate != step_rate) { bool reverse_e = la_step_rate > step_rate; - la_interval = calc_timer_interval((reverse_e ? la_step_rate - step_rate : step_rate - la_step_rate) >> current_block->la_scaling); + la_interval = calc_timer_interval(reverse_e ? la_step_rate - step_rate : step_rate - la_step_rate) << current_block->la_scaling; if (reverse_e != motor_direction(E_AXIS)) { TBI(last_direction_bits, E_AXIS); @@ -2384,7 +2384,7 @@ hal_timer_t Stepper::block_phase_isr() { #if ENABLED(LIN_ADVANCE) if (la_active) - la_interval = calc_timer_interval(current_block->nominal_rate >> current_block->la_scaling); + la_interval = calc_timer_interval(current_block->nominal_rate) << current_block->la_scaling; #endif } @@ -2706,7 +2706,7 @@ hal_timer_t Stepper::block_phase_isr() { #if ENABLED(LIN_ADVANCE) if (la_active) { const uint32_t la_step_rate = la_advance_steps < current_block->max_adv_steps ? current_block->la_advance_rate : 0; - la_interval = calc_timer_interval((current_block->initial_rate + la_step_rate) >> current_block->la_scaling); + la_interval = calc_timer_interval(current_block->initial_rate + la_step_rate) << current_block->la_scaling; } #endif } From a120dfe91c57ed7842a07182e0d780fe51ffbf09 Mon Sep 17 00:00:00 2001 From: Tom Brazier Date: Fri, 24 Mar 2023 09:09:55 +0000 Subject: [PATCH 2/3] A better approach: division by zero is always due to integer rounding and the right answer is to give an infinite timer interval --- Marlin/src/module/stepper.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index 314141c9a093..0f6d886b7b3e 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -2094,7 +2094,10 @@ hal_timer_t Stepper::calc_timer_interval(uint32_t step_rate) { #ifdef CPU_32_BIT - return uint32_t(STEPPER_TIMER_RATE) / step_rate; // A fast processor can just do integer division + if (step_rate != 0) + return uint32_t(STEPPER_TIMER_RATE) / step_rate; // A fast processor can just do integer division + + return HAL_TIMER_TYPE_MAX; #else @@ -2254,7 +2257,7 @@ hal_timer_t Stepper::block_phase_isr() { #if ENABLED(LIN_ADVANCE) if (la_active) { const uint32_t la_step_rate = la_advance_steps < current_block->max_adv_steps ? current_block->la_advance_rate : 0; - la_interval = calc_timer_interval(acc_step_rate + la_step_rate) << current_block->la_scaling; + la_interval = calc_timer_interval((acc_step_rate + la_step_rate) >> current_block->la_scaling); } #endif @@ -2326,7 +2329,7 @@ hal_timer_t Stepper::block_phase_isr() { const uint32_t la_step_rate = la_advance_steps > current_block->final_adv_steps ? current_block->la_advance_rate : 0; if (la_step_rate != step_rate) { bool reverse_e = la_step_rate > step_rate; - la_interval = calc_timer_interval(reverse_e ? la_step_rate - step_rate : step_rate - la_step_rate) << current_block->la_scaling; + la_interval = calc_timer_interval((reverse_e ? la_step_rate - step_rate : step_rate - la_step_rate) >> current_block->la_scaling); if (reverse_e != motor_direction(E_AXIS)) { TBI(last_direction_bits, E_AXIS); @@ -2384,7 +2387,7 @@ hal_timer_t Stepper::block_phase_isr() { #if ENABLED(LIN_ADVANCE) if (la_active) - la_interval = calc_timer_interval(current_block->nominal_rate) << current_block->la_scaling; + la_interval = calc_timer_interval(current_block->nominal_rate >> current_block->la_scaling); #endif } @@ -2706,7 +2709,7 @@ hal_timer_t Stepper::block_phase_isr() { #if ENABLED(LIN_ADVANCE) if (la_active) { const uint32_t la_step_rate = la_advance_steps < current_block->max_adv_steps ? current_block->la_advance_rate : 0; - la_interval = calc_timer_interval(current_block->initial_rate + la_step_rate) << current_block->la_scaling; + la_interval = calc_timer_interval((current_block->initial_rate + la_step_rate) >> current_block->la_scaling); } #endif } From 2f18457e04dcd0d70b527500079c0435770a8912 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Sat, 25 Mar 2023 20:18:31 -0500 Subject: [PATCH 3/3] style tweaks --- Marlin/src/module/stepper.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Marlin/src/module/stepper.cpp b/Marlin/src/module/stepper.cpp index 0f6d886b7b3e..f7436e51d538 100644 --- a/Marlin/src/module/stepper.cpp +++ b/Marlin/src/module/stepper.cpp @@ -2094,10 +2094,8 @@ hal_timer_t Stepper::calc_timer_interval(uint32_t step_rate) { #ifdef CPU_32_BIT - if (step_rate != 0) - return uint32_t(STEPPER_TIMER_RATE) / step_rate; // A fast processor can just do integer division - - return HAL_TIMER_TYPE_MAX; + // A fast processor can just do integer division + return step_rate ? uint32_t(STEPPER_TIMER_RATE) / step_rate : HAL_TIMER_TYPE_MAX; #else