-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix ESC spinup during settings save using circular DMA #11260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: maintenance-9.x
Are you sure you want to change the base?
Fix ESC spinup during settings save using circular DMA #11260
Conversation
This approach doesn't prevent DShot interruption during EEPROM writes. Committing for potential future refinement. Changes: - Added impl_timerPWMSetDMACircular() to switch DMA mode at runtime - Modified processDelayedSave() to use circular mode during writeEEPROM() - Called pwmCompleteMotorUpdate() 3x to latch DShot 0 packets Issue: DShot still shows gaps during settings save on oscilloscope. Next approach: Test with simple GPIO high instead of DShot.
Move circular DShot DMA code from processDelayedSave() to writeConfigToEEPROM(). This ensures the fix works for MSP_EEPROM_WRITE commands, not just delayed saves. The MSP call path is: MSP_EEPROM_WRITE → writeEEPROM() → writeConfigToEEPROM() → writeSettingsToEEPROM() The previous commit (a6ba116) had circular DMA in processDelayedSave(), which is only called for delayed saves (on disarm), not MSP commands. Changes: - Move circular DMA setup to writeConfigToEEPROM() in config_eeprom.c - Remove unused pwmSetMotorPinsHigh() function - Add pwm_output.h include to config_eeprom.c Test method: - MSP_EEPROM_WRITE command sent once per second - DShot signal monitored on oscilloscope - Confirmed: DShot no longer interrupted during settings save Issue: iNavFlight#10913 Related: iNavFlight#9441
Based on code-reviewer agent feedback: 1. Add missing AT32 platform implementation - Implement impl_timerPWMSetDMACircular() for AT32F43x targets - Uses AT32 loop_mode (ctrl_bit.lm) instead of DMA_SxCR_CIRC 2. Remove duplicate circular DMA code from config.c - processDelayedSave() calls writeEEPROM() which calls writeConfigToEEPROM() - writeConfigToEEPROM() already has circular DMA protection - Removed redundant nested enable/disable from config.c 3. Add ATOMIC_BLOCK protection to DMA mode switch - Consistent with existing impl_timerPWMStopDMA() pattern - Prevents interrupt interference during DMA reconfiguration - Applied to HAL, StdPeriph, and AT32 implementations Issue: iNavFlight#10913
Critical fixes for STM32H7 DMA circular mode: - Wait for EN bit to actually clear before changing mode (was the primary bug) - Disable/re-enable timer DMA requests during reconfiguration - Reload DMA transfer count after mode change - Clear pending DMA flags Without these changes, the mode change was being ignored because the DMA stream was still active when we tried to modify the configuration.
PR Compliance Guide 🔍All compliance sections have been disabled in the configurations. |
SITL doesn't have real PWM/motor hardware, so pwmSetMotorDMACircular() and pwmCompleteMotorUpdate() don't exist in SITL builds. Wrap these calls with #if \!defined(SITL_BUILD) to allow SITL builds to compile while preserving the ESC spinup fix for hardware builds.
Address qodo-code-review feedback: Add defensive timeout checks when waiting for DMA streams/channels to disable before reconfiguring. Changes: - H7 (timer_impl_hal.c): Check if timeout expired and abort if DMA still enabled - F4/F7 (timer_impl_stdperiph.c): Add wait loop for EN bit to clear with timeout check - AT32 (timer_impl_stdperiph_at32.c): Add wait loop for chen bit to clear with timeout check This prevents potential race conditions where DMA configuration could be modified while the stream is still active, which could cause unstable behavior.
Some hardware targets don't compile DShot support, causing linker errors when trying to call pwmSetMotorDMACircular() and pwmCompleteMotorUpdate(). Change guard from: #if \!defined(SITL_BUILD) To: #if \!defined(SITL_BUILD) && defined(USE_DSHOT) This ensures the functions are only called on targets that actually have DShot compiled in, fixing build failures on targets like BEEROTORF4.
- config.c: Remove comment about circular DMA protection location (obvious from context) - timer_impl_stdperiph_at32.c: Remove redundant comment about loop mode (already clear from 'Enable loop mode' / 'Disable loop mode' comments)
|
I ran some props removed bench testing with Dshot beeper enabled. And tried many conditions to make the motors start when disarmed. Even saving files to eeprom via the configurator connection. Which used to occasionally trigger uncontrolled motor startup. But it will likely require testing with a broader range of FC boards that log with SDIO, due the use of circular mode. Because it might cause conflicts depending on the allocation of the DMA streams on some flight controllers. |
|
Thanks Jetrell! From earlier testing I did, and then further research, it seems to be entirely driven by writing to the internal EEPROM (the same place the code is stored). The EEPROM can't be written and read at the same time, so code can't execute while writing to the EEPROM. Writing to a port that happens to have some kind of external flash on the other end doesn't seem to matter, because the external flash doesn't need to be read for code to execute. That is, it's not writing the storage that it is trying to run code from. So blackbox doesn't matter. I tested the blackbox stuff more extensively than I intended to, and found that it doesn't produce a glitch. :) Eventually realizing my mistake, I wrote to EEPROM instead and the DSHOT output immediately glitched - exactly like the STM documentation said it would. 😁 |
PR Compliance Guide 🔍All compliance sections have been disabled in the configurations. |
| uint32_t timeout = 10000; | ||
| while (LL_DMA_IsEnabledStream(dmaBase, streamLL) && timeout--) { | ||
| __NOP(); | ||
| } | ||
|
|
||
| // If timeout occurred, DMA stream is still enabled - abort reconfiguration | ||
| if (timeout == 0 && LL_DMA_IsEnabledStream(dmaBase, streamLL)) { | ||
| // Re-enable timer DMA request and return to avoid unstable state | ||
| LL_TIM_EnableDMAReq_CCx(tch->timHw->tim, lookupDMASourceTable[tch->timHw->channelIndex]); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Fix the incorrect DMA timeout check by re-evaluating the stream status after the loop instead of checking if timeout is zero. [possible issue, importance: 8]
| uint32_t timeout = 10000; | |
| while (LL_DMA_IsEnabledStream(dmaBase, streamLL) && timeout--) { | |
| __NOP(); | |
| } | |
| // If timeout occurred, DMA stream is still enabled - abort reconfiguration | |
| if (timeout == 0 && LL_DMA_IsEnabledStream(dmaBase, streamLL)) { | |
| // Re-enable timer DMA request and return to avoid unstable state | |
| LL_TIM_EnableDMAReq_CCx(tch->timHw->tim, lookupDMASourceTable[tch->timHw->channelIndex]); | |
| return; | |
| } | |
| uint32_t timeout = 10000; | |
| while (LL_DMA_IsEnabledStream(dmaBase, streamLL) && timeout--) { | |
| __NOP(); | |
| } | |
| // If timeout occurred, DMA stream is still enabled - abort reconfiguration | |
| if (LL_DMA_IsEnabledStream(dmaBase, streamLL)) { | |
| // Re-enable timer DMA request and return to avoid unstable state | |
| LL_TIM_EnableDMAReq_CCx(tch->timHw->tim, lookupDMASourceTable[tch->timHw->channelIndex]); | |
| return; | |
| } |
| for (int i = 0; i < getMotorCount(); i++) { | ||
| if (motors[i].pwmPort && motors[i].pwmPort->tch) { | ||
| impl_timerPWMSetDMACircular(motors[i].pwmPort->tch, circular); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Add a check for motors[i].pwmPort->configured before changing the DMA mode to ensure the motor port is configured. [general, importance: 6]
| for (int i = 0; i < getMotorCount(); i++) { | |
| if (motors[i].pwmPort && motors[i].pwmPort->tch) { | |
| impl_timerPWMSetDMACircular(motors[i].pwmPort->tch, circular); | |
| } | |
| } | |
| for (int i = 0; i < getMotorCount(); i++) { | |
| if (motors[i].pwmPort && motors[i].pwmPort->tch && motors[i].pwmPort->configured) { | |
| impl_timerPWMSetDMACircular(motors[i].pwmPort->tch, circular); | |
| } | |
| } |
| #if !defined(SITL_BUILD) && defined(USE_DSHOT) | ||
| // Prevent ESC spinup during settings save using circular DMA | ||
| pwmSetMotorDMACircular(true); | ||
|
|
||
| // Force motor updates to latch current (zero) throttle into circular DMA buffer | ||
| pwmCompleteMotorUpdate(); | ||
| delayMicroseconds(200); | ||
| pwmCompleteMotorUpdate(); | ||
| delayMicroseconds(200); | ||
| pwmCompleteMotorUpdate(); | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Gate the DMA mode switch on the runtime motor protocol (e.g., isMotorProtocolDshot()), and skip the whole sequence if DShot isn’t active to avoid unintended DMA reconfiguration on other setups. [Learned best practice, importance: 6]
| #if !defined(SITL_BUILD) && defined(USE_DSHOT) | |
| // Prevent ESC spinup during settings save using circular DMA | |
| pwmSetMotorDMACircular(true); | |
| // Force motor updates to latch current (zero) throttle into circular DMA buffer | |
| pwmCompleteMotorUpdate(); | |
| delayMicroseconds(200); | |
| pwmCompleteMotorUpdate(); | |
| delayMicroseconds(200); | |
| pwmCompleteMotorUpdate(); | |
| #endif | |
| #if !defined(SITL_BUILD) && defined(USE_DSHOT) | |
| if (isMotorProtocolDshot()) { | |
| // Prevent ESC spinup during settings save using circular DMA | |
| pwmSetMotorDMACircular(true); | |
| // Force motor updates to latch current (zero) throttle into circular DMA buffer | |
| pwmCompleteMotorUpdate(); | |
| delayMicroseconds(200); | |
| pwmCompleteMotorUpdate(); | |
| delayMicroseconds(200); | |
| pwmCompleteMotorUpdate(); | |
| } | |
| #endif |
| ATOMIC_BLOCK(NVIC_PRIO_MAX) { | ||
| // Temporarily disable DMA while modifying configuration | ||
| DMA_Cmd(tch->dma->ref, DISABLE); | ||
|
|
||
| // Wait for DMA stream to actually be disabled | ||
| // The EN bit doesn't clear immediately, especially if transfer is in progress | ||
| uint32_t timeout = 10000; | ||
| while ((tch->dma->ref->CR & DMA_SxCR_EN) && timeout--) { | ||
| __NOP(); | ||
| } | ||
|
|
||
| // If timeout occurred, DMA stream is still enabled - abort reconfiguration | ||
| if (timeout == 0 && (tch->dma->ref->CR & DMA_SxCR_EN)) { | ||
| DMA_Cmd(tch->dma->ref, ENABLE); // Re-enable and return | ||
| return; | ||
| } | ||
|
|
||
| // Modify the DMA mode | ||
| if (circular) { | ||
| tch->dma->ref->CR |= DMA_SxCR_CIRC; // Set circular bit | ||
| } else { | ||
| tch->dma->ref->CR &= ~DMA_SxCR_CIRC; // Clear circular bit | ||
| } | ||
|
|
||
| // Re-enable DMA | ||
| DMA_Cmd(tch->dma->ref, ENABLE); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Match the HAL implementation by disabling the timer’s DMA request source before disabling/reconfiguring the DMA stream, then re-enable it afterward to prevent mid-transition DMA triggers. [Learned best practice, importance: 5]
| ATOMIC_BLOCK(NVIC_PRIO_MAX) { | |
| // Temporarily disable DMA while modifying configuration | |
| DMA_Cmd(tch->dma->ref, DISABLE); | |
| // Wait for DMA stream to actually be disabled | |
| // The EN bit doesn't clear immediately, especially if transfer is in progress | |
| uint32_t timeout = 10000; | |
| while ((tch->dma->ref->CR & DMA_SxCR_EN) && timeout--) { | |
| __NOP(); | |
| } | |
| // If timeout occurred, DMA stream is still enabled - abort reconfiguration | |
| if (timeout == 0 && (tch->dma->ref->CR & DMA_SxCR_EN)) { | |
| DMA_Cmd(tch->dma->ref, ENABLE); // Re-enable and return | |
| return; | |
| } | |
| // Modify the DMA mode | |
| if (circular) { | |
| tch->dma->ref->CR |= DMA_SxCR_CIRC; // Set circular bit | |
| } else { | |
| tch->dma->ref->CR &= ~DMA_SxCR_CIRC; // Clear circular bit | |
| } | |
| // Re-enable DMA | |
| DMA_Cmd(tch->dma->ref, ENABLE); | |
| } | |
| ATOMIC_BLOCK(NVIC_PRIO_MAX) { | |
| // Disable timer DMA request first to stop new transfer triggers | |
| TIM_DMACmd(tch->timHw->tim, lookupDMASourceTable[tch->timHw->channelIndex], DISABLE); | |
| // Temporarily disable DMA while modifying configuration | |
| DMA_Cmd(tch->dma->ref, DISABLE); | |
| uint32_t timeout = 10000; | |
| while ((tch->dma->ref->CR & DMA_SxCR_EN) && timeout--) { | |
| __NOP(); | |
| } | |
| if (timeout == 0 && (tch->dma->ref->CR & DMA_SxCR_EN)) { | |
| // Restore timer DMA requests and return to avoid unstable state | |
| TIM_DMACmd(tch->timHw->tim, lookupDMASourceTable[tch->timHw->channelIndex], ENABLE); | |
| DMA_Cmd(tch->dma->ref, ENABLE); | |
| return; | |
| } | |
| if (circular) { | |
| tch->dma->ref->CR |= DMA_SxCR_CIRC; | |
| } else { | |
| tch->dma->ref->CR &= ~DMA_SxCR_CIRC; | |
| } | |
| DMA_Cmd(tch->dma->ref, ENABLE); | |
| TIM_DMACmd(tch->timHw->tim, lookupDMASourceTable[tch->timHw->channelIndex], ENABLE); | |
| } |
User description
Summary
Fixes ESC spinup/reboot when saving settings via configurator by enabling DMA circular mode during flash write operations.
Problem
Internal flash writes block CPU core execution for 20-200ms on STM32F4/F7/AT32F43x and STM32H7. This interrupts DShot signal transmission, causing ESCs to timeout and spin up motors.
Solution
Enable DMA circular mode before flash writes to allow DMA hardware to automatically repeat the last DShot packet (zero throttle) without CPU intervention.
Implementation:
DMA_SxCR_CIRCbitLL_DMA_MODE_CIRCULARwith proper synchronization (wait for EN bit clear)ctrl_bit.lm(loop mode)Testing
Tested with oscilloscope monitoring DShot signal during settings save:
Related Issues
Fixes #10913
Note: While related to Betaflight PR #12544, this addresses a different scenario:
INAV does not have the DShot beacon issue - motor values persist during beacon gaps.
PR Type
Bug fix
Description
Prevents ESC spinup during settings save by enabling DMA circular mode
Allows DMA to automatically repeat zero-throttle DShot packets during flash writes
Implements circular DMA support for STM32F4/F7, STM32H7, and AT32F43x platforms
Protects DMA reconfiguration with atomic blocks and timeout handling
Diagram Walkthrough
File Walkthrough
config_eeprom.c
Enable circular DMA during EEPROM writessrc/main/config/config_eeprom.c
pwm_output.hinclude for DMA circular mode controlpwmSetMotorDMACircular(true)before flash writes to enablecircular DMA
pwmCompleteMotorUpdate()to latch zerothrottle
pwmSetMotorDMACircular(false)afterwrites
timer_impl_hal.c
Implement circular DMA for STM32H7 platformssrc/main/drivers/timer_impl_hal.c
impl_timerPWMSetDMACircular()for STM32H7 using LL driversLL_DMA_MODE_CIRCULARandLL_DMA_MODE_NORMALtimer_impl_stdperiph.c
Implement circular DMA for STM32F4/F7 platformssrc/main/drivers/timer_impl_stdperiph.c
impl_timerPWMSetDMACircular()for STM32F4/F7 usingStdPeriph
DMA_SxCR_CIRCbit based on circular parametertimer_impl_stdperiph_at32.c
Implement circular DMA for AT32F43x platformssrc/main/drivers/timer_impl_stdperiph_at32.c
impl_timerPWMSetDMACircular()for AT32F43x platformctrl_bit.lm(loop mode) based on circular parameterpwm_output.h
Add DMA circular mode control functionsrc/main/drivers/pwm_output.h
pwmSetMotorDMACircular()function declarationpwm_output.c
Implement motor DMA circular mode wrappersrc/main/drivers/pwm_output.c
pwmSetMotorDMACircular()functionimplementation
timer_impl.h
Add platform DMA circular mode interfacesrc/main/drivers/timer_impl.h
impl_timerPWMSetDMACircular()function declaration