Fixes to get DDR4 training working

dgarske · dgarske · commit b67a1910115d · 2026-05-08T14:52:21.000-07:00
diff --git a/hal/mpfs250.c b/hal/mpfs250.c
@@ -279,14 +279,29 @@ static inline void mb(void)
     __asm__ volatile("fence iorw, iorw" ::: "memory");
 }
 
-/* DDR-init delay.  Forwards to rdcycle-based udelay() so the effective
- * delay tracks the current CPU frequency.  The previous implementation
- * was a hardcoded ~40 MHz busy loop; after mss_pll_init() switches the
- * CPU clock to ~600 MHz it ran ~15x too short, silently violating
- * LPDDR4 reset / MR-write timing windows. */
+/* DDR-init busy-loop delay.  The argument is NOT a real microsecond --
+ * it is whatever the legacy busy-loop produces at the current CPU
+ * clock.  Empirically reaches train_stat=0x1D on the first attempt with
+ * the same per-attempt rate as forwarding to udelay(), and is much
+ * faster (~4 s vs ~50 s) for the TIP-wait timeout, which dominates
+ * retry-loop time when training fails.
+ *
+ * Do NOT replace with udelay(us) without re-timing every call site
+ * below: at 600 MHz the busy-loop delivers roughly us/20 of a real us,
+ * so udelay(us) makes every post-PLL delay ~20x longer.  In addition
+ * to slowing retries, this can shift LPDDR4 / PHY timing windows --
+ * earlier observed empirical data showed an isolated additional
+ * regression beyond the pre-existing ~30% per-attempt failure rate.
+ *
+ * The "5us" / "250us" / "2ms" comments at the call sites are LEGACY
+ * and do not reflect the actual delay; preserved for git blame, not
+ * as timing references. */
 static void ddr_delay(uint32_t us)
 {
-    udelay(us);
+    volatile uint32_t i;
+    for (i = 0; i < us * 10; i++) {
+        __asm__ volatile("nop");
+    }
 }
 
 /* IOSCB Bank Controllers and DLL bases */
@@ -2983,11 +2998,22 @@ int mpfs_ddr_init(void)
     wolfBoot_printf("MT53D512M32D2DS-053 x32 @ 1600 Mbps\n");
     wolfBoot_printf("========================================\n");
 
