Progress with IFC Flash driver and mult-core support

Author: dgarske

arch.mk

@@ -1001,6 +1001,7 @@ ifeq ($(TARGET),nxp_t2080)
   LDFLAGS+=$(ARCH_FLAGS)
   LDFLAGS+=-Wl,--hash-style=both # generate both sysv and gnu symbol hash table
   LDFLAGS+=-Wl,--as-needed # remove weak functions not used
+  CFLAGS+=-DTEST_FLASH # enable flash erase/write/read test
   OBJS+=src/boot_ppc_mp.o # support for spin table
   UPDATE_OBJS:=src/update_ram.o
   OBJS+=src/fdt.o

hal/nxp_t2080.c

@@ -172,6 +172,13 @@ static void udelay(uint32_t delay_us)
     wait_ticks(delay_us * DELAY_US);
 }
 
+/* Pre-computed DELAY_US value for ram_udelay() (RAMFUNCTION).
+ * Must be initialized before any flash operations begin.
+ * ram_udelay() cannot call DELAY_US macro because it expands to
+ * hal_get_plat_clk() which lives in flash .text — during flash command
+ * mode, instruction fetch from flash returns status data, not code. */
+static uint32_t ram_delay_ticks;
+
 #if defined(ENABLE_IFC) && !defined(BUILD_LOADER_STAGE1)
 static int hal_flash_getid(void)
 {
@@ -228,6 +235,12 @@ static void hal_flash_init(void)
      * Flash write/erase use RAMFUNCTION to execute from DDR during
      * flash command mode (after .ramcode relocation in hal_init). */
 #endif /* ENABLE_IFC */
+
+    /* Pre-compute timebase ticks per microsecond for ram_udelay().
+     * Must be done while flash is in read mode — DELAY_US macro expands to
+     * hal_get_plat_clk() which is in flash .text and cannot be called from
+     * RAMFUNCTION code during flash command mode. */
+    ram_delay_ticks = DELAY_US;
 }
 
 void hal_ddr_init(void)
@@ -688,46 +701,103 @@ void hal_init(void)
 #endif
 }
 
+/* Direct UART character output from RAMFUNCTION code.
+ * Uses inline get8/set8 to access UART registers in CCSR space (own TLB).
+ * Safe to call during flash command mode — no flash code involved. */
+static void RAMFUNCTION ram_putchar(char c)
+{
+    while (!(get8(UART_LSR(UART_SEL)) & UART_LSR_THRE))
+        ;
+    set8(UART_THR(UART_SEL), c);
+}
+static void RAMFUNCTION ram_puthex4(uint8_t val)
+{
+    val &= 0xF;
+    ram_putchar(val < 10 ? '0' + val : 'A' + val - 10);
+}
+static void RAMFUNCTION ram_puthex16(uint16_t val)
+{
+    ram_puthex4(val >> 12);
+    ram_puthex4(val >> 8);
+    ram_puthex4(val >> 4);
+    ram_puthex4(val);
+}
+
 /* RAM-resident microsecond delay using inline timebase reads.
- * Cannot call wait_ticks() (in flash .text) from RAMFUNCTION code
- * while flash is in command mode — instruction fetch would return garbage. */
+ * Uses pre-computed ram_delay_ticks (initialized before flash ops).
+ * Cannot call DELAY_US macro — it expands to hal_get_plat_clk() in flash
+ * .text. The linker generates a trampoline that jumps to flash; during
+ * flash command mode, instruction fetch returns status data → crash. */
 static void RAMFUNCTION ram_udelay(uint32_t delay_us)
 {
     uint32_t tbl_start, tbl_now;
-    uint32_t ticks = delay_us * DELAY_US;
+    uint32_t ticks = delay_us * ram_delay_ticks;
     __asm__ __volatile__("mfspr %0,268" : "=r"(tbl_start));
     do {
         __asm__ __volatile__("mfspr %0,268" : "=r"(tbl_now));
     } while ((tbl_now - tbl_start) < ticks);
 }
 
+/* Inline TLB write — all RAMFUNCTION flash code must avoid calling set_tlb(),
+ * invalidate_dcache(), invalidate_icache() because those live in flash .text.
+ * During flash command mode, I-cache misses to flash return status data instead
+ * of instructions → PIL exception. Using mtspr/mfspr macros (inline asm) keeps
+ * everything in DDR .ramcode. */
+static void RAMFUNCTION ram_write_tlb(uint32_t mas0, uint32_t mas1,
+    uint32_t mas2, uint32_t mas3, uint32_t mas7)
+{
+    mtspr(MAS0, mas0);
+    mtspr(MAS1, mas1);
+    mtspr(MAS2, mas2);
+    mtspr(MAS3, mas3);
+    mtspr(MAS7, mas7);
+    __asm__ __volatile__("isync; msync; tlbwe; isync");
+}
+
 /* Switch flash TLB to cache-inhibited + guarded for direct flash chip access.
  * AMD flash commands require writes to reach the chip immediately and status
  * reads to come directly from the chip. With MAS2_M (cacheable), stores go
  * through the CPC coherency fabric; IFC does not support coherent writes and
- * returns a bus error (DSI). tlbre/tlbwe only modifies MAS2 and is unreliable
- * when the entry has IPROT=1; use set_tlb() to rewrite all MAS fields.
- * Must be called while flash is still in read-array mode (set_tlb lives in
- * flash .text, reachable via longcall while TLB is still M/cacheable). */
+ * returns a bus error (DSI).
+ * Fully inlined — no calls to flash-resident set_tlb(). */
 static void RAMFUNCTION hal_flash_cache_disable(void)
 {
-    set_tlb(1, 2,
-        FLASH_BASE_ADDR, FLASH_BASE_ADDR, FLASH_BASE_PHYS_HIGH,
-        MAS3_SX | MAS3_SW | MAS3_SR, MAS2_I | MAS2_G, 0,
-        FLASH_TLB_PAGESZ, 1);
+    ram_write_tlb(
+        BOOKE_MAS0(1, 2, 0),
+        BOOKE_MAS1(1, 1, 0, 0, FLASH_TLB_PAGESZ),
+        BOOKE_MAS2(FLASH_BASE_ADDR, MAS2_I | MAS2_G),
+        BOOKE_MAS3(FLASH_BASE_ADDR, 0, MAS3_SX | MAS3_SW | MAS3_SR),
+        BOOKE_MAS7(FLASH_BASE_PHYS_HIGH));
 }
 
 /* Restore flash TLB to cacheable mode after flash operation.
  * Flash must be back in read-array mode before calling (AMD_CMD_RESET sent).
- * Invalidate caches afterward so stale pre-erase data is not served. */
+ * Invalidate caches afterward so stale pre-erase data is not served.
+ * Fully inlined — no calls to flash-resident functions. */
 static void RAMFUNCTION hal_flash_cache_enable(void)
 {
-    set_tlb(1, 2,
-        FLASH_BASE_ADDR, FLASH_BASE_ADDR, FLASH_BASE_PHYS_HIGH,
-        MAS3_SX | MAS3_SW | MAS3_SR, MAS2_M, 0,
-        FLASH_TLB_PAGESZ, 1);
-    invalidate_dcache();
-    invalidate_icache();
+    uint32_t val;
+
+    ram_write_tlb(
+        BOOKE_MAS0(1, 2, 0),
+        BOOKE_MAS1(1, 1, 0, 0, FLASH_TLB_PAGESZ),
+        BOOKE_MAS2(FLASH_BASE_ADDR, MAS2_M),
+        BOOKE_MAS3(FLASH_BASE_ADDR, 0, MAS3_SX | MAS3_SW | MAS3_SR),
+        BOOKE_MAS7(FLASH_BASE_PHYS_HIGH));
+
+    /* Inline invalidate_dcache() — L1CSR0.CFI */
+    val = mfspr(L1CSR0);
+    val |= L1CSR_CFI;
+    __asm__ __volatile__("msync; isync");
+    mtspr(L1CSR0, val);
+    __asm__ __volatile__("isync");
+
+    /* Inline invalidate_icache() — L1CSR1.CFI */
+    val = mfspr(L1CSR1);
+    val |= L1CSR_CFI;
+    __asm__ __volatile__("msync; isync");
+    mtspr(L1CSR1, val);
+    __asm__ __volatile__("isync");
 }
 
 /* Clear IFC write-protect. T2080RM says IFC_CSPR should only be written
@@ -847,38 +917,52 @@ static int RAMFUNCTION hal_flash_status_wait(uint32_t sector, uint16_t mask,
     uint32_t timeout = 0;
     uint16_t read1, read2;
 
-    /* Replicate 8-bit AMD status mask to both bytes for parallel chips */
+    /* Replicate 8-bit AMD toggle bit to both bytes for parallel chips */
 #if FLASH_CFI_WIDTH == 16
