diff --git a/CLAUDE.md b/CLAUDE.md index e83301b..7d0c018 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -88,18 +88,9 @@ Reset/control, EXI integrity (speed sweep `ok=[1,1,1,1,1,1]`, per-speed dump clean at all six indices, raw reg dump `0x17` in slot 0), Ethernet, and SPRAM/DMA. The EXI front end runs at the real 27 MHz hardware rate. -(The ID tests report SKIP, not FAIL, once `id_phase` has been consumed — -reflash the FPGA to restore it. They decide that from the hardware, via -`iface_alive()`: HIPR and NWAYS are hardwired and independent of `id_phase`, so -if both read correctly the EXI path is fine and only a spent `id_phase` can -explain a wrong answer at address 0. - -⚠️ The `g_ncra_reset_done` flag alone is NOT sufficient, and this bites every -time a build is delivered over the wire: netboot replaces the DOL and restarts -the app with the flag clear, while the FPGA keeps running with `id_phase` -already spent by the previous run. The app then queries address 0, gets NCRA -back — correct behaviour — and called it a FAILURE. Software state cannot stand -in for hardware state across a self-update.) +(The ID tests simply PASS or FAIL now, in any order, at any point in a run — +see "The device ID is answerable at any time" below. The old SKIP machinery, +`iface_alive()` and `g_ncra_reset_done` are all gone.) **Timing closes on EVERY seed (re-measured 2026-10-10).** `python -m exi_bba.synth --seeds 8`: @@ -563,9 +554,11 @@ until byte 0 has been decoded — which is the latency in question. constant and belongs to the physical interface, not the register file, and `byte 0 == 0x00` identifies the query exactly (bit 7 clear = command space, bit 6 clear = read, reg 0). `SPIMode3Slave` recognises it and answers -`04 02 02 00` from the EXI clock domain with **no CDC at all**. The only thing -it needs from `sync` is `id_phase`, a single bit that changes once, crossed -with an `FFSynchronizer`. +`04 02 02 00` from the EXI clock domain with **no CDC at all** — and since +2026-10-10 it needs nothing from `sync` either: `id_arm` is tied high (the +`id_phase` bit and its `FFSynchronizer` are gone). The only remaining use of +the port is the `PHASE_METER` build, which disarms the fast path so the +constant `0x80` is not replaced by ID bytes. The register file's ID path was **removed**, not kept as a fallback: left in, it pushes bytes a 2-byte header has too few pops to drain — exactly how an @@ -745,12 +738,38 @@ not clear it (RST_N floats to its pull-up while the FPGA is unconfigured). A 12 V power cycle is required. Check ETH_3V3 and GC_ON (U12 gates the W5100 rail off GC_ON) before suspecting gateware. -**`id_phase` is a ONE-SHOT.** The device ID is returned for reads of address 0 -only until the first NCRA write with the reset bit, which clears `id_phase` -permanently (`bba_register_file.py:330`); after that address 0 reads back NCRA. -This is correct — it matches the GC's init order — but it means a re-run of any -probe that already issued an NCRA reset will read `00 00 00 00` for the device -ID and look broken. Reconfigure the FPGA (`iceprog -t` is enough) to restore it. +### ✅ The device ID is answerable AT ANY TIME (one-shot removed 2026-10-10) + +`id_phase` **used to be a one-shot**: set at reset and cleared permanently by +the GC's first NCRA write with the reset bit, after which a command-space read +of register 0 returned `0x00000000`. That is now gone — `id_arm` is tied high +and the register file has no `id_phase` output. + +⚠️ **The one-shot broke Swiss outright, which is how it was caught.** Swiss +calls `EXI_GetID` from `bba_exists()` *and* again from `bba_requires_init()`, +long after init has run: + +```c +if ((location & LOC_SERIAL_PORT_1) && EXI_GetID(EXI_CHANNEL_0, EXI_DEVICE_2, &id)) { + switch (id) { case EXI_MX98730EC_ID: ... +``` + +With the one-shot spent, that read returns `0x00000000`, no case matches, and +Swiss reports **no adapter at all** — not a broken one, absent. Running +`gc_bba_test` first was enough to spend it, so the adapter would "disappear" +after any test session. + +**The old justification was wrong.** The comment claimed the fast path would +otherwise "hijack address 0 forever so the GC could never read NCRA back". +NCRA is **register space** (`0x80`/`0xC0` header); the fast path requires +byte 0 == `0x00`, i.e. **command space**. They were never ambiguous, and no +driver reads command register 0 for anything else (libogc2 reads +`0x01`/`0x03`/`0x0B`/`0x0F`, Swiss reads `0x03`). Real hardware answers +`EXI_GetID` whenever asked — that is how EXI enumerates devices. + +Test: `bba_top` **T1b** reads the device ID *after* an NCRA reset, which T1 +(before any reset) structurally cannot catch. Mutation-tested: disarming the +fast path returns `00 00 00 00` for both, exactly what Swiss saw. Two bring-up gotchas, both cost real debugging time: - **The master must sample MISO during the CLK-LOW phase.** The FPGA drives MISO diff --git a/exi_bba/bba_register_file.py b/exi_bba/bba_register_file.py index 7e36b56..2d257d4 100644 --- a/exi_bba/bba_register_file.py +++ b/exi_bba/bba_register_file.py @@ -107,13 +107,6 @@ class BBARegisterFile(Elaboratable): # (DMA) bulk reads, not just ≤4-byte immediate transfers. self.cs_active = Signal() - # One-shot: set at reset, cleared by the GC's first NCRA reset. While - # set, a command-space read of register 0 returns the device ID -- but - # the REPLY is produced by SPIMode3Slave's fast path, not here, because - # a 2-byte command header leaves too little time for a round trip - # through both byte FIFOs. This output is what arms that path. - self.id_phase = Signal(init=1) - # ── Ring-overflow protection (RBFI) ─────────────────────────────── # RRP out to RXFrameAssembler so it can tell how much of the ring is # still unread and DROP frames instead of overwriting them. @@ -350,9 +343,6 @@ class BBARegisterFile(Elaboratable): # length for any driver that skips TWD. txdata_cnt = Signal(16) - # True until first NCRA reset write: return device ID on addr=0 reads - id_phase = self.id_phase - # Per-byte SPRAM read handshake (register-read path): sp_req marks a # request in flight; drain_ctr counts the read-phase dummy bytes. sp_req = Signal() @@ -603,11 +593,29 @@ class BBARegisterFile(Elaboratable): with m.State("CHDR_1"): with m.If(self.rx_rdy): m.d.comb += self.rx_en.eq(1) - # The device-ID query (command reg 0 while id_phase) is - # answered by SPIMode3Slave, not here: pushing anything - # for it would leave stale bytes in the tx FIFO that a - # 2-byte header has too few pops to drain. - with m.If(~is_write & (cmd_reg == 0) & id_phase): + # The device-ID query (command space, read, reg 0) is + # answered by SPIMode3Slave's fast path, not here: pushing + # anything for it would leave stale bytes in the tx FIFO + # that a 2-byte header has too few pops to drain. + # + # ⚠️ NO LONGER GATED ON A ONE-SHOT. This used to be + # `& id_phase`, a bit cleared by the GC's first NCRA + # reset, after which the device stopped identifying + # itself. Real hardware answers EXI_GetID whenever the + # GC asks -- that is how EXI enumerates devices -- and + # Swiss calls it from bba_exists() and again from + # bba_requires_init() long after init. With the one-shot + # spent, EXI_GetID returned 0x00000000 and Swiss reported + # NO ADAPTER AT ALL. + # + # The old justification was that the fast path would + # "hijack address 0 forever so the GC could never read + # NCRA back". That was wrong: NCRA is REGISTER space + # (0x80/0xC0 header) and this path requires byte 0 == + # 0x00, i.e. COMMAND space. They were never ambiguous, + # and no driver reads command register 0 for anything + # else (libogc2 reads 0x01/0x03/0x0B/0x0F, Swiss 0x03). + with m.If(~is_write & (cmd_reg == 0)): m.next = "DEVID" with m.Elif(is_write): m.next = "CMD_WRITE" @@ -661,7 +669,6 @@ class BBARegisterFile(Elaboratable): with m.If(self.rx_data[0]): m.d.sync += r_ncra[0].eq(0) # RESET self-clears m.d.sync += ncra_rst_pulse.eq(1) - m.d.sync += id_phase.eq(0) # An NCRA reset resets the RX ring, so the # pointers the GC reads MUST follow -- the # assembler's own write pointer goes back to diff --git a/exi_bba/bba_top.py b/exi_bba/bba_top.py index cb41d58..dd6b5a4 100644 --- a/exi_bba/bba_top.py +++ b/exi_bba/bba_top.py @@ -190,7 +190,6 @@ class BBATop(Elaboratable): # EXI outputs self.exi_miso.eq(cap.spi_miso), self.exi_miso_next.eq(cap.miso_next), - cap.id_phase.eq(reg.id_phase), # Ring-overflow protection: the assembler needs the GC's read # pointer to know what it must not overwrite, and reports a # dropped frame back as RBFI (IR[7]). @@ -626,6 +625,25 @@ if __name__ == "__main__": await ctx.delay(HP * (2000)) # let W5100 init run await exi_write(ctx, 0x00, [0x08]) # NCRA SR bit → enable RX await ctx.delay(HP * (HALF * 2)) + + # T1b: the device ID STILL ANSWERS after an NCRA reset. + # + # It used to be a one-shot: `id_phase` was cleared by the GC's first + # NCRA reset, after which a command-space read of reg 0 returned + # 0x00000000. Real hardware answers EXI_GetID whenever asked -- that + # is how EXI enumerates devices -- and Swiss calls it from + # bba_exists() AND again from bba_requires_init(), long after init. + # With the one-shot spent, Swiss reported NO ADAPTER AT ALL. + # + # T1 above reads the ID before any reset, so it cannot catch this; + # the reset two lines up is what makes this check meaningful. + dev2 = await exi_devid(ctx) + print(f"T1b device ID after NCRA reset: {[f'0x{b:02X}' for b in dev2]}") + if dev2 != [0x04, 0x02, 0x02, 0x00]: + errors.append(f"T1b device ID after NCRA reset: got {dev2} " + f"- the one-shot is back, Swiss would see no adapter") + await ctx.delay(HP * (HALF * 2)) + ctx.set(dut.w5100_int_n, 0) # W5100: a packet was received await ctx.delay(HP * (4000)) # let the W5100 RX + SPRAM write run ctx.set(dut.w5100_int_n, 1) diff --git a/exi_bba/exi_capture.py b/exi_bba/exi_capture.py index 1989bac..7403976 100644 --- a/exi_bba/exi_capture.py +++ b/exi_bba/exi_capture.py @@ -83,11 +83,6 @@ class ExiCapture(Elaboratable): self.spi_cs_n = Signal(init=1) self.spi_miso = Signal() self.miso_next = Signal(init=1) # for an SB_IO output register - # Register-file id_phase (sync domain). Crossed into exiclk so the - # bit engine can answer the device-ID query itself -- see the fast - # path in SPIMode3Slave. - self.id_phase = Signal(init=1) - # Core-facing RX read side self.rx_data = Signal(8) self.rx_rdy = Signal() @@ -133,12 +128,15 @@ class ExiCapture(Elaboratable): ] spi = SPIMode3Slave(phase_meter=self._phase_meter) - # id_phase is a slow single bit: it starts set and clears once, on the - # GC's first NCRA reset, which happens inside a register-space - # transaction with the clock running -- so it propagates well before - # the next transaction needs it. - m.submodules.id_sync = FFSynchronizer(self.id_phase, spi.id_arm, - o_domain="exiclk", init=1) + # The device-ID fast path is ARMED PERMANENTLY. It used to be gated + # by the register file's `id_phase`, a one-shot cleared on the GC's + # first NCRA reset and crossed in here with an FFSynchronizer -- so + # after init the device stopped identifying itself and Swiss's + # EXI_GetID returned 0x00000000, i.e. "no adapter". Real hardware + # answers EXI_GetID whenever asked. `id_arm` stays a port because + # the PHASE_METER diagnostic build needs to turn the fast path off + # (it would otherwise send ID bytes instead of the constant 0x80). + m.d.comb += spi.id_arm.eq(0 if self._phase_meter else 1) m.submodules.spi = spi # RX bytes cross exiclk -> sync; TX bytes cross sync -> exififo_r. diff --git a/gc_bba_test/source/tests.c b/gc_bba_test/source/tests.c index 4b9479c..ee200d8 100644 --- a/gc_bba_test/source/tests.c +++ b/gc_bba_test/source/tests.c @@ -5,54 +5,31 @@ #include #include -int g_ncra_reset_done = 0; static void msleep(u32 ms) { usleep(ms * 1000); } /* ───────────────────────── suite 1: identity ───────────────────────── */ -/* Is the EXI link demonstrably healthy apart from address 0? +/* The device ID is answerable AT ANY TIME, so these tests no longer skip. * - * Needed because `id_phase` is a ONE-SHOT in the FPGA, cleared by the first - * NCRA reset, and g_ncra_reset_done only tracks resets THIS RUN of the test - * app issued. That flag is software state standing in for hardware state, and - * the two come apart as soon as a build is delivered over the wire: netboot - * replaces the DOL and restarts the app with the flag clear, while the FPGA - * keeps running with id_phase already spent by the previous run. The ID tests - * then query address 0, get NCRA back -- correct behaviour -- and report a - * FAILURE for it. Reflashing the FPGA is the only thing that restores it. + * It used to be a one-shot in the FPGA -- `id_phase`, cleared by the GC's + * first NCRA reset -- so these tests had to guess whether a wrong answer meant + * a spent one-shot or a real fault, and they ran in a fixed order to get a + * reading at all. That was a deviation from real hardware, which answers + * EXI_GetID whenever the GC asks (it is how EXI enumerates devices). The + * deviation broke Swiss outright: it calls EXI_GetID from bba_exists() and + * again from bba_requires_init() long after init, and with the one-shot spent + * got 0x00000000, i.e. NO ADAPTER AT ALL. * - * HIPR and NWAYS are hardwired constants in the register file and do not - * depend on id_phase, so if BOTH read correctly the EXI path is fine and a - * wrong answer at address 0 can only be a spent id_phase. If they are wrong - * too, something real is broken and the test must still fail. */ -static int iface_alive(void) -{ - return bba_r8(REG_HIPR) == HIPR_PRESENT && - bba_r8(REG_NWAYS) == NWAYS_LINKUP; -} - + * With that removed, a wrong ID is simply a FAILURE again -- no skip, no + * iface_alive() discrimination, and no ordering requirement between suites. */ static int t_devid(char *m, size_t n) { u8 b[4]; int rc; - if (g_ncra_reset_done) { - snprintf(m, n, "skipped: id_phase is a one-shot; reflash/reset the FPGA"); - return 2; - } if ((rc = bba_devid(b)) < 0) { snprintf(m, n, "EXI error %d", rc); return 1; } - if (b[0]==0x04 && b[1]==0x02 && b[2]==0x02 && b[3]==0x00) { - snprintf(m, n, "%02X %02X %02X %02X (want 04 02 02 00)", - b[0], b[1], b[2], b[3]); - return 0; - } - if (iface_alive()) { - snprintf(m, n, "%02X %02X %02X %02X: id_phase spent, reflash FPGA", - b[0], b[1], b[2], b[3]); - return 2; - } snprintf(m, n, "%02X %02X %02X %02X (want 04 02 02 00)", b[0], b[1], b[2], b[3]); - return 1; + return (b[0]==0x04 && b[1]==0x02 && b[2]==0x02 && b[3]==0x00) ? 0 : 1; } static int t_hipr(char *m, size_t n) @@ -130,7 +107,6 @@ static int t_ncra_reset(char *m, size_t n) { int i, v; if (bba_w8(REG_NCRA, NCRA_RESET) < 0) { snprintf(m, n, "write failed"); return 1; } - g_ncra_reset_done = 1; for (i = 0; i < 50; i++) { v = bba_r8(REG_NCRA); if (v < 0) { snprintf(m, n, "read failed"); return 1; } @@ -269,14 +245,6 @@ static int t_raw_id(char *m, size_t n) u8 b[8]; int i, at = -1, rc; u16 h = 0x0000; - /* Fast path for a reset this run issued. Note this flag can be STALE - * (see iface_alive): it is clear after a netbooted update even though the - * FPGA's id_phase is long gone, which is why the result is also checked - * against iface_alive() below rather than trusted to this. */ - if (g_ncra_reset_done) { - snprintf(m, n, "skipped: id_phase is a one-shot; reflash/reset the FPGA"); - return 2; - } if ((rc = begin_raw()) < 0) { snprintf(m, n, "EXI error %d", rc); return 1; } EXI_Imm(BBA_EXI_CHAN, &h, 2, EXI_WRITE, NULL); EXI_Sync(BBA_EXI_CHAN); @@ -287,10 +255,7 @@ static int t_raw_id(char *m, size_t n) snprintf(m, n, "%02X %02X %02X %02X %02X %02X %02X %02X | 0x04 %s%d", b[0],b[1],b[2],b[3],b[4],b[5],b[6],b[7], at < 0 ? "absent" : "at slot ", at < 0 ? 0 : at); - if (at == 0) - return 0; - /* Same discrimination as t_devid: a spent id_phase is not a fault. */ - return iface_alive() ? 2 : 1; + return at == 0 ? 0 : 1; } /* ───────────────────────── suite 5: ethernet ───────────────────────── */ @@ -527,7 +492,6 @@ static int t_init_sequence(char *m, size_t n) return 1; } } - g_ncra_reset_done = 1; /* The sentinels must still read correctly afterwards -- if any write in * that list had been misdecoded, the most likely casualty is the state * these two report. */ diff --git a/gc_bba_test/source/tests.h b/gc_bba_test/source/tests.h index 1dc1f2c..c7ed3f6 100644 --- a/gc_bba_test/source/tests.h +++ b/gc_bba_test/source/tests.h @@ -31,7 +31,6 @@ extern const int g_nsuites; * clears id_phase in the gateware, after which address 0 reads back NCRA. * That is correct behaviour (it matches the GC's own init order), but it * means the ID test can only pass once per console power cycle. */ -extern int g_ncra_reset_done; void tests_run_test(test_t *t); void tests_run_suite(suite_t *s);