Conversation
… code, right now only metadata and Hardfault_log
ST-LIB Release Plan
Pending changes
|
There was a problem hiding this comment.
Pull request overview
Reserves dedicated flash sectors for non-code data (HardFault log + metadata) and switches HardFault/metadata addressing from hard-coded constants to linker-provided symbols.
Changes:
- Split internal flash into
FLASH(code) plusFLASH_ST/FLASH_BTreserved regions in both linker scripts. - Place
.hardfault_logand.metadata_poolintoFLASH_STand expose their addresses via linker symbols consumed byHardfaultTrace.h. - Invoke
Hard_fault_check()during board initialization and apply a small ADC constructor formatting change.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| STM32H723ZGTX_RAM.ld | Adjusts flash region layout; adds HardFault stack section (currently duplicated) |
| STM32H723ZGTX_FLASH.ld | Moves hardfault log + metadata pool into FLASH_ST and exports linker symbols |
| Src/HALAL/HardFault/HardfaultTrace.c | Adds linker symbol extern for HardFault log address |
| Inc/HALAL/HardFault/HardfaultTrace.h | Replaces fixed flash addresses with linker-symbol-based addresses |
| Inc/ST-LIB.hpp | Calls Hard_fault_check() during init when IWDG is enabled |
| Inc/HALAL/Services/ADC/ADC.hpp | Constructor formatting (single-line delegating ctor) |
| .changesets/flash-fix-minor.md | Adds patch changeset entry for the flash layout change |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .hardfault_stack (NOLOAD) : /*Stack memory to avoid memfault inside hardfault handler*/ | ||
| { | ||
| . = ALIGN(8); | ||
| _hf_stack_start = .; | ||
| . += 0x400; /* 1 KB */ | ||
| _hf_stack_end = .; | ||
| } >DTCMRAM | ||
|
|
There was a problem hiding this comment.
.hardfault_stack output section is defined twice in this linker script (and _hf_stack_start/_hf_stack_end are assigned twice). This will either reserve the stack region twice or overwrite the symbol values, making the memory layout ambiguous. Keep a single .hardfault_stack definition and define the symbols only once.
| .hardfault_stack (NOLOAD) : /*Stack memory to avoid memfault inside hardfault handler*/ | |
| { | |
| . = ALIGN(8); | |
| _hf_stack_start = .; | |
| . += 0x400; /* 1 KB */ | |
| _hf_stack_end = .; | |
| } >DTCMRAM |
| /* | ||
| .hard_fault_log has to be the first thing in the FLASH_ST | ||
| */ | ||
|
|
||
| .hardfault_stack (NOLOAD) : /*Stack memory to avoid memfault inside hardfault handler*/ | ||
| { | ||
| . = ALIGN(8); | ||
| _hf_stack_start = .; | ||
| . += 0x400; /* 1 KB */ | ||
| _hf_stack_end = .; | ||
| } >DTCMRAM |
There was a problem hiding this comment.
HardfaultTrace.h now depends on linker-provided symbols _hf_log and _metadata, but this RAM linker script does not define .hardfault_log / .metadata_pool sections nor PROVIDE(_hf_log=...) / PROVIDE(_metadata=...). Builds that use STM32H723ZGTX_RAM.ld and link HardfaultTrace.c will fail with undefined symbols (or point at the wrong address). Add equivalent FLASH_ST sections + PROVIDE symbols here, or conditionally compile out HardFault/metadata flash usage for the RAM build.
| .hardfault_stack (NOLOAD) : /*Stack memory to avoid memfault inside hardfault handler*/ | ||
| { | ||
| . = ALIGN(8); | ||
| _hf_stack_start = .; | ||
| . += 0x400; /* 1 KB */ | ||
| _hf_stack_end = .; | ||
| } >DTCMRAM | ||
|
|
There was a problem hiding this comment.
The new .hardfault_stack reserved region and symbols (_hf_stack_start/_hf_stack_end) are not referenced anywhere in the repo (no code/asm uses them), so this change currently just consumes 1KB of DTCMRAM without affecting HardFault behavior. Either wire these symbols into the HardFault handler to actually switch to the dedicated stack, or remove the reserved section to avoid wasting memory.
| .hardfault_stack (NOLOAD) : /*Stack memory to avoid memfault inside hardfault handler*/ | |
| { | |
| . = ALIGN(8); | |
| _hf_stack_start = .; | |
| . += 0x400; /* 1 KB */ | |
| _hf_stack_end = .; | |
| } >DTCMRAM |
| .hardfault_log : | ||
| { | ||
| KEEP(*(.hardfault_log)) | ||
| . = . + 0x200; | ||
| } >FLASH | ||
| . = ALIGN(4); | ||
| hf_log = .; | ||
| KEEP(*(.hardfault_log)); | ||
| . += 0x200; | ||
| } >FLASH_ST | ||
|
|
||
| . = ALIGN(4); | ||
|
|
||
| .metadata_pool : | ||
| { | ||
| . = ABSOLUTE(0x080DFD00); | ||
| . = ALIGN(4); | ||
| metadata = .; | ||
| KEEP(*(.metadata_pool)) | ||
| . += 0x100; | ||
| } >FLASH | ||
|
|
||
| } >FLASH_ST |
There was a problem hiding this comment.
.hardfault_log and .metadata_pool reserve flash space by advancing . (0x200 / 0x100), which means the built image will contain data for those addresses. If these regions are intended to persist runtime-written logs/metadata across firmware reflashes, consider marking the sections NOLOAD and/or ensuring the flashing process does not program/erase FLASH_ST, otherwise flashing the firmware will overwrite any stored log/metadata.
Created a flash size in sector 6 to keep other things differents than code, right now only metadata and Hardfault_log