-    uint16_t mask16 = (mask << 8) | mask;
     uint16_t toggle16 = (AMD_STATUS_TOGGLE << 8) | AMD_STATUS_TOGGLE;
 #else
-    uint16_t mask16 = mask;
     uint16_t toggle16 = AMD_STATUS_TOGGLE;
 #endif
+    (void)mask; /* mask parameter reserved for future DQ7 data polling */
 
     do {
-        /* detection of completion happens when reading status bits
-         * DQ6 and DQ2 stop toggling (0x44) */
+        /* AMD toggle detection: DQ6 toggles on consecutive reads during
+         * program/erase. When the operation completes, DQ6 reflects actual
+         * data and consecutive reads return the same value.
+         * NOTE: Do NOT check programmed data bits against a mask here —
+         * after write completes, the data depends on what was written, not
+         * on any fixed status bits. Only erase guarantees 0xFF data. */
 #if FLASH_CFI_WIDTH == 16
         read1 = FLASH_IO16_READ(sector, 0);
-        if ((read1 & toggle16) == 0)
-            read1 = FLASH_IO16_READ(sector, 0);
         read2 = FLASH_IO16_READ(sector, 0);
-        if ((read2 & toggle16) == 0)
-            read2 = FLASH_IO16_READ(sector, 0);
 #else
         read1 = FLASH_IO8_READ(sector, 0);
-        if ((read1 & toggle16) == 0)
-            read1 = FLASH_IO8_READ(sector, 0);
         read2 = FLASH_IO8_READ(sector, 0);
-        if ((read2 & toggle16) == 0)
-            read2 = FLASH_IO8_READ(sector, 0);
 #endif
+        /* Print first iteration reads for diagnostics */
+        if (timeout == 0) {
+            ram_putchar('[');
+            ram_puthex16(read1);
+            ram_putchar(':');
+            ram_puthex16(read2);
+            ram_putchar(']');
+        }
     #ifdef DEBUG_FLASH
         wolfBoot_printf("Wait toggle %x -> %x\n", read1, read2);
     #endif
-        if (read1 == read2 && ((read1 & mask16) == mask16))
+        /* DQ6 stopped toggling → operation complete */
+        if (((read1 ^ read2) & toggle16) == 0)
             break;
+        /* Check DQ5 (error) on both chips while still toggling */
+        if (read1 & ((AMD_STATUS_ERROR << 8) | AMD_STATUS_ERROR)) {
+            /* Read one more time to confirm it's not a false DQ5 */
+            read1 = FLASH_IO16_READ(sector, 0);
+            read2 = FLASH_IO16_READ(sector, 0);
+            if (((read1 ^ read2) & toggle16) == 0)
+                break; /* toggle stopped — was a race, not an error */
+            ret = -2; /* DQ5 error — program/erase failed */
+            break;
+        }
         ram_udelay(1);
     } while (timeout++ < timeout_us);
     if (timeout >= timeout_us) {
@@ -914,13 +998,15 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len)
 #endif
 
     /* Disable flash caching — AMD commands must reach the chip directly */
