Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
74 changes: 74 additions & 0 deletions src/main/drivers/system_at32f43x.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,79 @@
#include "target/system.h"
#include "at32f435_437_clock.h"

// See RM_AT32F435_437_EN_V2.05.pdf reference manual table 5-6 for more info.
#if 256 < TARGET_FLASH_SIZE
#define USD_EOPB0_SRAM_CONFIG_MASK 0x7
#else
#define USD_EOPB0_SRAM_CONFIG_MASK 0x3
#endif
Comment on lines +32 to +36

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Flash size macro missing 🐞 Bug ≡ Correctness

system_at32f43x.c now branches on TARGET_FLASH_SIZE, but AT32 builds define MCU_FLASH_SIZE in CMake
and other AT32 targets do not define TARGET_FLASH_SIZE; when undefined it evaluates as 0 in #if
expressions, leaving get_sram_config() with no return path and causing undefined behavior (or
-Werror build failure).
Agent Prompt
### Issue description
`system_at32f43x.c` uses `TARGET_FLASH_SIZE` in preprocessor conditionals, but the AT32 build system provides `MCU_FLASH_SIZE` (CMake) and most AT32 targets do not define `TARGET_FLASH_SIZE`. When undefined, `TARGET_FLASH_SIZE` is treated as `0` in `#if`, so the `get_sram_config()` switch compiles with no cases and no return, leading to undefined behavior (and often failing the build under `-Werror=return-type`).

### Issue Context
- AT32 CMake toolchain defines `MCU_FLASH_SIZE`, not `TARGET_FLASH_SIZE`.
- Only one AT32 target header was updated to define `TARGET_FLASH_SIZE`, leaving other AT32 targets inconsistent.

### Fix Focus Areas
- src/main/drivers/system_at32f43x.c[31-80]
- cmake/at32f4.cmake[80-101]
- src/main/target/BLUEBERRYF435WING/target.h[18-46]

### Suggested fix
- Replace `TARGET_FLASH_SIZE` usage with `MCU_FLASH_SIZE` (preferred), or add a single global fallback in a common header:
  - `#ifndef TARGET_FLASH_SIZE` / `#define TARGET_FLASH_SIZE MCU_FLASH_SIZE`.
- Remove the per-target `#define TARGET_FLASH_SIZE ...` if it becomes redundant, to avoid divergence.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


static flash_usd_eopb0_type get_sram_config(void)
{
extern uint32_t _SRAM_SIZE; // Defined in linker file
switch ((uint32_t)&_SRAM_SIZE) {
#if 256 == TARGET_FLASH_SIZE
case 448:
return FLASH_EOPB0_SRAM_448K;
case 512:
return FLASH_EOPB0_SRAM_512K;
case 384:
default:
return FLASH_EOPB0_SRAM_384K;
#elif 448 == TARGET_FLASH_SIZE
case 256:
return FLASH_EOPB0_SRAM_256K;
case 320:
return FLASH_EOPB0_SRAM_320K;
case 384:
return FLASH_EOPB0_SRAM_384K;
case 448:
return FLASH_EOPB0_SRAM_448K;
case 512:
return FLASH_EOPB0_SRAM_512K;
case 192:
default:
return FLASH_EOPB0_SRAM_192K;
#elif 1024 <= TARGET_FLASH_SIZE
case 128:
return FLASH_EOPB0_SRAM_128K;
case 256:
return FLASH_EOPB0_SRAM_256K;
case 320:
return FLASH_EOPB0_SRAM_320K;
case 384:
return FLASH_EOPB0_SRAM_384K;
case 448:
return FLASH_EOPB0_SRAM_448K;
case 512:
return FLASH_EOPB0_SRAM_512K;
case 192:
default:
return FLASH_EOPB0_SRAM_192K;
#endif
}
}