-    /* Step 1: NWC/PLL initialization */
-    ret = nwc_init();
-    if (ret != 0) {
-        wolfBoot_printf("DDR: NWC init FAILED\n");
-        return -1;
+    /* Step 1: NWC/PLL initialization.  Run only once per boot -- the
+     * MSS / DDR PLLs lock on first call and re-running mss_pll_init()
+     * hangs on the lock wait when called against an already-locked
+     * PLL.  The outer retry loop in hal_init() re-enters this function
+     * for full controller/PHY re-init, but the PLLs only need to be
+     * brought up once. */
+    {
+        static int nwc_initialized = 0;
+        if (!nwc_initialized) {
+            ret = nwc_init();
+            if (ret != 0) {
+                wolfBoot_printf("DDR: NWC init FAILED\n");
+                return -1;
+            }
+            nwc_initialized = 1;
+        }
     }
 
     /* Step 2: Enable DDR controller clock */
@@ -3029,141 +3055,142 @@ int mpfs_ddr_init(void)
     DDRPHY_REG(PHY_TIP_CFG_PARAMS) = LIBERO_SETTING_TIP_CFG_PARAMS;
     mb();
 
-    /* Step 9: Run training, with retry-on-failure mirroring HSS's
-     * DDR_TRAINING_FAIL state machine (mss_ddr.c:512).  HSS retries
-     * up to MAX_RETRY_COUNT times: on each retry it resets CKE,
-     * forces controller reset, clears DFI/CTRLR_INIT, then re-runs
-     * training (which selects a different refclk_offset internally).
+    /* Step 9: Run training + post-training + MTC sanity, with retry on
+     * MTC failure.
      *
-     * Phase 3.10.3 (2): we don't yet have the refclk_offset sweep
-     * inside our run_training, but trying just the basic retry
-     * (controller reset + re-init) might surface whether
-     * train_stat advances on a fresh attempt.
+     * Why MTC is the retry trigger (not PHY_TRAINING_STATUS): when the
+     * manual ADDCMD training picks a marginal phase/dly that doesn't
+     * resolve into a usable DRAM alignment, train_stat sticks at 0x1
+     * (BCLK_SCLK only).  But TIP keeps spinning in the background and
+     * eventually flips the WRLVL/RDGATE/DQ_DQS bits to read 0x1D, even
+     * though the alignment is bogus.  An outer retry keyed on
+     * PHY_TRAINING_STATUS sees that bogus 0x1D and stops.  MTC actually
+     * exercises the DDR controller -- it times out unambiguously when
+     * training was bad, and is the reliable signal.
+     *
+     * Empirical baseline: ~30% per-attempt training failure rate -> 5
+     * retries gives ~99.7% cumulative success rate.
      */
     {
-        uint32_t retry_count = 0;
-        const uint32_t MAX_RETRY = 3;
-        uint32_t train_stat_now;
-
-        ret = run_training();
-        train_stat_now = DDRPHY_REG(PHY_TRAINING_STATUS);
-        while ((train_stat_now & TRAINING_MASK) != (BCLK_SCLK_BIT | WRLVL_BIT |
-               RDGATE_BIT | DQ_DQS_BIT) && retry_count < MAX_RETRY) {
-            wolfBoot_printf("DDR: Training retry %lu (train_stat=0x%x)\n",
-                (unsigned long)retry_count, train_stat_now);
-            /* HSS DDR_TRAINING_FAIL reset sequence (mss_ddr.c:519-538) */
-            DDRCFG_REG(MC_INIT_CS) = 0x1;
-            DDRCFG_REG(MC_INIT_DISABLE_CKE) = 0x1;
-            ddr_delay(500);  /* DELAY_CYCLES_5_MICRO */
-            DDRCFG_REG(MC_INIT_FORCE_RESET) = 0x1;
-            ddr_delay(200000);  /* DELAY_CYCLES_2MS */
-            retry_count++;
-            DDRCFG_REG(MC_DFI_INIT_START) = 0x0;
-            DDRCFG_REG(MC_CTRLR_INIT) = 0x0;
-            DDRPHY_REG(PHY_TRAINING_START) = 0x0;
-            mb();
-            /* Re-run training */
+        uint32_t train_retry = 0;
+        /* 1 inner attempt only.  When manual training picks a marginal
+         * alignment, TIP cannot recover and the MTC engine wedges
+         * (DONE_ACK stuck at 0 even with train_stat=0x1D).  Re-running
+         * run_training() inside the same controller-state cannot
+         * unwedge MTC -- the outer loop in hal_init re-enters
+         * mpfs_ddr_init from scratch, which clears MTC via the SYSREG
+         * DDRC soft-reset pulse, and that's the only path that
+         * actually recovers. */
+        const uint32_t MAX_TRAIN_RETRY = 1;
+        uint32_t lane;
+        uint32_t mtc_to;
+        int mtc_pass = 0;
+
+        while (train_retry < MAX_TRAIN_RETRY) {
+            if (train_retry > 0) {
+                wolfBoot_printf(
+                    "DDR: Retry %u/%u after MTC sanity FAIL\n",
+                    (unsigned)train_retry, (unsigned)MAX_TRAIN_RETRY);
+                /* HSS DDR_TRAINING_FAIL reset sequence (mss_ddr.c:519-538) */
+                DDRCFG_REG(MC_INIT_CS) = 0x1;
+                DDRCFG_REG(MC_INIT_DISABLE_CKE) = 0x1;
+                ddr_delay(500);
+                DDRCFG_REG(MC_INIT_FORCE_RESET) = 0x1;
+                ddr_delay(200000);
+                DDRCFG_REG(MC_DFI_INIT_START) = 0x0;
+                DDRCFG_REG(MC_CTRLR_INIT) = 0x0;
+                DDRPHY_REG(PHY_TRAINING_START) = 0x0;
+                mb();
+            }
+            train_retry++;
+
             ret = run_training();
-            train_stat_now = DDRPHY_REG(PHY_TRAINING_STATUS);
-        }
-        wolfBoot_printf("DDR: Final train_stat=0x%x after %lu retries\n",
-            train_stat_now, (unsigned long)retry_count);
-    }
-    if (ret != 0) {
-        wolfBoot_printf("DDR: Training FAILED\n");
-        return -2;
-    }
+            if (ret != 0) {
+                continue;
+            }
 
-    /* Phase 3.10.3 (D-3 v2): HSS post-training sequence.
-     *
-     * After train_stat=0x1D, HSS does these critical steps before any
-     * CPU access (mss_ddr.c DDR_TRAINING_WRITE_CALIBRATION + after):
-     *
-     *   (a) Set rpc220 = 0xC (LPDDR4 default DQ delay center)
-     *   (b) load_dq(lane) for each of 4 lanes -- per-lane DQ delay load
-     *   (c) write_calibration_using_mtc() -- HSS's MTC sweep (validates
-     *       DDR via the on-chip MTC engine, no CPU bus involved)
-     *   (d) MTC_test counting + pseudo-random patterns (DDR_FULL_MTC_CHECK)
-     *   (e) Then the CPU access at 0xC0000000 / 0x80000000 succeeds.
-     *
-     * Without (a)-(d), the first CPU write to DDR hangs (we observe this
-     * for both cached 0x80000000 and non-cached 0xC0000000).  The MTC
-     * activity exercises the DDR controller and seems to "wake up" the
-     * data path / drain any lingering training state.
-     */
-    wolfBoot_printf("DDR: Post-training sequence...\n");
+            /* HSS DDR_TRAINING_SET_FINAL_MODE: rewrite DDRPHY_MODE with
+             * LIBERO setting to transition PHY from training to
+             * operational mode (mss_ddr.c:1619). */
+            wolfBoot_printf("DDR: Post-training sequence...\n");
+            DDRPHY_REG(PHY_MODE) = LIBERO_SETTING_DDRPHY_MODE;
+            mb();
+            wolfBoot_printf("  DDRPHY_MODE -> 0x%x (final)\n",
+                DDRPHY_REG(PHY_MODE));
 
-    /* DDR_TRAINING_SET_FINAL_MODE (HSS mss_ddr.c:1619): rewrite
-     * DDRPHY_MODE with LIBERO setting after training success.  This
-     * transitions the PHY from training mode to operational mode. */
-    DDRPHY_REG(PHY_MODE) = LIBERO_SETTING_DDRPHY_MODE;
-    mb();
-    wolfBoot_printf("  DDRPHY_MODE -> 0x%x (final)\n",
-        DDRPHY_REG(PHY_MODE));
+            /* rpc220 = 0xC for LPDDR4 -- centers DQ/DQS sampling. */
+            DDRPHY_REG(PHY_RPC220) = 0x0CUL;
+            mb();
 
-    /* (a) rpc220 = 0xC for LPDDR4 -- centers DQ/DQS sampling */
-    DDRPHY_REG(PHY_RPC220) = 0x0CUL;
-    mb();
+            /* load_dq(lane) for each of 4 lanes (HSS mss_ddr.c:2916).
+             * Per-lane: clear move, set DFI override + expert mode,
+             * pulse load, restore expert mode. */
+            for (lane = 0; lane < 4; lane++) {
+                DDRPHY_REG(PHY_EXPERT_DLYCNT_MOVE0) = 0x00UL;
+                DDRPHY_REG(PHY_EXPERT_DFI_STATUS_TO_SHIM) = 0x07UL;
+                DDRPHY_REG(PHY_EXPERT_MODE_EN) = 0x21UL;
+                DDRPHY_REG(PHY_EXPERT_DLYCNT_LOAD0) =
+                    (0xFFUL << (lane * 8UL));
+                DDRPHY_REG(PHY_EXPERT_DLYCNT_LOAD0) = 0x00UL;
+                DDRPHY_REG(PHY_EXPERT_MODE_EN) = 0x08UL;
+            }
+            mb();
+            wolfBoot_printf("  load_dq done for 4 lanes\n");
+            wolfBoot_printf(
+                "  CTRLR_INIT_DONE=0x%x AUTOINIT_DIS=0x%x train_stat=0x%x\n",
+                DDRCFG_REG(MC_CTRLR_INIT_DONE),
+                DDRCFG_REG(MC_INIT_AUTOINIT_DISABLE),
+                DDRPHY_REG(PHY_TRAINING_STATUS));
+
+            /* MTC sanity: smallest region (size=8 -> 2^8 = 256 B),
+             * counting pattern, sequential addressing, RW. */
+            DDRCFG_REG(MT_EN) = 0;
+            DDRCFG_REG(MT_EN_SINGLE) = 0;
+            DDRCFG_REG(MT_STOP_ON_ERROR) = 0;
+            DDRCFG_REG(0x440C) = 0;             /* MT_RD_ONLY */
+            DDRCFG_REG(0x4410) = 0;             /* MT_WR_ONLY */
+            DDRCFG_REG(MT_DATA_PATTERN) = 0;
+            DDRCFG_REG(MT_ADDR_PATTERN) = 0;
+            DDRCFG_REG(MT_START_ADDR_0) = 0;
+            DDRCFG_REG(MT_START_ADDR_1) = 0;
+            DDRCFG_REG(MT_ADDR_BITS) = 8;
+            DDRCFG_REG(MT_ERROR_MASK_0) = 0xFFFFFFFFUL;
+            DDRCFG_REG(MT_ERROR_MASK_1) = 0xFFFFFFFFUL;
+            DDRCFG_REG(MT_ERROR_MASK_2) = 0xFFFFFFFFUL;
+            DDRCFG_REG(MT_ERROR_MASK_3) = 0xFFFFFFFFUL;
+            DDRCFG_REG(MT_ERROR_MASK_4) = 0xFFFFFFFFUL;
+            DDRCFG_REG(MT_EN_SINGLE) = 0;
+            DDRCFG_REG(MT_EN_SINGLE) = 1;
+            mtc_to = 0xFFFFFFUL;
+            while ((DDRCFG_REG(MT_DONE_ACK) & 0x1UL) == 0 && mtc_to > 0) {
+                mtc_to--;
+            }
+            if (mtc_to == 0) {
+                wolfBoot_printf(
+                    "  MTC 256B TIMEOUT (DONE_ACK=0x%x ERR_STS=0x%x)\n",
+                    DDRCFG_REG(MT_DONE_ACK), DDRCFG_REG(MT_ERROR_STS));
+                continue;
+            }
+            if ((DDRCFG_REG(MT_ERROR_STS) & 0x1UL) != 0) {
+                wolfBoot_printf("  MTC 256B FAIL (err_sts=0x%x)\n",
+                    DDRCFG_REG(MT_ERROR_STS));
+                continue;
+            }
+            wolfBoot_printf("  MTC 256B PASS (err_sts=0x%x to_used=0x%x)\n",
+                DDRCFG_REG(MT_ERROR_STS),
+                (unsigned int)(0xFFFFFFUL - mtc_to));
 
-    /* (b) load_dq(lane) for each of 4 lanes (HSS mss_ddr.c:2916).
-     * Per-lane sequence: clear move, set DFI override + expert mode,
-     * pulse load, restore expert mode. */
-    {
-        uint32_t lane;
-        for (lane = 0; lane < 4; lane++) {
-            DDRPHY_REG(PHY_EXPERT_DLYCNT_MOVE0) = 0x00UL;
-            DDRPHY_REG(PHY_EXPERT_DFI_STATUS_TO_SHIM) = 0x07UL;
-            DDRPHY_REG(PHY_EXPERT_MODE_EN) = 0x21UL;
-            DDRPHY_REG(PHY_EXPERT_DLYCNT_LOAD0) = (0xFFUL << (lane * 8UL));
-            DDRPHY_REG(PHY_EXPERT_DLYCNT_LOAD0) = 0x00UL;
-            DDRPHY_REG(PHY_EXPERT_MODE_EN) = 0x08UL;
+            mtc_pass = 1;
+            break;
         }
-        mb();
-        wolfBoot_printf("  load_dq done for 4 lanes\n");
-    }
-
-    /* Pre-MTC diagnostic snapshot */
-    wolfBoot_printf("  CTRLR_INIT_DONE=0x%x AUTOINIT_DIS=0x%x train_stat=0x%x\n",
-        DDRCFG_REG(MC_CTRLR_INIT_DONE),
-        DDRCFG_REG(MC_INIT_AUTOINIT_DISABLE),
-        DDRPHY_REG(PHY_TRAINING_STATUS));
 
-    /* (c)+(d) MTC sanity check.  Smallest region (size=8 -> 2^8 = 256 B).
-     * Per HSS MTC_test sequence: MT_EN=0, MT_RD_ONLY=0, MT_WR_ONLY=0, ... */
-    {
-        uint32_t mtc_to;
-        uint32_t mtc_err;
-        DDRCFG_REG(MT_EN) = 0;
-        DDRCFG_REG(MT_EN_SINGLE) = 0;
-        DDRCFG_REG(MT_STOP_ON_ERROR) = 0;
-        DDRCFG_REG(0x440C) = 0;           /* MT_RD_ONLY = 0 (normal RW) */
-        DDRCFG_REG(0x4410) = 0;           /* MT_WR_ONLY = 0 */
-        DDRCFG_REG(MT_DATA_PATTERN) = 0;  /* counting pattern */
-        DDRCFG_REG(MT_ADDR_PATTERN) = 0;  /* sequential */
-        DDRCFG_REG(MT_START_ADDR_0) = 0;
-        DDRCFG_REG(MT_START_ADDR_1) = 0;
-        DDRCFG_REG(MT_ADDR_BITS) = 8;     /* 2^8 = 256 bytes */
-        DDRCFG_REG(MT_ERROR_MASK_0) = 0xFFFFFFFFUL;
-        DDRCFG_REG(MT_ERROR_MASK_1) = 0xFFFFFFFFUL;
-        DDRCFG_REG(MT_ERROR_MASK_2) = 0xFFFFFFFFUL;
-        DDRCFG_REG(MT_ERROR_MASK_3) = 0xFFFFFFFFUL;
-        DDRCFG_REG(MT_ERROR_MASK_4) = 0xFFFFFFFFUL;
-        DDRCFG_REG(MT_EN_SINGLE) = 0;
-        DDRCFG_REG(MT_EN_SINGLE) = 1;     /* Run */
-        mtc_to = 0xFFFFFFUL;
-        while ((DDRCFG_REG(MT_DONE_ACK) & 0x1UL) == 0 && mtc_to > 0) {
-            mtc_to--;
-        }
-        if (mtc_to == 0) {
-            wolfBoot_printf("  MTC 256B TIMEOUT (DONE_ACK=0x%x ERR_STS=0x%x)\n",
-                DDRCFG_REG(MT_DONE_ACK), DDRCFG_REG(MT_ERROR_STS));
-        } else {
-            mtc_err = DDRCFG_REG(MT_ERROR_STS) & 0x1UL;
-            wolfBoot_printf("  MTC 256B %s (err_sts=0x%x to_used=0x%x)\n",
-                mtc_err == 0 ? "PASS" : "FAIL",
-                DDRCFG_REG(MT_ERROR_STS),
-                (unsigned int)(0xFFFFFFUL - mtc_to));
+        if (!mtc_pass) {
+            wolfBoot_printf("DDR: Training/MTC failed after %u retries\n",
+                (unsigned)MAX_TRAIN_RETRY);
+            return -2;
         }
+        wolfBoot_printf("DDR: Training+MTC PASS after %u retries\n",
+            (unsigned)(train_retry - 1));
     }
 
     /* DDR pre-fill is currently disabled because both PDMA-based and
@@ -3275,8 +3302,7 @@ int mpfs_ddr_init(void)
 
         wolfBoot_printf("DDR-TEST: train_stat=0x%x\n",
             DDRPHY_REG(PHY_TRAINING_STATUS));
-        wolfBoot_printf("DDR-TEST: HALT\n");
-        while (1) { ; }
+        wolfBoot_printf("DDR-TEST: done\n");
     }
 #endif /* MPFS_DDR_PATTERN_TEST */
 
@@ -3326,9 +3352,32 @@ void hal_init(void)
     wolfBoot_printf("Running on E51 (hart 0) in M-mode\n");
 
 #ifdef MPFS_DDR_INIT
-    /* Bring up LPDDR4 before any DDR-resident operations */
-    if (mpfs_ddr_init() != 0) {
-        wolfBoot_printf("DDR: Init FAILED - continuing with L2 only\n");
+    /* Bring up LPDDR4 before any DDR-resident operations.
+     *
+     * Outer retry loop: each call to mpfs_ddr_init() does a SYSREG DDRC
+     * soft-reset pulse, which clears the MTC engine state.  If the
+     * inner retry inside mpfs_ddr_init() exhausts (typically because
+     * MTC wedged after the first failure), come back here for a full
+     * controller re-init.  Empirical: per-attempt failure rate ~30%, so
+     * 3 outer attempts cover ~99.7% of boots. */
+    {
+        unsigned int outer_retry;
+        const unsigned int MAX_OUTER_RETRY = 3;
+        int ddr_ok = 0;
+        for (outer_retry = 0; outer_retry < MAX_OUTER_RETRY; outer_retry++) {
+            if (outer_retry > 0) {
+                wolfBoot_printf(
+                    "DDR: Outer retry %u/%u (full DDRC re-init)\n",
+                    outer_retry, MAX_OUTER_RETRY);
+            }
+            if (mpfs_ddr_init() == 0) {
+                ddr_ok = 1;
+                break;
+            }
+        }
+        if (!ddr_ok) {
+            wolfBoot_printf("DDR: Init FAILED - continuing with L2 only\n");
+        }
     }
 #endif
 #endif