+    ram_putchar('w'); /* checkpoint: entering write */
     hal_flash_cache_disable();
     hal_flash_clear_wp();
 
     /* Reset flash to read-array mode in case previous operation left it
      * in command mode (e.g. after a timeout or incomplete operation) */
     FLASH_IO8_WRITE(0, 0, AMD_CMD_RESET);
     ram_udelay(50);
+    ram_putchar('r'); /* checkpoint: reset done, starting loop */
 
     pos = 0;
     while (len > 0) {
@@ -939,9 +1025,10 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len)
     #endif
 
         hal_flash_unlock_sector(sector);
+        ram_putchar('u'); /* checkpoint: unlock done */
         FLASH_IO8_WRITE(sector, offset, AMD_CMD_WRITE_TO_BUFFER);
-        /* Word count (N-1) must be replicated to both chips */
         FLASH_IO8_WRITE(sector, offset, (nwords-1));
+        ram_putchar('b'); /* checkpoint: buffer cmd + count sent */
 
         for (i=0; i<nwords; i++) {
             const uint8_t* ptr = &data[pos];
@@ -952,23 +1039,43 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len)
         #endif
             pos += (FLASH_CFI_WIDTH/8);
         }
+        ram_putchar('l'); /* checkpoint: data loaded */
+        /* Ensure all data stores reach IFC before confirm */
+        __asm__ __volatile__("sync" ::: "memory");
         FLASH_IO8_WRITE(sector, offset, AMD_CMD_WRITE_BUFFER_CONFIRM);
-        /* Typical 410us */
+        /* Ensure confirm write reaches flash before polling */
+        __asm__ __volatile__("sync; isync" ::: "memory");
+        ram_putchar('c'); /* checkpoint: confirm sent */
 
-        /* poll for program completion - max 200ms */
+        /* poll for program completion - max 200ms (typical 410us) */
         ret = hal_flash_status_wait(sector, 0x44, 200*1000);
+        if (ret == 0) {
+            ram_putchar('p'); /* pass */
+        } else if (ret == -2) {
+            ram_putchar('E'); /* DQ5 error */
+        } else {
+            ram_putchar('T'); /* timeout */
+        }
         if (ret != 0) {
-            /* Reset flash to read-array mode BEFORE calling printf */
+            uint16_t readback;
+            /* Reset flash to read-array mode BEFORE reading back data */
             FLASH_IO8_WRITE(sector, 0, AMD_CMD_RESET);
             ram_udelay(50);
-            wolfBoot_printf("Flash Write: Timeout at sector %d\n", sector);
+            /* Read back offset 0 to see if data was actually written */
+            readback = FLASH_IO16_READ(sector, 0);
+            ram_putchar('=');
+            ram_puthex16(readback);
+            wolfBoot_printf("\nFlash Write: %s at sector %d (ret %d)\n",
+                ret == -2 ? "DQ5 error" : "Timeout", sector, ret);
             break;
         }
+        ram_putchar('.'); /* checkpoint: page write OK */
 
         address += xfer;
         len -= xfer;
     }
 
+    ram_putchar('d'); /* checkpoint: write loop done */
     /* Restore flash caching — flash is back in read-array mode */
     hal_flash_cache_enable();
     return ret;
@@ -1118,15 +1225,24 @@ static void hal_mp_up(uint32_t bootpg, uint32_t spin_table_ddr)
         wolfBoot_printf("MP: Timeout enabling additional cores!\n");
     }
 