static void init_sram_config(void)
{
// Make sure the SRAM config is correct
const flash_usd_eopb0_type sram_cfg = get_sram_config();
if (((USD->eopb0) & USD_EOPB0_SRAM_CONFIG_MASK) != sram_cfg) {
flash_unlock();
flash_user_system_data_erase();
flash_eopb0_config(sram_cfg);
systemReset();
Comment on lines +89 to +92

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

3. Usd erase wipes settings 🐞 Bug ≡ Correctness

init_sram_config() uses flash_user_system_data_erase() (documented to erase all USD except FAP) but
then only restores eopb0, so any existing USD fields (SSB/DATA/EPP/QSPIKEY/etc.) are lost whenever
SRAM config differs.
Agent Prompt
### Issue description
The new SRAM configuration path erases the entire User System Data (USD) area but reprograms only `eopb0`. Per the vendor driver, this erase clears all USD fields except the FAP byte, so board-specific/provisioned values in other USD fields can be destroyed.

### Issue Context
`usd_type` contains multiple fields beyond `eopb0` (`ssb`, `data0/1`, `epp*`, `qspikey[]`, etc.). If any of these are set (factory provisioning, security settings, boot behavior), erasing USD during firmware boot can cause permanent behavior changes.

### Fix Focus Areas
- src/main/drivers/system_at32f43x.c[84-94]
- lib/main/AT32F43x/Drivers/AT32F43x_StdPeriph_Driver/src/at32f435_437_flash.c[508-558]
- lib/main/AT32F43x/Drivers/AT32F43x_StdPeriph_Driver/inc/at32f435_437_flash.h[631-652]

### Suggested fix
- Avoid a full USD erase if the platform supports updating `eopb0` without erasing the whole USD block.
- If erase is required, first snapshot all USD fields that must be preserved (read `USD->...`), perform the erase, then reprogram **all** preserved fields plus the desired `eopb0` value.
- At minimum, explicitly set any required USD fields to known-safe defaults after erase, rather than leaving them erased.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}
Comment on lines +88 to +93

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Flash ops unchecked 🐞 Bug ☼ Reliability

init_sram_config() erases/programs USD and immediately resets without checking
flash_user_system_data_erase()/flash_eopb0_config() return values; if the operations fail, the board
can reboot into the same mismatch and get stuck in a persistent reset loop.
Agent Prompt
### Issue description
`init_sram_config()` ignores the return statuses of `flash_user_system_data_erase()` and `flash_eopb0_config()`, then calls `systemReset()` unconditionally. If erase/program fails, the SRAM-config mismatch will still be present on next boot and the device can repeatedly erase/reset.

### Issue Context
The vendor flash driver APIs return `flash_status_type` specifically to allow callers to detect failures/timeouts. Additionally, the vendor driver documents that USD erase changes `eopb0` to `0xFF` (which can affect SRAM sizing), so resetting without confirming successful re-programming is risky.

### Fix Focus Areas
- src/main/drivers/system_at32f43x.c[84-94]
- lib/main/AT32F43x/Drivers/AT32F43x_StdPeriph_Driver/inc/at32f435_437_flash.h[671-690]
- lib/main/AT32F43x/Drivers/AT32F43x_StdPeriph_Driver/src/at32f435_437_flash.c[508-599]

### Suggested fix
- Capture and validate return values:
  - If erase fails: abort the update and avoid reset (enter a safe failure mode, or keep running with current config).
  - If programming fails after a successful erase: avoid reset-loop; signal failure and stop.
- Only call `systemReset()` after confirming successful programming.
- Consider re-locking flash (and/or USD) if execution continues after failure.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}

// void systemReset(void)
// {
// __disable_irq();
// NVIC_SystemReset();
// }



void SetSysClock(void);

void enableGPIOPowerUsageAndNoiseReductions(void)
Expand Down Expand Up @@ -116,6 +189,7 @@ uint32_t systemBootloaderAddress(void)

void systemInit(void)
{
init_sram_config();
//config system clock to 288mhz usb 48mhz
system_clock_config();
// Configure NVIC preempt/priority groups
Expand Down
2 changes: 1 addition & 1 deletion src/main/target/BLUEBERRYF435WING/target.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
* PH0 HEXT IN
* PH1 HEXT OUT
*/
#define TARGET_FLASH_SIZE 1024
#define LED0 PC13
#define LED1 PC14
#define LED0_INVERTED
Expand Down
2 changes: 1 addition & 1 deletion src/main/target/link/at32_flash_f43xG.ld
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

*/


_SRAM_SIZE = 192;

MEMORY
{
Expand Down
2 changes: 1 addition & 1 deletion src/main/target/link/at32_flash_f43xM.ld
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

*/


_SRAM_SIZE = 192;

MEMORY
{
Expand Down
2 changes: 1 addition & 1 deletion src/main/target/link/at32_flash_f43xM_bl.ld
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

*/


_SRAM_SIZE = 192;

MEMORY
{
Expand Down
2 changes: 1 addition & 1 deletion src/main/target/link/at32_flash_f43xM_for_bl.ld
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

*/


_SRAM_SIZE = 192;

MEMORY
{
Expand Down
Loading