From b5acd17db79a1ffa360f595486b707dcec021762 Mon Sep 17 00:00:00 2001 From: JohnR Date: Mon, 19 Dec 2022 11:23:37 +0000 Subject: [PATCH 1/2] Fix potential I2S buffer overwrite Code only checked for one space in DMA buffer but could write two words if nextMainISR and nextAdvanceISR time out at same time. Fix prioritises nextMainISR over LA to only write one word per loop (LA would be delayed by 4usec max. if both time out at same time). --- Marlin/src/HAL/ESP32/i2s.cpp | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/Marlin/src/HAL/ESP32/i2s.cpp b/Marlin/src/HAL/ESP32/i2s.cpp index d9bad4ec2d12..b361e1a0e101 100644 --- a/Marlin/src/HAL/ESP32/i2s.cpp +++ b/Marlin/src/HAL/ESP32/i2s.cpp @@ -148,31 +148,29 @@ void stepperTask(void *parameter) { xQueueReceive(dma.queue, &dma.current, portMAX_DELAY); dma.rw_pos = 0; - while (dma.rw_pos < DMA_SAMPLE_COUNT) { - // Fill with the port data post pulse_phase until the next step - if (nextMainISR && TERN1(LIN_ADVANCE, nextAdvanceISR)) - i2s_push_sample(); - - // i2s_push_sample() is also called from Stepper::pulse_phase_isr() and Stepper::advance_isr() - // in a rare case where both are called, we need to double decrement the counters - const uint8_t push_count = 1 + (!nextMainISR && TERN0(LIN_ADVANCE, !nextAdvanceISR)); - + while (dma.rw_pos < DMA_SAMPLE_COUNT) { + if (!nextMainISR) { + Stepper::pulse_phase_isr(); + nextMainISR = Stepper::block_phase_isr(); + } #if ENABLED(LIN_ADVANCE) - if (!nextAdvanceISR) { + else if (!nextAdvanceISR) { Stepper::advance_isr(); nextAdvanceISR = Stepper::la_interval; } - else if (nextAdvanceISR == Stepper::LA_ADV_NEVER) - nextAdvanceISR = Stepper::la_interval; #endif + else + i2s_push_sample(); - if (!nextMainISR) { - Stepper::pulse_phase_isr(); - nextMainISR = Stepper::block_phase_isr(); - } + nextMainISR--; - nextMainISR -= push_count; - TERN_(LIN_ADVANCE, nextAdvanceISR -= push_count); + #if ENABLED(LIN_ADVANCE) + if (nextAdvanceISR == Stepper::LA_ADV_NEVER) + nextAdvanceISR = Stepper::la_interval; + + if (nextAdvanceISR && nextAdvanceISR != Stepper::LA_ADV_NEVER) + nextAdvanceISR--; + #endif } } } From 5286615faf2fea91122e50c50423b9b9b9d5ff42 Mon Sep 17 00:00:00 2001 From: Scott Lahteine Date: Fri, 30 Dec 2022 23:53:50 -0600 Subject: [PATCH 2/2] ws --- Marlin/src/HAL/ESP32/i2s.cpp | 10 +++++----- Marlin/src/HAL/SAMD21/timers.cpp | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Marlin/src/HAL/ESP32/i2s.cpp b/Marlin/src/HAL/ESP32/i2s.cpp index b361e1a0e101..63ceed4c9dcd 100644 --- a/Marlin/src/HAL/ESP32/i2s.cpp +++ b/Marlin/src/HAL/ESP32/i2s.cpp @@ -148,7 +148,7 @@ void stepperTask(void *parameter) { xQueueReceive(dma.queue, &dma.current, portMAX_DELAY); dma.rw_pos = 0; - while (dma.rw_pos < DMA_SAMPLE_COUNT) { + while (dma.rw_pos < DMA_SAMPLE_COUNT) { if (!nextMainISR) { Stepper::pulse_phase_isr(); nextMainISR = Stepper::block_phase_isr(); @@ -159,16 +159,16 @@ void stepperTask(void *parameter) { nextAdvanceISR = Stepper::la_interval; } #endif - else + else i2s_push_sample(); nextMainISR--; #if ENABLED(LIN_ADVANCE) - if (nextAdvanceISR == Stepper::LA_ADV_NEVER) + if (nextAdvanceISR == Stepper::LA_ADV_NEVER) nextAdvanceISR = Stepper::la_interval; - - if (nextAdvanceISR && nextAdvanceISR != Stepper::LA_ADV_NEVER) + + if (nextAdvanceISR && nextAdvanceISR != Stepper::LA_ADV_NEVER) nextAdvanceISR--; #endif } diff --git a/Marlin/src/HAL/SAMD21/timers.cpp b/Marlin/src/HAL/SAMD21/timers.cpp index bd26ba079f4e..82fa162f1743 100644 --- a/Marlin/src/HAL/SAMD21/timers.cpp +++ b/Marlin/src/HAL/SAMD21/timers.cpp @@ -135,7 +135,7 @@ void HAL_timer_start(const uint8_t timer_num, const uint32_t frequency) { } else if (timer_config[timer_num].type==TimerType::tcc) { - + Tcc * const tc = timer_config[timer_num].pTcc; PM->APBCMASK.reg |= PM_APBCMASK_TCC0;