Remove the device-ID one-shot: it made Swiss see no adapter at all

id_phase was set at reset and cleared permanently by the GC's first NCRA reset,
after which a command-space read of register 0 returned 0x00000000.  Swiss
calls EXI_GetID from bba_exists() AND again from bba_requires_init(), long
after init, and matches the result against EXI_MX98730EC_ID -- so with the
one-shot spent no case matched and Swiss reported NO ADAPTER, not a broken one.
Running gc_bba_test was enough to spend it, so the adapter would "disappear"
after any test session.

The justification in the comment was wrong.  It claimed the fast path would
otherwise hijack address 0 so the GC could never read NCRA back -- but NCRA is
REGISTER space (0x80/0xC0 header) while 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 0x03).
Real hardware answers EXI_GetID whenever asked; that is how EXI enumerates
devices.

id_arm is now tied high, and the id_phase output, the FFSynchronizer that
crossed it into the pin domain, and the BBATop wiring are all gone.  The port
stays only so the PHASE_METER build can disarm the fast path, which would
otherwise send ID bytes instead of its constant 0x80.

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.

gc_bba_test: the ID tests no longer skip.  A wrong ID is a FAILURE again, with
no iface_alive() discrimination and no ordering requirement between suites --
the ID is readable at any point in a run.  g_ncra_reset_done and iface_alive()
are removed outright rather than left as a dead flag.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-10-10 14:33:12 +02:00
parent 991342c0b5
commit 90a583dd46
6 changed files with 104 additions and 99 deletions
+40 -21
View File
@@ -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 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. 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 — (The ID tests simply PASS or FAIL now, in any order, at any point in a run —
reflash the FPGA to restore it. They decide that from the hardware, via see "The device ID is answerable at any time" below. The old SKIP machinery,
`iface_alive()`: HIPR and NWAYS are hardwired and independent of `id_phase`, so `iface_alive()` and `g_ncra_reset_done` are all gone.)
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.)
**Timing closes on EVERY seed (re-measured 2026-10-10).** `python -m **Timing closes on EVERY seed (re-measured 2026-10-10).** `python -m
exi_bba.synth --seeds 8`: 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 constant and belongs to the physical interface, not the register file, and
`byte 0 == 0x00` identifies the query exactly (bit 7 clear = command space, `byte 0 == 0x00` identifies the query exactly (bit 7 clear = command space,
bit 6 clear = read, reg 0). `SPIMode3Slave` recognises it and answers 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 `04 02 02 00` from the EXI clock domain with **no CDC at all** — and since
it needs from `sync` is `id_phase`, a single bit that changes once, crossed 2026-10-10 it needs nothing from `sync` either: `id_arm` is tied high (the
with an `FFSynchronizer`. `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, 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 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 12 V power cycle is required. Check ETH_3V3 and GC_ON (U12 gates the W5100 rail
off GC_ON) before suspecting gateware. off GC_ON) before suspecting gateware.
**`id_phase` is a ONE-SHOT.** The device ID is returned for reads of address 0 ### ✅ The device ID is answerable AT ANY TIME (one-shot removed 2026-10-10)
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. `id_phase` **used to be a one-shot**: set at reset and cleared permanently by
This is correct — it matches the GC's init order — but it means a re-run of any the GC's first NCRA write with the reset bit, after which a command-space read
probe that already issued an NCRA reset will read `00 00 00 00` for the device of register 0 returned `0x00000000`. That is now gone — `id_arm` is tied high
ID and look broken. Reconfigure the FPGA (`iceprog -t` is enough) to restore it. 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: Two bring-up gotchas, both cost real debugging time:
- **The master must sample MISO during the CLK-LOW phase.** The FPGA drives MISO - **The master must sample MISO during the CLK-LOW phase.** The FPGA drives MISO
+23 -16
View File
@@ -107,13 +107,6 @@ class BBARegisterFile(Elaboratable):
# (DMA) bulk reads, not just ≤4-byte immediate transfers. # (DMA) bulk reads, not just ≤4-byte immediate transfers.
self.cs_active = Signal() 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) ─────────────────────────────── # ── Ring-overflow protection (RBFI) ───────────────────────────────
# RRP out to RXFrameAssembler so it can tell how much of the ring is # RRP out to RXFrameAssembler so it can tell how much of the ring is
# still unread and DROP frames instead of overwriting them. # still unread and DROP frames instead of overwriting them.
@@ -350,9 +343,6 @@ class BBARegisterFile(Elaboratable):
# length for any driver that skips TWD. # length for any driver that skips TWD.
txdata_cnt = Signal(16) 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 # Per-byte SPRAM read handshake (register-read path): sp_req marks a
# request in flight; drain_ctr counts the read-phase dummy bytes. # request in flight; drain_ctr counts the read-phase dummy bytes.
sp_req = Signal() sp_req = Signal()
@@ -603,11 +593,29 @@ class BBARegisterFile(Elaboratable):
with m.State("CHDR_1"): with m.State("CHDR_1"):
with m.If(self.rx_rdy): with m.If(self.rx_rdy):
m.d.comb += self.rx_en.eq(1) m.d.comb += self.rx_en.eq(1)
# The device-ID query (command reg 0 while id_phase) is # The device-ID query (command space, read, reg 0) is
# answered by SPIMode3Slave, not here: pushing anything # answered by SPIMode3Slave's fast path, not here: pushing
# for it would leave stale bytes in the tx FIFO that a # anything for it would leave stale bytes in the tx FIFO
# 2-byte header has too few pops to drain. # that a 2-byte header has too few pops to drain.
with m.If(~is_write & (cmd_reg == 0) & id_phase): #
# ⚠️ 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" m.next = "DEVID"
with m.Elif(is_write): with m.Elif(is_write):
m.next = "CMD_WRITE" m.next = "CMD_WRITE"
@@ -661,7 +669,6 @@ class BBARegisterFile(Elaboratable):
with m.If(self.rx_data[0]): with m.If(self.rx_data[0]):
m.d.sync += r_ncra[0].eq(0) # RESET self-clears m.d.sync += r_ncra[0].eq(0) # RESET self-clears
m.d.sync += ncra_rst_pulse.eq(1) m.d.sync += ncra_rst_pulse.eq(1)
m.d.sync += id_phase.eq(0)
# An NCRA reset resets the RX ring, so the # An NCRA reset resets the RX ring, so the
# pointers the GC reads MUST follow -- the # pointers the GC reads MUST follow -- the
# assembler's own write pointer goes back to # assembler's own write pointer goes back to
+19 -1
View File
@@ -190,7 +190,6 @@ class BBATop(Elaboratable):
# EXI outputs # EXI outputs
self.exi_miso.eq(cap.spi_miso), self.exi_miso.eq(cap.spi_miso),
self.exi_miso_next.eq(cap.miso_next), 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 # Ring-overflow protection: the assembler needs the GC's read
# pointer to know what it must not overwrite, and reports a # pointer to know what it must not overwrite, and reports a
# dropped frame back as RBFI (IR[7]). # dropped frame back as RBFI (IR[7]).
@@ -626,6 +625,25 @@ if __name__ == "__main__":
await ctx.delay(HP * (2000)) # let W5100 init run await ctx.delay(HP * (2000)) # let W5100 init run
await exi_write(ctx, 0x00, [0x08]) # NCRA SR bit → enable RX await exi_write(ctx, 0x00, [0x08]) # NCRA SR bit → enable RX
await ctx.delay(HP * (HALF * 2)) 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 ctx.set(dut.w5100_int_n, 0) # W5100: a packet was received
await ctx.delay(HP * (4000)) # let the W5100 RX + SPRAM write run await ctx.delay(HP * (4000)) # let the W5100 RX + SPRAM write run
ctx.set(dut.w5100_int_n, 1) ctx.set(dut.w5100_int_n, 1)
+9 -11
View File
@@ -83,11 +83,6 @@ class ExiCapture(Elaboratable):
self.spi_cs_n = Signal(init=1) self.spi_cs_n = Signal(init=1)
self.spi_miso = Signal() self.spi_miso = Signal()
self.miso_next = Signal(init=1) # for an SB_IO output register 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 # Core-facing RX read side
self.rx_data = Signal(8) self.rx_data = Signal(8)
self.rx_rdy = Signal() self.rx_rdy = Signal()
@@ -133,12 +128,15 @@ class ExiCapture(Elaboratable):
] ]
spi = SPIMode3Slave(phase_meter=self._phase_meter) spi = SPIMode3Slave(phase_meter=self._phase_meter)
# id_phase is a slow single bit: it starts set and clears once, on the # The device-ID fast path is ARMED PERMANENTLY. It used to be gated
# GC's first NCRA reset, which happens inside a register-space # by the register file's `id_phase`, a one-shot cleared on the GC's
# transaction with the clock running -- so it propagates well before # first NCRA reset and crossed in here with an FFSynchronizer -- so
# the next transaction needs it. # after init the device stopped identifying itself and Swiss's
m.submodules.id_sync = FFSynchronizer(self.id_phase, spi.id_arm, # EXI_GetID returned 0x00000000, i.e. "no adapter". Real hardware
o_domain="exiclk", init=1) # 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 m.submodules.spi = spi
# RX bytes cross exiclk -> sync; TX bytes cross sync -> exififo_r. # RX bytes cross exiclk -> sync; TX bytes cross sync -> exififo_r.
+13 -49
View File
@@ -5,54 +5,31 @@
#include <unistd.h> #include <unistd.h>
#include <ogc/lwp_watchdog.h> #include <ogc/lwp_watchdog.h>
int g_ncra_reset_done = 0;
static void msleep(u32 ms) { usleep(ms * 1000); } static void msleep(u32 ms) { usleep(ms * 1000); }
/* ───────────────────────── suite 1: identity ───────────────────────── */ /* ───────────────────────── 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 * It used to be a one-shot in the FPGA -- `id_phase`, cleared by the GC's
* NCRA reset, and g_ncra_reset_done only tracks resets THIS RUN of the test * first NCRA reset -- so these tests had to guess whether a wrong answer meant
* app issued. That flag is software state standing in for hardware state, and * a spent one-shot or a real fault, and they ran in a fixed order to get a
* the two come apart as soon as a build is delivered over the wire: netboot * reading at all. That was a deviation from real hardware, which answers
* replaces the DOL and restarts the app with the flag clear, while the FPGA * EXI_GetID whenever the GC asks (it is how EXI enumerates devices). The
* keeps running with id_phase already spent by the previous run. The ID tests * deviation broke Swiss outright: it calls EXI_GetID from bba_exists() and
* then query address 0, get NCRA back -- correct behaviour -- and report a * again from bba_requires_init() long after init, and with the one-shot spent
* FAILURE for it. Reflashing the FPGA is the only thing that restores it. * got 0x00000000, i.e. NO ADAPTER AT ALL.
* *
* HIPR and NWAYS are hardwired constants in the register file and do not * With that removed, a wrong ID is simply a FAILURE again -- no skip, no
* depend on id_phase, so if BOTH read correctly the EXI path is fine and a * iface_alive() discrimination, and no ordering requirement between suites. */
* 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;
}
static int t_devid(char *m, size_t n) static int t_devid(char *m, size_t n)
{ {
u8 b[4]; u8 b[4];
int rc; 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 ((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]); 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) 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; int i, v;
if (bba_w8(REG_NCRA, NCRA_RESET) < 0) { snprintf(m, n, "write failed"); return 1; } 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++) { for (i = 0; i < 50; i++) {
v = bba_r8(REG_NCRA); v = bba_r8(REG_NCRA);
if (v < 0) { snprintf(m, n, "read failed"); return 1; } 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]; u8 b[8];
int i, at = -1, rc; int i, at = -1, rc;
u16 h = 0x0000; 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; } 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_Imm(BBA_EXI_CHAN, &h, 2, EXI_WRITE, NULL);
EXI_Sync(BBA_EXI_CHAN); 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", 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], 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); at < 0 ? "absent" : "at slot ", at < 0 ? 0 : at);
if (at == 0) return at == 0 ? 0 : 1;
return 0;
/* Same discrimination as t_devid: a spent id_phase is not a fault. */
return iface_alive() ? 2 : 1;
} }
/* ───────────────────────── suite 5: ethernet ───────────────────────── */ /* ───────────────────────── suite 5: ethernet ───────────────────────── */
@@ -527,7 +492,6 @@ static int t_init_sequence(char *m, size_t n)
return 1; return 1;
} }
} }
g_ncra_reset_done = 1;
/* The sentinels must still read correctly afterwards -- if any write in /* The sentinels must still read correctly afterwards -- if any write in
* that list had been misdecoded, the most likely casualty is the state * that list had been misdecoded, the most likely casualty is the state
* these two report. */ * these two report. */
-1
View File
@@ -31,7 +31,6 @@ extern const int g_nsuites;
* clears id_phase in the gateware, after which address 0 reads back NCRA. * 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 * 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. */ * 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_test(test_t *t);
void tests_run_suite(suite_t *s); void tests_run_suite(suite_t *s);