Skip to content

fix(stm32f4): Hangs during flashing firmware #7415

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.

Sign up for GitHub

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

Open
richardclli wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(stm32f4): Hangs during flashing firmware #7415

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b14c674
fix(stm32): disable interrupts during flash erase/program to prevent …
richardclli Jun 1, 2026
182eddc
fix(stm32): replace broken HAL timeouts with DWT cycle counter during…
richardclli Jun 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 128 additions & 6 deletions radio/src/targets/common/arm/stm32/flash_driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,55 @@ static uint32_t stm32_flash_get_sector_size(uint32_t sector)
return 128 * 1024;
}

#define FLASH_TIMEOUT_MS 15000

static bool flash_drv_wait_last_op()
{
CoreDebug->DEMCR |= CoreDebug_DEMCR_TRCENA_Msk;
DWT->CTRL |= DWT_CTRL_CYCCNTENA_Msk;

uint32_t start = DWT->CYCCNT;
uint32_t timeout_cycles =
(uint32_t)(FLASH_TIMEOUT_MS * (SystemCoreClock / 1000UL));

while (__HAL_FLASH_GET_FLAG(FLASH_FLAG_BSY)) {
if ((DWT->CYCCNT - start) > timeout_cycles) {
__HAL_FLASH_CLEAR_FLAG(FLASH_FLAG_EOP | FLASH_FLAG_WRPERR |
FLASH_FLAG_PGAERR | FLASH_FLAG_PGPERR |
FLASH_FLAG_PGSERR);
return false;
}
}

__HAL_FLASH_CLEAR_FLAG(FLASH_FLAG_EOP);

if (__HAL_FLASH_GET_FLAG(FLASH_FLAG_WRPERR | FLASH_FLAG_PGAERR |
FLASH_FLAG_PGPERR | FLASH_FLAG_PGSERR)) {
__HAL_FLASH_CLEAR_FLAG(FLASH_FLAG_WRPERR | FLASH_FLAG_PGAERR |
FLASH_FLAG_PGPERR | FLASH_FLAG_PGSERR |
FLASH_FLAG_EOP);
return false;
}

return true;
}

static void flash_drv_flush_caches()
{
if (FLASH->ACR & FLASH_ACR_ICEN) {
FLASH->ACR &= ~FLASH_ACR_ICEN;
FLASH->ACR |= FLASH_ACR_ICRST;
FLASH->ACR &= ~FLASH_ACR_ICRST;
FLASH->ACR |= FLASH_ACR_ICEN;
}
if (FLASH->ACR & FLASH_ACR_DCEN) {
FLASH->ACR &= ~FLASH_ACR_DCEN;
FLASH->ACR |= FLASH_ACR_DCRST;
FLASH->ACR &= ~FLASH_ACR_DCRST;
FLASH->ACR |= FLASH_ACR_DCEN;
}
}

#elif defined(STM32H7) || defined(STM32H7RS)

static uint32_t stm32_flash_get_sector(uint32_t address)
Expand Down Expand Up @@ -129,6 +178,40 @@ static inline void stm32_flash_lock() { HAL_FLASH_Lock(); }

