Skip to content

Merge devel branch to main#13

Open
chwon64 wants to merge 21 commits intomainfrom
devel
Open

Merge devel branch to main#13
chwon64 wants to merge 21 commits intomainfrom
devel

Conversation

@chwon64
Copy link

@chwon64 chwon64 commented Feb 19, 2026

The devel branch contains stm32h7b3 bootloader.

Copy link
Member

@terrillmoore terrillmoore left a comment

Choose a reason for hiding this comment

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

This PR bundles two independent bodies of work: (1) mccibootloader_image tool improvements (fix #7 header probing, --debug flag, argv fix, Doxygen, comment fixes) and (2) full STM32H7B3 bootloader platform support. Reviewing both below.

We'll pull the tool changes out as a separate PR so they can be merged independently. Terry will guide that.


Build-breakers

Missing libmcci_bootloader_cm7.a in bootloader link list. LIBS_McciBootloader_stm32h7b3 lists only libmcci_bootloader_stm32h7.a and libmcci_bootloader_stm32h7b3.a. The MCCI_DO_BOOTLOADER macro appends libmcci_bootloader.a and libmcci_tweetnacl.a, but the cm7 architecture library is never linked. The symbols from platform/arch/cm7/src/ won't be found.

No CFLAGS_CPU override for Cortex-M7. setup.mk sets CFLAGS_CPU ?= -mcpu=cortex-m0plus -mthumb. The STM32H7B3 is a Cortex-M7, and there's no mechanism to override CPU flags per-bootloader target. Building the H7B3 bootloader with CM0+ flags will produce wrong code (or fail to compile if CM7-specific instructions are used).

Possible duplicate symbols. SOURCES_libmcci_bootloader_stm32h7 includes both SOC-level files and arch-level files (mccibootloaderplatform_stm32h7_checkimagevalid.c, _getappinfo.c, _getsignatureblock.c, _startapp.c). These appear to duplicate files that also exist in platform/arch/cm7/src/ and are compiled into libmcci_bootloader_cm7.a. If both libraries end up linked, there will be duplicate symbol errors. If only the stm32h7 library is linked (and cm7 is not), then the cm7 library definition is dead code in the Makefile. Either way, the intent needs clarification.

Flash driver concern

waitForDone() clears status via rSR + 4 in mccibootloader_stm32h7_systemflash.c. This writes to whatever register is 4 bytes after FLASH_SR. On STM32H7, FLASH_SR bits are w1c (write-1-to-clear) and should be cleared by writing back to SR itself. If SR+4 happens to be FLASH_CCR on this part variant, it might work by coincidence, but this needs verification against the STM32H7B3 reference manual.

Copy-paste artifacts

  • mccibootloaderboard_stm32h7b3_storage.c line 3: Module says mccibootloaderboard_catenaabz_storage.c. End-of-file comment (line 106) also says catenaabz_storage.c.
  • mccibootloaderboard_stm32h7b3_platforminterface.c line 6: Function description says "for MCCI Catena 4801 boards".
  • mccibootloaderboard_stm32h7b3_storage.c propagates the "constraings" typo (should be "constraints").

Minor issues

  • platform/board/st/stm32h7b3/mk/mccibootloader.ld is missing a final newline.
  • The Doxyfile is 2539 lines of mostly defaults. Consider a minimal Doxyfile that only sets non-default values, or add it in a separate PR.

Tool changes look good

The mccibootloader_image refactoring to support multiple AppInfo offsets via templates is clean. The probeHeader() approach with vAppInfoOffsets[] table is straightforward. The argv increment fix for --app-version is a real bug fix. The PENDSTCLR bit-position fix and comment corrections in mcci_arm_cm0plus.h are correct.

@terrillmoore
Copy link
Member

Update: The following changes from this PR have already been cherry-picked and merged to main:

  1. Tool improvements (PR Fix #7: mccibootloader_image multi-arch header support #18, merged): --debug flag, multi-offset AppInfo probing, --app-version argv fix, version bump to 0.4.9.1, templatized vectors/page-zero types.
  2. Tool documentation and naming cleanup (PR Update tool docs for multi-arch AppInfo probing #19, merged): Documented --debug and multi-architecture AppInfo offsets in the tool README and CLAUDE.md. Renamed McciBootloader_Stm32h7_PageZero_Wire_t to McciBootloader_CortexM7Compact_PageZero_Wire_t to use architecture-level naming consistently.
  3. Doxygen setup (cherry-picked directly): The two Fix Add doxygen setup #9 commits (Doxyfile, logo asset, .gitignore update).

Please rebase this branch onto current main before continuing. The remaining work is the STM32H7B3 platform support, which still has the build issues noted in the earlier review (missing cm7 lib in link list, no CFLAGS_CPU override for Cortex-M7, possible duplicate symbols from mccibootloaderplatform_entry.c).

@terrillmoore
Copy link
Member

Note: the Doxygen setup from this PR was cherry-picked to main and further refined in PR #20 (Make targets, warning fixes, Doxyfile upgrade). GitHub Pages auto-publishing was added in PR #22. The Doxygen-related changes in this PR are now superseded by those merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants