Peer review feedback

Author: dgarske

arch.mk

@@ -563,7 +563,10 @@ endif
 ## RISCV (32-bit)
 ifeq ($(ARCH),RISCV)
   CROSS_COMPILE?=riscv32-unknown-elf-
-  ARCH_FLAGS=-march=rv32imac -mabi=ilp32 -mcmodel=medany
+  # GCC 12+ requires explicit zicsr/zifencei extensions
+  RISCV32_ZICSR := $(shell echo "" | $(CC) -march=rv32imac_zicsr -x c -c - -o /dev/null 2>/dev/null && echo _zicsr)
+  RISCV32_ZIFENCEI := $(shell echo "" | $(CC) -march=rv32imac_zifencei -x c -c - -o /dev/null 2>/dev/null && echo _zifencei)
+  ARCH_FLAGS=-march=rv32imac$(RISCV32_ZICSR)$(RISCV32_ZIFENCEI) -mabi=ilp32 -mcmodel=medany
   CFLAGS+=-fno-builtin-printf -DUSE_M_TIME -g -nostartfiles -DARCH_RISCV
   CFLAGS+=$(ARCH_FLAGS)
   LDFLAGS+=$(ARCH_FLAGS)
@@ -605,12 +608,17 @@ ifeq ($(ARCH),RISCV64)
     UPDATE_OBJS?=src/update_ram.o
   endif
 
+  # GCC 12+ / binutils 2.38+ split CSR and fence.i instructions into
+  # separate extensions (zicsr, zifencei). Detect and add if supported.
+  RISCV64_ZICSR := $(shell echo "" | $(CC) -march=rv64imac_zicsr -x c -c - -o /dev/null 2>/dev/null && echo _zicsr)
+  RISCV64_ZIFENCEI := $(shell echo "" | $(CC) -march=rv64imac_zifencei -x c -c - -o /dev/null 2>/dev/null && echo _zifencei)
+
   ifeq ($(RISCV_MMODE),1)
     # E51 core: rv64imac (no FPU, no crypto extensions)
-    ARCH_FLAGS=-march=rv64imac -mabi=lp64 -mcmodel=medany
+    ARCH_FLAGS=-march=rv64imac$(RISCV64_ZICSR)$(RISCV64_ZIFENCEI) -mabi=lp64 -mcmodel=medany
   else
     # U54 cores: rv64gc (with FPU)
-    ARCH_FLAGS=-march=rv64imafd -mabi=lp64d -mcmodel=medany
+    ARCH_FLAGS=-march=rv64imafd$(RISCV64_ZICSR)$(RISCV64_ZIFENCEI) -mabi=lp64d -mcmodel=medany
 
     # FDT support required
     CFLAGS+=-DWOLFBOOT_FDT

hal/mpfs250.c

@@ -1079,12 +1079,15 @@ static void qspi_uart_program(void)
     uart_qspi_puts("READY\r\n");
 
     /* Receive destination address then data length (4 bytes LE each) */
-    addr = 0;
-    for (int i = 0; i < 4; i++)
-        addr |= ((uint32_t)uart_qspi_rx() << (i * 8));
-    size = 0;
-    for (int i = 0; i < 4; i++)
-        size |= ((uint32_t)uart_qspi_rx() << (i * 8));
+    {
+        int i;
+        addr = 0;
+        for (i = 0; i < 4; i++)
+            addr |= ((uint32_t)uart_qspi_rx() << (i * 8));
+        size = 0;
+        for (i = 0; i < 4; i++)
+            size |= ((uint32_t)uart_qspi_rx() << (i * 8));
+    }
 
     wolfBoot_printf("QSPI-PROG: addr=0x%x size=%u bytes\r\n", addr, size);
 

hal/riscv.h

@@ -83,8 +83,6 @@
 #define MODE_PREFIX(__suffix)    s##__suffix
 #endif
 
-
-
 /* ============================================================================
  * CSR Access Macros
  * ============================================================================ */

src/boot_riscv.c

@@ -419,7 +419,7 @@ void do_boot(const uint32_t *app_offset)
 #endif
 #ifdef MMU
     unsigned long dts_addr;
-#else
+#elif __riscv_xlen == 64
     unsigned long dts_addr = 0;
 #endif
 

src/boot_riscv_start.S

@@ -63,11 +63,12 @@ _reset:
      * 5. Handle multi-hart (E51 runs, U54s park in WFI)
      * ======================================================================== */
 
-#if 0 //def TARGET_mpfs250
+    /* Early UART initialization for M-mode boot on MPFS.
+     * Intentionally disabled: only needed for low-level bring-up debug
+     * to enable serial output before C runtime is available.
+     * To re-enable, change the guard to `#ifdef TARGET_mpfs250`. */
+#if 0
     /*
-     * Early UART initialization for M-mode boot on MPFS.
-     * This enables debug output during the boot sequence.
-     */
     li    t0, 0x20000000           /* UART0 base */
     li    t1, 0x20002000           /* SYSREG_BASE */
     /* Enable UART0 clock */

src/sdhci.c

@@ -167,7 +167,7 @@ void sdhci_irq_handler(void)
     /* Signal that interrupt was handled */
     g_mmc_irq_pending = 1;
 
-#ifdef DEBUG_SDHCI_IRQ
+#ifdef DEBUG_SDHCI_IRQ /* separate from DEBUG_SDHCI: IRQ handler is very noisy */
     wolfBoot_printf("sdhci_irq_handler: status=0x%08X, flags=0x%02X\n",
         status, g_mmc_irq_status);
 #endif

src/update_disk.c

@@ -242,9 +242,9 @@ void RAMFUNCTION wolfBoot_start(void)
     uint32_t load_off;
     const uint8_t *hdr_ptr = NULL;
 #ifdef MMU
-    uint8_t *dts_addr = NULL;
+    uint8_t *dts_addr = NULL; /* used by do_boot() under MMU */
     #ifdef WOLFBOOT_FDT
-    uint32_t dts_size = 0;
+    uint32_t dts_size = 0;    /* only used for FIT image parsing */
     #endif
 #endif
     char part_name[4] = {'P', ':', 'X', '\0'};

tools/scripts/mpfs_qspi_prog.py

@@ -43,12 +43,12 @@
     print("Error: 'pyserial' not installed. Run: pip install pyserial")
     sys.exit(1)
 
-BAUD_RATE  = 115200
-CHUNK_SIZE = 256
-ACK_BYTE   = 0x06
-PROMPT_TIMEOUT  = 60   # seconds to wait for wolfBoot prompt after reset
-ERASE_TIMEOUT   = 120  # seconds for sector erase (64KB sectors, ~800ms each)
-DONE_TIMEOUT    = 30   # seconds to wait for DONE after last chunk
+BAUD_RATE       = 115200
+CHUNK_SIZE      = 256
+ACK_BYTE        = 0x06
+PROMPT_TIMEOUT  = 60    # seconds to wait for wolfBoot prompt after reset
+ERASE_TIMEOUT   = 120   # seconds for sector erase (64KB sectors, ~800ms each)
+DONE_TIMEOUT    = 30    # seconds to wait for DONE after last chunk
 
 
 def wait_for(port, keyword, timeout_sec, label=""):