-    /* Disable all timebases */
-    set32(RCPM_PCTBENR, 0);
+    /* Synchronize and reset timebase across all cores.
+     * On e6500, mtspr to TBL/TBU (SPR 284/285) may cause an illegal
+     * instruction exception — skip timebase reset if secondary cores
+     * did not start (timebase sync only matters for multi-core). */
+    if ((active_cores & all_cores) == all_cores) {
+        /* Disable all timebases */
+        set32(RCPM_PCTBENR, 0);
 
-    /* Reset our timebase */
-    mtspr(SPRN_TBWU, 0);
-    mtspr(SPRN_TBWL, 0);
+        /* Reset our timebase */
+        mtspr(SPRN_TBWU, 0);
+        mtspr(SPRN_TBWL, 0);
 
-    /* Enable timebase for all cores */
-    set32(RCPM_PCTBENR, all_cores);
+        /* Enable timebase for all cores */
+        set32(RCPM_PCTBENR, all_cores);
+    } else {
+        /* Only re-enable timebase for boot core */
+        set32(RCPM_PCTBENR, (1 << whoami));
+    }
 }
 
 static void hal_mp_init(void)

src/boot_ppc.c

@@ -185,7 +185,7 @@ void boot_entry_C(void)
     }
 
     /* Run wolfBoot! */
-#ifdef ENABLE_DDR
+#if defined(ENABLE_DDR) && defined(DDR_STACK_TOP)
     /* DDR is initialized, .data and .bss are set up.
      * Switch stack from CPC SRAM to DDR for:
      * 1. Better performance (DDR stack is cacheable by L1/L2/CPC)

src/boot_ppc_mp.S

@@ -132,43 +132,15 @@ branch_prediction:
         mtspr   L1CSR2, r8
 
 #if defined(CORE_E6500) /* --- L2 E6500 --- */
-ccsr_tlb_mp:
-        /* e6500 L2 uses memory-mapped CCSR registers (L2_CLUSTER_BASE).
-         * Secondary cores have no TLBs on entry — only the boot page
-         * translation provides initial access. Add a temporary CCSR
-         * mapping (TLB1 entry 2) so L2 setup can access the registers. */
-        set_tlb(1, 2,
-                CCSRBAR, CCSRBAR, CCSRBAR_PHYS_HIGH,
-                MAS3_SW | MAS3_SR, MAS2_I | MAS2_G, 0,
-                CCSRBAR_SIZE, 0, r11)
-l2_setup_cache:
-        /* E6500CORERM: 11.7 L2 cache state */
-        /* R5 = L2 cluster 1 base */
-        lis     r5,     L2_CLUSTER_BASE(0)@h
-        ori     r5, r5, L2_CLUSTER_BASE(0)@l
-        /* Invalidate and clear locks */
-        lis     r1,     (L2CSR0_L2FI | L2CSR0_L2LFC)@h
-        ori     r1, r1, (L2CSR0_L2FI | L2CSR0_L2LFC)@l
-        sync
-        stw     r1, L2CSR0(r5)
-
-        /* poll till invalidate and lock bits are cleared */
-l2_poll_invclear:
-        lwz     r4, L2CSR0(r5)
-        and.    r4, r1, r4
-        bne     l2_poll_invclear
-        isync
-
-        /* set stash id to (coreID * 2) + 32 + L2 (1) */
-        addi    r3, r8,1
-        stw     r3, L2CSR1(r5)
-
-        /* enable L2 with parity */
-        sync
-        isync
-        lis     r4, (L2CSR0_L2E | L2CSR0_L2PE)@h
-        stw     r4, L2CSR0(r5)
-        isync
+        /* e6500 L2 is per-cluster (shared by all cores in the cluster).
+         * The primary core already invalidated and enabled L2 during boot.
+         * Secondary cores must NOT do L2FI (flash invalidate) — it discards
+         * ALL dirty L2 lines including the primary core's stack, return
+         * addresses, and cached code, causing the primary core to crash
+         * (typically SRR0=0 from corrupted return address).
+         * L1 stash ID (set above via L1CSR2 SPR) is per-core and sufficient.
+         * L2CSR1 (stash ID) is per-cluster and already set by core 0.
+         * No CCSR TLB mapping needed since we skip L2 register access. */
 
 #elif defined(CORE_E5500) /* --- L2 E5500 --- */
 l2_setup_cache: