From 284a1d5eb9e6009bb26528fa494195ae04d4a263 Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 23 Apr 2026 11:35:12 -0700 Subject: [PATCH] Peer review fixes --- config/examples/stm32u3.config | 4 ++-- docs/Targets.md | 16 +++++---------- hal/stm32u3.c | 36 ++++++++++++---------------------- hal/stm32u3.h | 6 +++--- test-app/app_stm32u3.c | 17 ++++++++++------ 5 files changed, 34 insertions(+), 45 deletions(-) diff --git a/config/examples/stm32u3.config b/config/examples/stm32u3.config index 51d08aa2..f35e84a9 100644 --- a/config/examples/stm32u3.config +++ b/config/examples/stm32u3.config @@ -18,9 +18,9 @@ V?=0 SPMATH?=1 RAM_CODE?=1 DUALBANK_SWAP?=0 -# Flash layout for dual-bank (2x512KB, 8KB pages): +# Flash layout for dual-bank (2x512KB, 4KB pages): # Bank 1 (0x08000000): wolfBoot (64KB) + BOOT partition (448KB) -# Bank 2 (0x08080000): UPDATE partition (448KB) + SWAP (8KB) +# Bank 2 (0x08080000): UPDATE partition (448KB) + SWAP (4KB) # Future: DUALBANK_SWAP=1 eliminates swap and uses HW bank-swap WOLFBOOT_SECTOR_SIZE?=0x1000 WOLFBOOT_PARTITION_SIZE?=0x70000 diff --git a/docs/Targets.md b/docs/Targets.md index ed987d3d..c3b32b3d 100644 --- a/docs/Targets.md +++ b/docs/Targets.md @@ -1713,8 +1713,8 @@ arm-none-eabi-gdb The STM32U3 family (for example the STM32U385RG on NUCLEO-U385RG-Q) is a Cortex-M33 part **without TrustZone**, so the port is single-image only -(no `-tz` or `-ns` variants). 1 MB internal flash, 256 KB SRAM, 8 KB -pages, 128-bit (quad-word) flash write quantum. +(no `-tz` or `-ns` variants). 1 MB internal flash, 256 KB SRAM, 4 KB +pages, 64-bit (double-word) flash write quantum. ### Flash layout (stm32u3.config) @@ -1764,17 +1764,11 @@ The test app blinks LD2 (PA5): slow on v1, fast on v2 (post-update). ### Testing an Update -Sign the test application as version 2 and write the update trigger -magic (`pBOOT`) at the tail of the partition: +Sign the test application as version 2, build the update image with the +`pBOOT` trigger magic, and flash it: ```sh -tools/keytools/sign --ecc384 --sha384 test-app/image.bin \ - wolfboot_signing_private_key.der 2 -echo -n "pBOOT" > trigger_magic.bin -./tools/bin-assemble/bin-assemble \ - update.bin \ - 0x0 test-app/image_v2_signed.bin \ - 0x6FFFB trigger_magic.bin +./tools/scripts/prepare_update_u3.sh 2 STM32_Programmer_CLI -c port=SWD reset=HWrst \ -d update.bin 0x08080000 -v -rst ``` diff --git a/hal/stm32u3.c b/hal/stm32u3.c index 44bbd630..b79c3061 100644 --- a/hal/stm32u3.c +++ b/hal/stm32u3.c @@ -20,7 +20,7 @@ */ /* STM32U3 family (e.g. NUCLEO-U385RG-Q). Cortex-M33 without TrustZone. - * Always dual-bank 1 MB flash (2 x 512 KB), 8 KB pages, 64-bit + * Always dual-bank 1 MB flash (2 x 512 KB), 4 KB pages, 64-bit * (double-word) write quantum. * No traditional PLL -- MSIS switches directly between MSIRC1 (24 MHz) * and MSIRC0 (96 MHz). @@ -37,8 +37,12 @@ static void RAMFUNCTION flash_set_waitstates(unsigned int waitstates) { uint32_t reg = FLASH_ACR; - if ((reg & FLASH_ACR_LATENCY_MASK) != waitstates) + if ((reg & FLASH_ACR_LATENCY_MASK) != waitstates) { FLASH_ACR = (reg & ~FLASH_ACR_LATENCY_MASK) | waitstates; + /* RM: read-back to confirm LATENCY accepted before clock switch */ + while ((FLASH_ACR & FLASH_ACR_LATENCY_MASK) != waitstates) + ; + } } static RAMFUNCTION void flash_wait_complete(void) @@ -83,7 +87,9 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) /* RM step 6: write first word, then second word */ dst[i >> 2] = dword[0]; + ISB(); dst[(i >> 2) + 1] = dword[1]; + ISB(); /* RM step 8: wait for BSY clear */ flash_wait_complete(); @@ -103,15 +109,7 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len) void RAMFUNCTION hal_flash_unlock(void) { flash_wait_complete(); - /* Unlock both NS and Secure flash controllers */ - if ((FLASH_NS_CR & FLASH_CR_LOCK) != 0) { - FLASH_NS_KEYR = FLASH_KEY1; - DMB(); - FLASH_NS_KEYR = FLASH_KEY2; - DMB(); - while ((FLASH_NS_CR & FLASH_CR_LOCK) != 0) - ; - } + /* Unlock NS flash controller (TZEN=0, secure unlock not needed) */ if ((FLASH_NS_CR & FLASH_CR_LOCK) != 0) { FLASH_NS_KEYR = FLASH_KEY1; DMB(); @@ -196,13 +194,6 @@ int RAMFUNCTION hal_flash_erase(uint32_t address, int len) return 0; } -static void RAMFUNCTION stm32u3_reboot(void) -{ - AIRCR = AIRCR_SYSRESETREQ | AIRCR_VKEY; - while (1) - ; -} - /* --- UART: USART1 on PA9 (TX) / PA10 (RX), AF7 --- */ #define USART1_BASE (0x40013800U) @@ -335,7 +326,7 @@ void hal_init(void) #if defined(DEBUG_UART) && defined(__WOLFBOOT) uart_init(); - uart_write("wolfBoot HAL Init\n", 18); + uart_write("wolfBoot HAL Init\n", sizeof("wolfBoot HAL Init\n") - 1); #endif } @@ -360,9 +351,8 @@ void RAMFUNCTION hal_cache_invalidate(void) return; if ((ICACHE_SR & ICACHE_SR_BUSYF) == 0) ICACHE_CR |= ICACHE_CR_CACHEINV; - if (ICACHE_SR & ICACHE_SR_BUSYF) { - while ((ICACHE_SR & ICACHE_SR_BSYENDF) == 0) - ; - } + /* Wait unconditionally for invalidation to complete */ + while ((ICACHE_SR & ICACHE_SR_BSYENDF) == 0) + ; ICACHE_SR |= ICACHE_SR_BSYENDF; } diff --git a/hal/stm32u3.h b/hal/stm32u3.h index 2595b3a1..8636c169 100644 --- a/hal/stm32u3.h +++ b/hal/stm32u3.h @@ -20,7 +20,7 @@ */ /* STM32U3 family (e.g. STM32U385RG on NUCLEO-U385RG-Q). - * Cortex-M33, no TrustZone. 1 MB dual-bank flash (2 × 512 KB, 8 KB pages). + * Cortex-M33, no TrustZone. 1 MB dual-bank flash (2 × 512 KB, 4 KB pages). * Reference: RM0487, stm32u385xx.h from STM32Cube_FW_U3_V1.3.0. * * Peripheral base addresses differ from STM32U5: @@ -66,7 +66,7 @@ #define RCC_ICSCR1_MSISDIV_2 (1) /* /2 */ #define RCC_ICSCR1_MSISDIV_4 (2) /* /4 */ #define RCC_ICSCR1_MSISDIV_8 (3) /* /8 */ -/* MSIS RC source: bit 31 MSISSEL — 0 = MSIRC1 (24 MHz), 1 = MSIRC0 (96 MHz) */ +/* MSIS RC source: bit 31 MSISSEL — 0 = MSIRC0 (96 MHz), 1 = MSIRC1 (24 MHz) */ #define RCC_ICSCR1_MSISSEL (1u << 31) #define RCC_CFGR1 (*(volatile uint32_t *)(RCC_BASE + 0x1C)) @@ -130,7 +130,7 @@ #define FLASH_NS_SR (*(volatile uint32_t *)(FLASH_BASE + 0x20)) #define FLASH_NS_CR (*(volatile uint32_t *)(FLASH_BASE + 0x28)) -/* Secure flash registers — required for erase/write even with TZEN=0 */ +/* Secure flash registers (unused when TZEN=0, kept for reference) */ #define FLASH_SKEYR (*(volatile uint32_t *)(FLASH_BASE + 0x0C)) #define FLASH_SSR (*(volatile uint32_t *)(FLASH_BASE + 0x24)) #define FLASH_SCR (*(volatile uint32_t *)(FLASH_BASE + 0x2C)) diff --git a/test-app/app_stm32u3.c b/test-app/app_stm32u3.c index 04b271bb..b8116c9b 100644 --- a/test-app/app_stm32u3.c +++ b/test-app/app_stm32u3.c @@ -83,7 +83,10 @@ static void busy_delay(uint32_t count) void main(void) { uint32_t version; + uint32_t v; uint32_t on_ticks, off_ticks; + char num[4]; + int idx = 0; hal_init(); led_init(); @@ -93,12 +96,14 @@ void main(void) version = wolfBoot_current_firmware_version(); - { - char v = '0' + (char)(version & 0x7F); - char msg[] = "App version: X\r\n"; - msg[13] = v; - uart_write(msg, sizeof(msg) - 1); - } + v = version; + if (v >= 100) { num[idx++] = '0' + (v / 100); v %= 100; } + if (v >= 10 || idx > 0) { num[idx++] = '0' + (v / 10); v %= 10; } + num[idx++] = '0' + v; + num[idx] = '\0'; + uart_write("App version: ", sizeof("App version: ") - 1); + uart_write(num, idx); + uart_write("\r\n", 2); /* v1: slow blink. v2+: fast blink. */ if (version >= 2) {