Peer review fixes

Author: dgarske

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

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
 ```

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;
 }

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))

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) {