spi_mode3_slave: document the engine, and fix three stale comments
418 -> 439 lines, 31% -> 14% comment-only. The reasoning that matters here is
kept in full, because the edge conventions and the TX launch phase are the two
things in this design that cost silicon bugs; what went was the history around
them and the console measurement logs, which CLAUDE.md already holds.
Three comments described mechanisms that no longer exist:
* `miso_next` was documented as feeding "an SB_IO output register clocked on
the FALLING edge". It is clocked on the RISING edge (NEG_TRIGGER=0) --
that is the whole point of the full-cycle-early launch. The file
contradicted itself, since the comment on the assignment 100 lines later
said "after the next RISING edge" correctly.
* `id_arm` was documented as "the register file's id_phase, synchronised
into this domain by the parent". The one-shot was removed; id_arm is tied
high and the register file has no id_phase. It is now driven low only by
the PHASE_METER build.
* T8's rationale said the gate exists so that "once the GC's first NCRA
reset clears id_phase, address 0 reads back NCRA". That justification was
not merely stale, it was WRONG -- NCRA is register space and the fast path
requires command space, so they were never ambiguous. T8's real job is to
prove a disarmed build passes the FIFO byte through, which is what
PHASE_METER depends on.
Structure: elaborate() now delegates to _rx_bytes, _device_id and _tx_bits,
each carrying the reasoning for its own block -- why rx_byte/rx_valid must be
combinational (a registered pulse loses the last byte of every transaction),
why the device ID is answered here rather than through the FIFOs, and the
output-path budget that forces the early launch.
VERIFIED EQUIVALENT: generated RTLIL is identical for BOTH build variants
(phase_meter False and True), 395 lines each, after stripping \src attributes.
Executable testbench code is unchanged apart from one trailing comment. The
suite still has teeth: mutating the TX load back to rx_bit == 0 (a cycle late)
fails 8 of the 12 checks. Full module suite passes and bba_top T9 is 6/6 at
EXI_HALF=1, 2 and 8.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+186
-165
@@ -1,38 +1,30 @@
|
|||||||
"""SPI Mode 3 byte-oriented slave for the EXI bus — CLOCKED BY EXI CLK.
|
"""SPI Mode 3 byte-oriented slave for the EXI bus, clocked by the EXI clock.
|
||||||
|
|
||||||
CPOL=1, CPHA=1 (real Mode 3): CLK idles HIGH, the master CHANGES MOSI on the
|
Mode 3 is CPOL=1, CPHA=1: CLK idles HIGH, the master changes MOSI on the
|
||||||
falling edge and both ends SAMPLE on the rising edge.
|
FALLING edge, and both ends sample on the RISING edge.
|
||||||
|
|
||||||
Architecture
|
The engine is clocked by the EXI clock pin itself rather than oversampled from
|
||||||
------------
|
a faster domain, so there is exactly one clock edge and one phase reference in
|
||||||
The engine is clocked by the EXI clock itself rather than oversampling it from
|
the whole module:
|
||||||
a faster domain:
|
|
||||||
|
|
||||||
exiclk posedge of EXI CLK — sample MOSI, assemble RX bytes, AND
|
exiclk posedge of EXI CLK. Samples MOSI, assembles RX bytes, and
|
||||||
launch MISO one bit early (see the TX
|
launches MISO -- see `_tx_bits` for why MISO goes out on this
|
||||||
note in elaborate)
|
edge rather than the falling one.
|
||||||
CS_N asynchronous reset
|
spi_cs_n asynchronous reset of that domain.
|
||||||
|
|
||||||
Why, and what it fixes (hardware 2026-10-03)
|
Oversampling cannot work here, which is why it is not an option to fall back
|
||||||
--------------------------------------------
|
to: it puts BIT-level timing across a clock-domain boundary, and the engine's
|
||||||
The previous version oversampled EXI CLK from a 54 MHz `capture` domain, which
|
view of the clock then lags the pin by two `FFSynchronizer` stages plus a
|
||||||
put BIT-level timing across a clock-domain boundary: the engine's view of the
|
register -- about 55 ns against a 37 ns bit time at 27 MHz. Clocking from the
|
||||||
clock lagged the pin by 2 stages of `FFSynchronizer` plus a register, ~3 ticks
|
pin makes MISO clock-to-Q and leaves the byte-level `AsyncFIFO`s as the only
|
||||||
≈ 55 ns. One EXI bit at 27 MHz is 37 ns, so MISO came out ~2 bits late and the
|
crossing, where a whole byte time (~296 ns) of slack is available instead of a
|
||||||
master read `0x17` as `05 C0` (= `0x17 >> 2`). Register reads were correct up
|
fraction of a bit.
|
||||||
to 4 MHz and failed from 8 MHz — the sweep read `ok=[1,1,1,0,0,0]`.
|
|
||||||
|
|
||||||
No amount of oversampling fixes that: at 54 MHz there are only 2 ticks per bit,
|
EXI CLK is on pin 44, which sits at die (6,0) on the 5k-sg48 = global buffer
|
||||||
and the design cannot close the 81 MHz that 3x would need. Clocking the engine
|
input net 6, so it drives a real clock network with no board change.
|
||||||
from the EXI pin makes MISO clock-to-Q instead, and moves the ONLY remaining
|
|
||||||
crossing to the byte-level AsyncFIFOs, which have a whole byte-time (~296 ns at
|
|
||||||
27 MHz) of slack instead of a fraction of a bit.
|
|
||||||
|
|
||||||
EXI CLK (pin 44) sits at die (6,0) on the 5k-sg48, which is global buffer input
|
The clock domain is created by the PARENT (`ExiCapture`) so the byte FIFOs can
|
||||||
net 6 — so it can drive a real clock network with no board change.
|
name it; this module only uses `m.d.exiclk`.
|
||||||
|
|
||||||
The domains are created by the PARENT (`ExiCapture`) so the byte FIFOs can name
|
|
||||||
them; this module only uses `m.d.exiclk`.
|
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from amaranth import *
|
from amaranth import *
|
||||||
@@ -45,43 +37,49 @@ class SPIMode3Slave(Elaboratable):
|
|||||||
|
|
||||||
Ports
|
Ports
|
||||||
-----
|
-----
|
||||||
spi_clk / spi_mosi / spi_cs_n : raw inputs from the GC. spi_clk drives the
|
spi_clk, spi_mosi, spi_cs_n
|
||||||
exiclk domain and spi_cs_n its async reset (both wired by
|
Raw inputs from the GC. `spi_clk` drives the `exiclk` domain and
|
||||||
the parent); they are still listed here so the module is self-describing
|
`spi_cs_n` is its async reset, both wired by the parent; they are
|
||||||
and so spi_cs_n can gate MISO combinationally.
|
listed here so the module is self-describing.
|
||||||
spi_miso : output to the GC; idles HIGH while CS is deasserted.
|
spi_miso
|
||||||
rx_byte : last complete received byte (valid while rx_valid is high)
|
Serial output to the GC. Idles HIGH while CS is deasserted.
|
||||||
rx_valid : 1-cycle pulse in `exiclk` when rx_byte holds a new byte
|
miso_next
|
||||||
tx_byte : byte to transmit; sampled at each byte boundary
|
The same bit, but combinational from the pre-edge state, so it can
|
||||||
tx_load : 1-cycle pulse asking upstream for the next byte, one per byte
|
feed an SB_IO output register clocked on the RISING edge. The
|
||||||
boundary. No header-length or slot-threshold logic: see the TX comment
|
synthesised build drives the pad from that register; see `synth.py`.
|
||||||
in elaborate() for why unconditional pops are both sufficient and
|
id_arm
|
||||||
necessary.
|
Enables the device-ID fast path. Normally tied high -- real hardware
|
||||||
|
answers EXI_GetID whenever it is asked -- and driven low only by the
|
||||||
|
PHASE_METER diagnostic build, so the constant test pattern is not
|
||||||
|
replaced by ID bytes.
|
||||||
|
rx_byte
|
||||||
|
Last complete received byte, valid while `rx_valid` is high.
|
||||||
|
rx_valid
|
||||||
|
One-cycle pulse in `exiclk` when `rx_byte` holds a new byte.
|
||||||
|
tx_byte
|
||||||
|
Byte to transmit, sampled at each byte boundary.
|
||||||
|
tx_load
|
||||||
|
One-cycle pulse per byte boundary asking upstream for the next byte.
|
||||||
|
Unconditional: there is no header-length or slot-threshold logic, and
|
||||||
|
`_tx_bits` explains why that is both sufficient and necessary.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
def __init__(self, phase_meter=False):
|
def __init__(self, phase_meter=False):
|
||||||
# DIAGNOSTIC: when set, every byte the engine transmits is the constant
|
# DIAGNOSTIC build: every transmitted byte becomes the constant 0x80
|
||||||
# 0x80 instead of the FIFO head. The load/shift/mux/output path is
|
# instead of the FIFO head, leaving the load/shift/output path
|
||||||
# otherwise untouched, so the register file and both byte FIFOs drop
|
# untouched. The register file and both byte FIFOs therefore drop out
|
||||||
# out of the picture and what the host reads back is purely the
|
# of the MISO path, and whatever the host reads back is purely the
|
||||||
# engine's byte PHASE: 0x80 = aligned, 0x40 = 1 bit late, 0x20 = 2 bits
|
# engine's byte PHASE -- 0x80 aligned, 0x40 one bit late, 0x20 two
|
||||||
# late, and so on. Not a functional build.
|
# bits late. Not a functional bitstream.
|
||||||
self._phase_meter = phase_meter
|
self._phase_meter = phase_meter
|
||||||
|
|
||||||
self.spi_clk = Signal(init=1) # idles HIGH
|
self.spi_clk = Signal(init=1) # idles HIGH
|
||||||
self.spi_mosi = Signal()
|
self.spi_mosi = Signal()
|
||||||
self.spi_cs_n = Signal(init=1) # active LOW
|
self.spi_cs_n = Signal(init=1) # active LOW
|
||||||
|
|
||||||
self.spi_miso = Signal(init=1)
|
self.spi_miso = Signal(init=1)
|
||||||
# The SAME bit as spi_miso, but combinational from the PRE-edge state,
|
|
||||||
# so it can feed an SB_IO output register clocked on the falling edge.
|
|
||||||
# Driving MISO from a flop inside the IO block removes the fabric
|
|
||||||
# routing between the shift register and the pad, which is what the
|
|
||||||
# 27 MHz GameCube read fails on -- see the note in synth.py.
|
|
||||||
self.miso_next = Signal(init=1)
|
self.miso_next = Signal(init=1)
|
||||||
|
|
||||||
# Asserted while the device-ID query is still answerable (the register
|
|
||||||
# file's id_phase, synchronised into this domain by the parent).
|
|
||||||
self.id_arm = Signal()
|
self.id_arm = Signal()
|
||||||
|
|
||||||
self.rx_byte = Signal(8)
|
self.rx_byte = Signal(8)
|
||||||
@@ -91,40 +89,59 @@ class SPIMode3Slave(Elaboratable):
|
|||||||
|
|
||||||
def elaborate(self, platform):
|
def elaborate(self, platform):
|
||||||
m = Module()
|
m = Module()
|
||||||
|
rx_bit, nxt = self._rx_bytes(m)
|
||||||
|
id_sel, id_byte = self._device_id(m, rx_bit, nxt)
|
||||||
|
self._tx_bits(m, rx_bit, id_sel, id_byte)
|
||||||
|
return m
|
||||||
|
|
||||||
# ── RX: sample MOSI on the RISING edge (Mode 3 samples on edge 2) ──
|
def _rx_bytes(self, m):
|
||||||
|
"""Sample MOSI on the rising edge and assemble bytes, MSB first.
|
||||||
|
|
||||||
|
Returns `(rx_bit, nxt)`, where `nxt` is the shift register's next
|
||||||
|
value -- i.e. the complete byte during the final bit.
|
||||||
|
|
||||||
|
`rx_byte`/`rx_valid` are COMBINATIONAL rather than registered. The
|
||||||
|
clock stops and CS rises immediately after a transaction's final bit,
|
||||||
|
so a registered pulse would need one more clock edge that never
|
||||||
|
arrives, and the LAST byte of every transaction would be silently
|
||||||
|
lost: a 4-byte write landed as DE AD BE 00, and a 34-byte TX stream as
|
||||||
|
15 bytes. Driving them combinationally makes the parent's FIFO
|
||||||
|
capture the byte on the very edge that completes it.
|
||||||
|
"""
|
||||||
rx_shift = Signal(8)
|
rx_shift = Signal(8)
|
||||||
rx_bit = Signal(3)
|
rx_bit = Signal(3)
|
||||||
|
|
||||||
nxt = Cat(self.spi_mosi, rx_shift[:-1]) # MSB-first: shift left, new bit at LSB
|
nxt = Cat(self.spi_mosi, rx_shift[:-1]) # shift left, new bit at LSB
|
||||||
m.d.exiclk += rx_shift.eq(nxt)
|
m.d.exiclk += [rx_shift.eq(nxt),
|
||||||
m.d.exiclk += rx_bit.eq(rx_bit + 1)
|
rx_bit.eq(rx_bit + 1)]
|
||||||
|
|
||||||
# rx_byte/rx_valid are COMBINATIONAL, not registered. The clock stops
|
m.d.comb += [self.rx_byte .eq(nxt),
|
||||||
# and CS rises immediately after a transaction's final bit, so a
|
self.rx_valid.eq(rx_bit == 7)]
|
||||||
# registered pulse would need one more clock edge that never arrives
|
return rx_bit, nxt
|
||||||
# and the LAST byte of every transaction would be silently lost
|
|
||||||
# (symptom: a 4-byte write landed as DE AD BE 00, and a 34-byte TX
|
|
||||||
# stream as 15 bytes). Driving them combinationally makes the FIFO
|
|
||||||
# capture the byte on the very edge that completes it.
|
|
||||||
m.d.comb += self.rx_byte .eq(nxt)
|
|
||||||
m.d.comb += self.rx_valid.eq(rx_bit == 7)
|
|
||||||
|
|
||||||
# ── Device-ID fast path, answered HERE and not by the register file ──
|
def _device_id(self, m, rx_bit, nxt):
|
||||||
# A command-space header is only TWO bytes, so a reply that travels
|
"""Answer the device-ID query from this domain, not the register file.
|
||||||
# rx AsyncFIFO -> sync FSM -> tx AsyncFIFO has one byte time (296 ns
|
|
||||||
# at 27 MHz) to make it back. That round trip costs ~280-320 ns, so
|
Returns `(id_sel, id_byte)` for the TX mux.
|
||||||
# it is marginal by construction, and measured on a console it LOST:
|
|
||||||
# the ID came back one byte late (00 04 02 02). The inter-transfer
|
A command-space header is only TWO bytes, so a reply that travels
|
||||||
# gap cannot rescue it either -- the reply must be in hand by the last
|
rx `AsyncFIFO` → `sync` FSM → tx `AsyncFIFO` has one byte time to make
|
||||||
# rising edge of the header transfer, and the clock is paused after
|
it back -- 296 ns at 27 MHz -- against a round trip costing roughly
|
||||||
# that edge, so nothing can update during the gap.
|
280-320 ns. That is marginal by construction, and on a console it
|
||||||
#
|
lost: the ID came back one byte late.
|
||||||
# The device ID is a CONSTANT and belongs to the physical interface
|
|
||||||
# rather than to the register file, so answer it from this domain and
|
The inter-transfer gap cannot rescue it, which is the non-obvious
|
||||||
# skip both crossings entirely. byte 0 == 0x00 identifies the query
|
part. The reply must be in hand by the LAST rising edge of the header
|
||||||
# exactly: bit 7 clear = command space, bit 6 clear = read, reg = 0.
|
transfer, and the clock is paused after that edge, so no edge exists
|
||||||
byte_idx = Signal(3) # bytes completed this transaction (saturating)
|
during the gap for anything to update.
|
||||||
|
|
||||||
|
The device ID is a constant and belongs to the physical interface
|
||||||
|
rather than to the register file, so answering it here skips both
|
||||||
|
crossings. `byte 0 == 0x00` identifies the query exactly: bit 7 clear
|
||||||
|
selects command space, bit 6 clear selects read, and the register
|
||||||
|
number is 0.
|
||||||
|
"""
|
||||||
|
byte_idx = Signal(3) # bytes completed this transaction, saturating
|
||||||
id_mode = Signal()
|
id_mode = Signal()
|
||||||
with m.If(rx_bit == 7):
|
with m.If(rx_bit == 7):
|
||||||
with m.If(byte_idx != 7):
|
with m.If(byte_idx != 7):
|
||||||
@@ -140,56 +157,54 @@ class SPIMode3Slave(Elaboratable):
|
|||||||
with m.Case(2): m.d.comb += id_byte.eq(0x02)
|
with m.Case(2): m.d.comb += id_byte.eq(0x02)
|
||||||
with m.Case(3): m.d.comb += id_byte.eq(0x02)
|
with m.Case(3): m.d.comb += id_byte.eq(0x02)
|
||||||
with m.Default(): m.d.comb += id_byte.eq(0x00)
|
with m.Default(): m.d.comb += id_byte.eq(0x00)
|
||||||
|
|
||||||
id_sel = Signal()
|
id_sel = Signal()
|
||||||
m.d.comb += id_sel.eq(id_mode & (byte_idx >= 1) & (byte_idx <= 4))
|
m.d.comb += id_sel.eq(id_mode & (byte_idx >= 1) & (byte_idx <= 4))
|
||||||
|
return id_sel, id_byte
|
||||||
|
|
||||||
# ── TX: launch MISO a FULL CYCLE EARLY, on the RISING edge ────────
|
def _tx_bits(self, m, rx_bit, id_sel, id_byte):
|
||||||
# The host samples MISO on the rising edge. The obvious thing is to
|
"""Shift MISO out, launching each bit a FULL CYCLE EARLY.
|
||||||
# drive it on the preceding FALLING edge, which gives the output path
|
|
||||||
# a HALF period -- 18.5 ns at 27 MHz. That is not enough on this
|
The host samples MISO on the rising edge. The obvious thing is to
|
||||||
# part, and the shortfall is irreducible:
|
drive it on the preceding FALLING edge, which gives the output path a
|
||||||
#
|
half period -- 18.5 ns at 27 MHz. That is not enough on this part,
|
||||||
# MISO changes at (falling edge AT THE CONNECTOR)
|
and the shortfall cannot be removed by any RTL change, because of what
|
||||||
# + input pad delay
|
the path is made of:
|
||||||
# + GLOBAL CLOCK NETWORK INSERTION <-- the big one
|
|
||||||
# + IOB clock-to-Q
|
MISO changes at (falling edge AT THE CONNECTOR)
|
||||||
# + output pad delay
|
+ input pad delay
|
||||||
# ... and the host still needs its own setup time on top.
|
+ GLOBAL CLOCK NETWORK INSERTION <- the big one
|
||||||
#
|
+ IOB clock-to-Q
|
||||||
# Measured on a GameCube 2026-10-03 with PHASE_METER=1 (the engine
|
+ output pad delay
|
||||||
# drives a constant 0x80 per byte, so the host reads the engine's byte
|
... and the host still needs its own setup time on top.
|
||||||
# phase directly, with the register file and both byte FIFOs out of the
|
|
||||||
# path). Per-speed dump, first two slots per EXI speed index:
|
About 9 ns was removed from the fabric output path in three steps and
|
||||||
#
|
the console result stayed BYTE IDENTICAL each time. Nine nanoseconds
|
||||||
# 0:8080 1:8080 2:8080 3:8080 4:8080 5:C040
|
out of 18.5 cannot change nothing if propagation were the whole story,
|
||||||
#
|
so the rest is the irreducible part: getting the clock into the chip
|
||||||
# Perfect to 16 MHz, one bit late at 27 MHz. An 8-slot dump at 27 MHz
|
and the data back out.
|
||||||
# read C0 40 40 40 C0 40 40 40 -- a UNIFORM one-bit lag (ones at bit
|
|
||||||
# 1, 9, 17, 25, 33, ...), with extra highs at bits 0 and 32 where
|
So the fix doubles the budget instead of shrinking the delay: present
|
||||||
# libogc splits the read into two 4-byte transfers and MISO tri-states
|
the bit the host samples at rising edge N on rising edge N-1, giving
|
||||||
# to its pull-up in the gap.
|
the whole path a FULL period (37 ns at 27 MHz). The output then
|
||||||
#
|
changes shortly AFTER the edge on which the host sampled the previous
|
||||||
# Crucially, ~9 ns was removed from the fabric output path first (the
|
bit, which is safe at both ends -- the insertion delay that caused the
|
||||||
# EXI_J4 pad merge, then the CS mux, then the whole fabric route by
|
problem now covers the host's hold time, and a full period covers its
|
||||||
# moving the output into an SB_IO register) and the result stayed BYTE
|
setup.
|
||||||
# IDENTICAL. Nine nanoseconds out of an 18.5 ns budget cannot change
|
|
||||||
# nothing if propagation delay were the whole story, so the remaining
|
If a LOW speed index ever regresses while 27 MHz still works, that is
|
||||||
# cost is the part no RTL change can touch: getting the clock in and
|
the HOLD edge of this window (the launch being too early), not a setup
|
||||||
# the data back out of the chip.
|
problem.
|
||||||
#
|
|
||||||
# So stop shrinking the delay and DOUBLE THE BUDGET. Present the bit
|
`tx_out` is therefore clocked on the rising edge and loads at
|
||||||
# the host will sample at rising edge N on rising edge N-1, giving the
|
`rx_bit == 7`, one bit time earlier than a falling-edge shifter
|
||||||
# whole path a FULL period (37 ns at 27 MHz) instead of half. The
|
loading at `rx_bit == 0`. `init=0xFF` parks MISO high under CS reset.
|
||||||
# output then changes shortly AFTER the edge on which the host samples
|
|
||||||
# the previous bit, which is safe in both directions: the insertion
|
`tx_load` pops once per byte boundary unconditionally. That is
|
||||||
# delay that caused the problem now covers the host's hold time, and
|
sufficient because the register file pushes its response during the
|
||||||
# a full period covers its setup.
|
header's trailing pad byte, and necessary because the pops are also
|
||||||
#
|
what drains whatever a previous transaction left in the FIFO.
|
||||||
# `tx_out` is therefore clocked on the RISING edge and loads at
|
"""
|
||||||
# rx_bit == 7 -- one bit-time earlier than a falling-edge shifter
|
|
||||||
# loading at rx_bit == 0. init=0xFF parks MISO high under CS reset.
|
|
||||||
# This also removes `tx_hold` and the whole exiclk_n domain: the
|
|
||||||
# engine now has ONE clock edge and ONE phase reference.
|
|
||||||
tx_out = Signal(8, init=0xFF)
|
tx_out = Signal(8, init=0xFF)
|
||||||
tx_src = Signal(8) # the byte loaded at the next byte boundary
|
tx_src = Signal(8) # the byte loaded at the next byte boundary
|
||||||
m.d.comb += tx_src.eq(
|
m.d.comb += tx_src.eq(
|
||||||
@@ -202,13 +217,11 @@ class SPIMode3Slave(Elaboratable):
|
|||||||
m.d.exiclk += tx_out.eq(Cat(0, tx_out[:-1]))
|
m.d.exiclk += tx_out.eq(Cat(0, tx_out[:-1]))
|
||||||
|
|
||||||
m.d.comb += self.spi_miso.eq(tx_out[7])
|
m.d.comb += self.spi_miso.eq(tx_out[7])
|
||||||
# What tx_out[7] WILL be after the next RISING edge -- this is what the
|
# What tx_out[7] WILL be after the next rising edge, which is what the
|
||||||
# SB_IO output register captures. Mirrors the load/shift choice above
|
# SB_IO output register captures. Mirrors the load/shift choice above
|
||||||
# exactly; T6 proves the two agree bit for bit.
|
# exactly; T6 proves the two agree bit for bit.
|
||||||
m.d.comb += self.miso_next.eq(Mux(rx_bit == 7, tx_src[7], tx_out[6]))
|
m.d.comb += self.miso_next.eq(Mux(rx_bit == 7, tx_src[7], tx_out[6]))
|
||||||
|
|
||||||
return m
|
|
||||||
|
|
||||||
|
|
||||||
# ── Testbench ─────────────────────────────────────────────────────────────
|
# ── Testbench ─────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
@@ -221,7 +234,7 @@ class _Harness(Elaboratable):
|
|||||||
|
|
||||||
def elaborate(self, platform):
|
def elaborate(self, platform):
|
||||||
m = Module()
|
m = Module()
|
||||||
m.domains.exiclk = cd_p = ClockDomain(async_reset=True)
|
m.domains.exiclk = cd_p = ClockDomain(async_reset=True)
|
||||||
d = self.dut
|
d = self.dut
|
||||||
m.d.comb += [
|
m.d.comb += [
|
||||||
cd_p.clk.eq(d.spi_clk), cd_p.rst.eq(d.spi_cs_n),
|
cd_p.clk.eq(d.spi_clk), cd_p.rst.eq(d.spi_cs_n),
|
||||||
@@ -248,11 +261,16 @@ if __name__ == "__main__":
|
|||||||
|
|
||||||
# A STRICT Mode 3 master. Unlike a convenience model that parks MOSI long
|
# A STRICT Mode 3 master. Unlike a convenience model that parks MOSI long
|
||||||
# before the edge, this one changes MOSI exactly ON the falling edge and
|
# before the edge, this one changes MOSI exactly ON the falling edge and
|
||||||
# samples MISO in the CLK-low phase. That is what makes the edge
|
# samples MISO in the CLK-LOW phase. Both details are what make the edge
|
||||||
# convention observable: a slave that sampled MOSI on the falling edge
|
# conventions observable at all:
|
||||||
# would capture the previous bit, and one that drove MISO on the rising
|
#
|
||||||
# edge would come out a bit late. Both bugs shipped to hardware against a
|
# * A slave that sampled MOSI on the falling edge would capture the
|
||||||
# lax master that could not see them.
|
# PREVIOUS bit. A master that parks MOSI early cannot see that, and
|
||||||
|
# a Mode 2/Mode 3 mix-up reached silicon against one that did.
|
||||||
|
# * MISO must be read in the clock-low phase, because the engine drives
|
||||||
|
# it ON the rising edge. Sampling just after raising the clock sees
|
||||||
|
# the NEXT bit in a zero-delay simulation, which would report
|
||||||
|
# everything one bit early while the hardware was correct.
|
||||||
async def spi_byte(ctx, mosi_val, probe_rx=False):
|
async def spi_byte(ctx, mosi_val, probe_rx=False):
|
||||||
miso = 0
|
miso = 0
|
||||||
rx = None
|
rx = None
|
||||||
@@ -284,9 +302,9 @@ if __name__ == "__main__":
|
|||||||
ctx.set(dut.tx_byte, 0)
|
ctx.set(dut.tx_byte, 0)
|
||||||
await ctx.delay(HP * 4)
|
await ctx.delay(HP * 4)
|
||||||
|
|
||||||
# T1 — RX samples MOSI on the RISING edge, MSB first.
|
# T1 — RX samples MOSI on the RISING edge, MSB first. rx_byte and
|
||||||
# rx_byte/rx_valid are combinational and peak during the final bit,
|
# rx_valid are combinational and peak during the final bit, which is
|
||||||
# which is the instant the parent's FIFO captures them.
|
# the instant the parent's FIFO captures them.
|
||||||
await cs_low(ctx)
|
await cs_low(ctx)
|
||||||
seen = []
|
seen = []
|
||||||
for b in (0x80, 0x00, 0x31, 0x00):
|
for b in (0x80, 0x00, 0x31, 0x00):
|
||||||
@@ -297,7 +315,10 @@ if __name__ == "__main__":
|
|||||||
check("T1 rx_valid", [v for v, _ in seen], [1, 1, 1, 1])
|
check("T1 rx_valid", [v for v, _ in seen], [1, 1, 1, 1])
|
||||||
|
|
||||||
# T2 — TX: the byte latched at the end of slot N appears in slot N+1,
|
# T2 — TX: the byte latched at the end of slot N appears in slot N+1,
|
||||||
# MSB first. Slot 0 is the reset value, before any load.
|
# MSB first. Slot 0 is the header byte: the host ignores MISO there
|
||||||
|
# and nothing has been loaded yet, so it reads the CS-reset parking
|
||||||
|
# value (tx_out init=0xFF, which is what holds MISO high while
|
||||||
|
# deselected). The bits the host actually uses start at slot 1.
|
||||||
await cs_low(ctx)
|
await cs_low(ctx)
|
||||||
ctx.set(dut.tx_byte, 0xA5)
|
ctx.set(dut.tx_byte, 0xA5)
|
||||||
s0 = await spi_byte(ctx, 0x00)
|
s0 = await spi_byte(ctx, 0x00)
|
||||||
@@ -306,10 +327,6 @@ if __name__ == "__main__":
|
|||||||
ctx.set(dut.tx_byte, 0x00)
|
ctx.set(dut.tx_byte, 0x00)
|
||||||
s2 = await spi_byte(ctx, 0x00)
|
s2 = await spi_byte(ctx, 0x00)
|
||||||
await cs_high(ctx)
|
await cs_high(ctx)
|
||||||
# Slot 0 is the header byte: the host ignores MISO there and the
|
|
||||||
# engine has not loaded anything yet, so it reads the CS-reset parking
|
|
||||||
# value (tx_out init=0xFF, which is what keeps MISO high while
|
|
||||||
# deselected). The bits the host actually uses start at slot 1.
|
|
||||||
check("T2 miso slot0 (pre-load)", hex(s0), hex(0xFF))
|
check("T2 miso slot0 (pre-load)", hex(s0), hex(0xFF))
|
||||||
check("T2 miso slot1", hex(s1), hex(0xA5))
|
check("T2 miso slot1", hex(s1), hex(0xA5))
|
||||||
check("T2 miso slot2", hex(s2), hex(0x3C))
|
check("T2 miso slot2", hex(s2), hex(0x3C))
|
||||||
@@ -333,16 +350,18 @@ if __name__ == "__main__":
|
|||||||
|
|
||||||
# T5 — a transaction that BEGINS ON A RISING EDGE. The host asserts
|
# T5 — a transaction that BEGINS ON A RISING EDGE. The host asserts
|
||||||
# CS while CLK is already low, so the first edge the device sees is a
|
# CS while CLK is already low, so the first edge the device sees is a
|
||||||
# rise and the transaction carries one fewer FALLING edge than rising.
|
# rise and the transaction carries one fewer falling edge than rising.
|
||||||
# A TX phase counted on falling edges slips one bit here and never
|
# A TX phase counted on falling edges slips one bit here and never
|
||||||
# recovers; a phase taken from rx_bit cannot.
|
# recovers; a phase taken from rx_bit cannot.
|
||||||
#
|
#
|
||||||
# THIS IS THE HARDWARE BUG. On a GameCube the phase-meter build read
|
# THIS IS A HARDWARE BUG THIS TEST EXISTS TO CATCH. With a
|
||||||
# 0x20 (= 0x80 >> 2) in every slot, identically at 1 MHz and 27 MHz.
|
# free-running falling-edge counter the console's phase-meter build
|
||||||
# Nothing in the old testbench started a transaction this way -- every
|
# read 0x20 (= 0x80 >> 2) in every slot, identically at 1 MHz and
|
||||||
# case asserted CS with CLK high -- so the falling and rising counts
|
# 27 MHz -- speed independence being the tell that it was not a
|
||||||
# always matched and the slip was structurally invisible, exactly as
|
# propagation problem. Every earlier test asserted CS with CLK high,
|
||||||
# the Mode 2/3 mix-up was invisible to a master that parked MOSI early.
|
# so falls and rises always matched and the slip was structurally
|
||||||
|
# invisible. When adding SPI tests, vary the edge the transaction
|
||||||
|
# starts on.
|
||||||
ctx.set(dut.spi_clk, 0) # CLK already LOW ...
|
ctx.set(dut.spi_clk, 0) # CLK already LOW ...
|
||||||
ctx.set(dut.spi_cs_n, 0) # ... when CS asserts
|
ctx.set(dut.spi_cs_n, 0) # ... when CS asserts
|
||||||
await ctx.delay(HP)
|
await ctx.delay(HP)
|
||||||
@@ -355,11 +374,11 @@ if __name__ == "__main__":
|
|||||||
check("T5 rising-edge start, slot1", hex(s1), hex(0xA5))
|
check("T5 rising-edge start, slot1", hex(s1), hex(0xA5))
|
||||||
check("T5 rising-edge start, slot2", hex(s2), hex(0x3C))
|
check("T5 rising-edge start, slot2", hex(s2), hex(0x3C))
|
||||||
|
|
||||||
# T6 — miso_next must equal what spi_miso BECOMES on the next RISING
|
# T6 — miso_next must equal what spi_miso BECOMES on the next rising
|
||||||
# edge. The synthesised build drives the pad from an SB_IO register
|
# edge. The synthesised build drives the pad from an SB_IO register
|
||||||
# clocked on that edge (synth.py), so if this ever diverges the
|
# clocked on that edge (synth.py), so if these ever diverge then the
|
||||||
# hardware and the simulation stop agreeing about MISO -- which is
|
# hardware and the simulation stop agreeing about MISO -- precisely
|
||||||
# precisely the class of bug that has cost this design the most time.
|
# the class of bug that has cost this design the most time.
|
||||||
ctx.set(dut.spi_clk, 1)
|
ctx.set(dut.spi_clk, 1)
|
||||||
ctx.set(dut.spi_cs_n, 0)
|
ctx.set(dut.spi_cs_n, 0)
|
||||||
await ctx.delay(HP)
|
await ctx.delay(HP)
|
||||||
@@ -369,7 +388,7 @@ if __name__ == "__main__":
|
|||||||
for _bit in range(8):
|
for _bit in range(8):
|
||||||
ctx.set(dut.spi_clk, 0)
|
ctx.set(dut.spi_clk, 0)
|
||||||
await ctx.delay(HP)
|
await ctx.delay(HP)
|
||||||
predicted = ctx.get(dut.miso_next) # before the RISING edge
|
predicted = ctx.get(dut.miso_next) # before the rising edge
|
||||||
ctx.set(dut.spi_clk, 1)
|
ctx.set(dut.spi_clk, 1)
|
||||||
await ctx.delay(HP)
|
await ctx.delay(HP)
|
||||||
if ctx.get(dut.spi_miso) != predicted:
|
if ctx.get(dut.spi_miso) != predicted:
|
||||||
@@ -379,9 +398,10 @@ if __name__ == "__main__":
|
|||||||
|
|
||||||
# T7 — the device-ID fast path. A command-space query (byte 0 = 0x00,
|
# T7 — the device-ID fast path. A command-space query (byte 0 = 0x00,
|
||||||
# byte 1 = 0x00) must be answered 04 02 02 00 straight from this
|
# byte 1 = 0x00) must be answered 04 02 02 00 straight from this
|
||||||
# domain, with the tx FIFO empty: the register file cannot make the
|
# domain, with the tx FIFO deliberately feeding 0x00 so that only the
|
||||||
# round trip inside a 2-byte header's one byte time, which on a
|
# fast path can be the source. The register file cannot make the
|
||||||
# console returned the ID one byte late (00 04 02 02).
|
# round trip inside a 2-byte header's one byte time; on a console it
|
||||||
|
# returned the ID one byte late.
|
||||||
ctx.set(dut.id_arm, 1)
|
ctx.set(dut.id_arm, 1)
|
||||||
ctx.set(dut.tx_byte, 0x00) # FIFO deliberately contributes nothing
|
ctx.set(dut.tx_byte, 0x00) # FIFO deliberately contributes nothing
|
||||||
await cs_low(ctx)
|
await cs_low(ctx)
|
||||||
@@ -392,9 +412,10 @@ if __name__ == "__main__":
|
|||||||
check("T7 device ID", [hex(b) for b in got],
|
check("T7 device ID", [hex(b) for b in got],
|
||||||
[hex(x) for x in (0x04, 0x02, 0x02, 0x00)])
|
[hex(x) for x in (0x04, 0x02, 0x02, 0x00)])
|
||||||
|
|
||||||
# T8 — and it must be GATED. Once the GC's first NCRA reset clears
|
# T8 — and the fast path must be GATED by id_arm, so that a build
|
||||||
# id_phase, address 0 reads back NCRA, so the fast path must stop
|
# which disarms it gets the FIFO byte through untouched. That is what
|
||||||
# hijacking the reply and let the FIFO through.
|
# the PHASE_METER bitstream relies on: its constant 0x80 pattern would
|
||||||
|
# otherwise be replaced by ID bytes in the slots it measures.
|
||||||
ctx.set(dut.id_arm, 0)
|
ctx.set(dut.id_arm, 0)
|
||||||
ctx.set(dut.tx_byte, 0x5A)
|
ctx.set(dut.tx_byte, 0x5A)
|
||||||
await cs_low(ctx)
|
await cs_low(ctx)
|
||||||
|
|||||||
Reference in New Issue
Block a user