static int stm32_flash_erase_sector(uint32_t address)
{
int ret = 0;

#if defined(STM32F2) || defined(STM32F4)

uint32_t sector = stm32_flash_get_sector(address);

__disable_irq();
__DSB();

stm32_flash_unlock();

if (sector > 11) sector += 4;

CLEAR_BIT(FLASH->CR, FLASH_CR_PSIZE);
FLASH->CR |= FLASH_PSIZE_WORD;
CLEAR_BIT(FLASH->CR, FLASH_CR_SNB);
FLASH->CR |= FLASH_CR_SER | (sector << FLASH_CR_SNB_Pos);
FLASH->CR |= FLASH_CR_STRT;

if (!flash_drv_wait_last_op()) {
ret = -1;
}

CLEAR_BIT(FLASH->CR, FLASH_CR_SER | FLASH_CR_SNB);

__DSB();
__enable_irq();

flash_drv_flush_caches();

stm32_flash_lock();

#else

FLASH_EraseInitTypeDef eraseInit;
eraseInit.TypeErase = FLASH_TYPEERASE_SECTORS;
eraseInit.Sector = stm32_flash_get_sector(address);
Expand All @@ -142,15 +225,19 @@ static int stm32_flash_erase_sector(uint32_t address)
eraseInit.VoltageRange = FLASH_VOLTAGE_RANGE_3;
#endif

int ret = 0;
uint32_t sector_errors = 0;

__disable_irq();
stm32_flash_unlock();
if (HAL_FLASHEx_Erase(&eraseInit, &sector_errors) != HAL_OK) {
ret = -1;
}

stm32_flash_lock();
__enable_irq();
Comment on lines +230 to +237

@coderabbitai coderabbitai Bot Jun 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Unconditional __enable_irq() may unexpectedly alter caller's interrupt state.

If interrupts were already disabled before entering this function, __enable_irq() will enable them unexpectedly. Use PRIMASK save/restore pattern to preserve the original interrupt state.

Proposed fix 
   int ret = 0;
   uint32_t sector_errors = 0;

+  uint32_t primask = __get_PRIMASK();
   __disable_irq();
   stm32_flash_unlock();
   if (HAL_FLASHEx_Erase(&eraseInit, &sector_errors) != HAL_OK) {
     ret = -1;
   }

   stm32_flash_lock();
-  __enable_irq();
+  __set_PRIMASK(primask);
   return ret;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
__disable_irq();
stm32_flash_unlock();
if (HAL_FLASHEx_Erase(&eraseInit, &sector_errors) != HAL_OK) {
ret = -1;
}
stm32_flash_lock();
__enable_irq();
uint32_t primask = __get_PRIMASK();
__disable_irq();
stm32_flash_unlock();
if (HAL_FLASHEx_Erase(&eraseInit, &sector_errors) != HAL_OK) {
ret = -1;
}
stm32_flash_lock();
__set_PRIMASK(primask);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@radio/src/targets/common/arm/stm32/flash_driver.cpp` around lines 148 - 155,
The code unconditionally calls __enable_irq() after using __disable_irq(), which
can enable interrupts that were previously disabled by the caller; change the
pattern to save and restore the caller's PRIMASK instead: at function entry
(around where __disable_irq() is currently called) read the current PRIMASK via
__get_PRIMASK(), then call __disable_irq(), perform stm32_flash_unlock(),
HAL_FLASHEx_Erase(...), stm32_flash_lock(), and finally restore the original
interrupt state by calling __set_PRIMASK(saved_primask) instead of
unconditionally calling __enable_irq(); update usage around
stm32_flash_unlock()/stm32_flash_lock()/HAL_FLASHEx_Erase to use this
save/restore PRIMASK pattern.


#endif

return ret;
}

Expand All @@ -162,18 +249,49 @@ static int stm32_flash_erase_sector(uint32_t address)
#define FLASH_PROG_WORDS 4UL
#define _FLASH_PROGRAM(address, p_data) \
HAL_FLASH_Program(FLASH_TYPEPROGRAM_QUADWORD, address, (uintptr_t)p_data)
#else
#define FLASH_PROG_WORDS 1UL
#define _FLASH_PROGRAM(address, p_data) \
HAL_FLASH_Program(FLASH_TYPEPROGRAM_WORD, address, *p_data)
#endif

static int stm32_flash_program(uint32_t address, void* data, uint32_t len)
{
int ret = 0;

#if defined(STM32F2) || defined(STM32F4)

uint32_t* p_data = (uint32_t*)data;
uint32_t end_addr = address + len;

int ret = 0;
__disable_irq();
__DSB();
stm32_flash_unlock();

while (address < end_addr) {
CLEAR_BIT(FLASH->CR, FLASH_CR_PSIZE);
FLASH->CR |= FLASH_PSIZE_WORD;
FLASH->CR |= FLASH_CR_PG;

*(__IO uint32_t*)address = *p_data;

if (!flash_drv_wait_last_op()) {
ret = -1;
break;
}

CLEAR_BIT(FLASH->CR, FLASH_CR_PG);

address += sizeof(uint32_t);
p_data++;
}

__DSB();
__enable_irq();
stm32_flash_lock();
Comment on lines +263 to +287

@coderabbitai coderabbitai Bot Jun 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Always clear FLASH_CR_PG in the failure path.

Line 274 breaks out before FLASH_CR_PG is cleared. If flash_drv_wait_last_op() times out or reports an error, the controller stays in program mode through the rest of the epilogue, which is unsafe for the next flash access.

Suggested fix 
   while (address < end_addr) {
     CLEAR_BIT(FLASH->CR, FLASH_CR_PSIZE);
     FLASH->CR |= FLASH_PSIZE_WORD;
     FLASH->CR |= FLASH_CR_PG;
@@
     if (!flash_drv_wait_last_op()) {
       ret = -1;
       break;
     }
 
     CLEAR_BIT(FLASH->CR, FLASH_CR_PG);
@@
   }
 
+  CLEAR_BIT(FLASH->CR, FLASH_CR_PG);
   __DSB();
   __enable_irq();
   stm32_flash_lock();
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@radio/src/targets/common/arm/stm32/flash_driver.cpp` around lines 263 - 287,
The loop can exit on failure while FLASH_CR_PG remains set; ensure FLASH_CR_PG
is cleared before breaking: in the loop around flash_drv_wait_last_op() (and
immediately before any early return/break), clear FLASH_CR_PG (use
CLEAR_BIT(FLASH->CR, FLASH_CR_PG)) so the controller is not left in program
mode; keep the existing __DSB()/__enable_irq()/stm32_flash_lock() epilogue but
perform the CLEAR_BIT(FLASH->CR, FLASH_CR_PG) in the failure branch right after
detecting !flash_drv_wait_last_op() and before ret = -1; this change affects the
block that sets FLASH_CR_PG and calls flash_drv_wait_last_op() in
flash_driver.cpp.


#else

uint32_t* p_data = (uint32_t*)data;
uint32_t end_addr = address + len;

__disable_irq();
stm32_flash_unlock();
while (address < end_addr) {
if (_FLASH_PROGRAM(address, p_data) != HAL_OK) {
Expand All @@ -186,6 +304,10 @@ static int stm32_flash_program(uint32_t address, void* data, uint32_t len)
}

stm32_flash_lock();
__enable_irq();

#endif

return ret;
}

Expand Down