Peer review and debug cleanups

Author: dgarske

docs/Targets.md

@@ -3393,6 +3393,56 @@ Executing Initialization File: /opt/Freescale/CodeWarrior_PA_10.5.1/PA/PA_Suppor
 thread break: Stopped, 0x0, 0x0, cpuPowerPCBig,  Connected (state, tid, pid, cpu, target)
 ```
 
+### Known Issues and Workarounds (NXP T2080)
+
+#### RAMFUNCTION and Flash XIP Constraints
+
+The NAII 68PPC2 uses two S29GL01GS x8 NOR flash chips in parallel (16-bit bus)
+on IFC CS0. wolfBoot executes in place (XIP) from this flash. AMD NOR flash
+enters command mode (autoselect, program, erase) bank-wide, so instruction
+fetch from flash during these operations returns status data instead of code.
+
+All flash write/erase functions and their helpers must be marked `RAMFUNCTION`
+to place them in the `.ramcode` section, which is copied to DDR and remapped
+via TLB9. Key constraints:
+
+- **No calls to flash-resident functions**: The linker generates trampolines
+  for cross-section calls that jump back to flash addresses. `ram_udelay()`
+  originally called `hal_get_plat_clk()` (in flash `.text`) via a trampoline,
+  causing crashes during flash command mode. Fixed by pre-computing the delay
+  ticks constant (`ram_delay_ticks`) in `hal_flash_init()`.
+
+- **Inline TLB and cache operations**: `hal_flash_cache_disable()` and
+  `hal_flash_cache_enable()` use `ram_write_tlb()` (inline mtspr) and direct
+  L1CSR0/L1CSR1 manipulation instead of calling flash-resident `set_tlb()`,
+  `invalidate_dcache()`, or `invalidate_icache()`.
+
+- **Write-to-Buffer-Abort-Reset**: After a WBP failure (DQ1 abort or timeout),
+  plain `AMD_CMD_RESET` (0xF0) is ignored. The S29GL01GS requires the full
+  unlock + reset sequence to exit the WBP-abort state.
+
+#### Write-Buffer-Program Timing
+
+The S29GL01GS write-buffer-program (WBP) command sequence (unlock, 0x25,
+word count, data, 0x29 confirm) is timing-sensitive. The entire sequence from
+unlock through confirm must execute without bus-stalling delays (e.g., UART
+output via `ram_putchar`). Inserting delays of ~87us (one UART character at
+115200 baud) between WBP steps triggers a DQ1 abort on the flash.
+
+#### Multi-Core (ENABLE_MP)
+
+The e6500 L2 cache is per-cluster (shared by all 4 cores). Secondary cores
+must NOT perform L2 flash-invalidate (L2FI), as this discards all dirty L2
+lines including the primary core's stack and code. The `boot_ppc_mp.S`
+secondary core startup skips L2 init for e6500 (primary core already
+initialized the shared L2). Secondary cores only set L1 stash ID via L1CSR2.
+
+#### Cold Boot Stack
+
+CPC SRAM is unreliable as early stack on cold power-on due to CoreNet bus
+errors. The T2080 port uses L1 locked D-cache (16KB at `L1_CACHE_ADDR`) as
+the initial stack, matching the U-Boot approach.
+
 
 ## NXP MCXA153
 

hal/nxp_ppc.h

@@ -205,6 +205,7 @@ static inline void uart_putc_early(char c)
     volatile uint8_t *thr = (volatile uint8_t *)(CCSRBAR + 0x11C500UL);
     while (!(*lsr & 0x20)) {}  /* poll THRE */
     *thr = (uint8_t)c;
+    __asm__ __volatile__("eieio" : : : "memory");
 }
 #else
 static inline void uart_putc_early(char c) { (void)c; }

hal/nxp_t2080.c

@@ -169,7 +169,7 @@ static uint32_t hal_get_bus_clk(void)
 #define DELAY_US  (TIMEBASE_HZ / 1000000)
 static void udelay(uint32_t delay_us)
 {
-    wait_ticks(delay_us * DELAY_US);
+    wait_ticks((unsigned long long)delay_us * DELAY_US);
 }
 
 /* Pre-computed DELAY_US value for ram_udelay() (RAMFUNCTION).
@@ -710,19 +710,6 @@ static void RAMFUNCTION ram_putchar(char c)
         ;
     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.
  * Uses pre-computed ram_delay_ticks (initialized before flash ops).
  * Cannot call DELAY_US macro — it expands to hal_get_plat_clk() in flash
@@ -814,8 +801,10 @@ static void RAMFUNCTION hal_flash_clear_wp(void)
         __asm__ __volatile__("sync; isync");
         /* Verify WP cleared */
         cspr = get32(IFC_CSPR(0));
+    #ifdef DEBUG_UART
         wolfBoot_printf("WP clear: CSPR0=0x%x%s\n", cspr,
             (cspr & IFC_CSPR_WP) ? " (FAILED)" : " (OK)");
+    #endif
     }
 }
 
@@ -939,14 +928,6 @@ static int RAMFUNCTION hal_flash_status_wait(uint32_t sector, uint16_t mask,
         read1 = FLASH_IO8_READ(sector, 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
@@ -998,24 +979,13 @@ 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 */
-
-    /* Diagnostic: read erased data before write to verify both chips respond.
-     * Expected 0xFFFF after erase. If high byte != 0xFF, chip may be dead. */
-    {
-        uint16_t erased = FLASH_IO16_READ(address / FLASH_SECTOR_SIZE, 0);
-        ram_putchar('{');
-        ram_puthex16(erased);
-        ram_putchar('}');
-    }
 
     pos = 0;
     while (len > 0) {
@@ -1033,12 +1003,14 @@ int RAMFUNCTION hal_flash_write(uint32_t address, const uint8_t *data, int len)
             sector, offset, xfer, pos);
     #endif
 
+        /* Write-Buffer-Program: the entire command sequence from unlock
+         * through confirm must execute without interruption. No UART
+         * output or other bus-stalling operations between steps —
+         * S29GL01GS has a write-buffer load timeout (~25-50us) that
+         * causes DQ1 abort if data or confirm arrives too late. */
         hal_flash_unlock_sector(sector);
-        ram_putchar('u'); /* checkpoint: unlock done */
         FLASH_IO8_WRITE(sector, offset, AMD_CMD_WRITE_TO_BUFFER);
         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];
         #if FLASH_CFI_WIDTH == 16
@@ -1048,57 +1020,28 @@ 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);
-        /* Ensure confirm write reaches flash before polling */
-        __asm__ __volatile__("sync; isync" ::: "memory");
-        ram_putchar('c'); /* checkpoint: confirm sent */
 
         /* 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) {
-            uint16_t readback;
             /* Write-to-Buffer-Abort-Reset: plain AMD_CMD_RESET (0xF0) is
              * IGNORED during an active WBP or WBP-abort state. Must use
              * the unlock + reset sequence per S29GL01GS datasheet. */
             hal_flash_unlock_sector(sector);
             FLASH_IO8_WRITE(sector, 0, AMD_CMD_RESET);
             ram_udelay(50);
-            /* Read back offset 0 to see if data was actually written */
-            readback = FLASH_IO16_READ(sector, 0);
-            ram_putchar('=');
-            ram_puthex16(readback);
-            /* Verify flash exited command mode (second read should match) */
-            {
-                uint16_t verify = FLASH_IO16_READ(sector, 0);
-                if (readback != verify) {
-                    ram_putchar('!'); /* still in command mode */
-                    /* Try one more reset */
-                    hal_flash_unlock_sector(sector);
-                    FLASH_IO8_WRITE(sector, 0, AMD_CMD_RESET);
-                    ram_udelay(50);
-                }
-            }
-            wolfBoot_printf("\nFlash Write: %s at sector %d (ret %d)\n",
-                ret == -2 ? "DQ5 error" : "Timeout", sector, ret);
+            wolfBoot_printf("Flash Write: %s at sector %d\n",
+                ret == -2 ? "DQ5 error" : "Timeout", sector);
             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;
@@ -1370,8 +1313,12 @@ int hal_dts_fixup(void* dts_addr)
         fdt_version(fdt), fdt_totalsize(fdt));
 
     /* expand total size */
-    fdt->totalsize += 2048; /* expand by 2KB */
-    wolfBoot_printf("FDT: Expanded (2KB) to %d bytes\n", fdt->totalsize);
+    {
+        uint32_t new_size = (uint32_t)fdt_totalsize(fdt) + 2048U;
+        fdt_set_totalsize(fdt, new_size);
+        wolfBoot_printf("FDT: Expanded (2KB) to %d bytes\n",
+            fdt_totalsize(fdt));
+    }
 
     /* fixup the memory region - single bank */
     off = fdt_find_devtype(fdt, -1, "memory");