From ca0f7313fdb9cedeb39ce0cef9f845f0a5e24346 Mon Sep 17 00:00:00 2001 From: Nathanael Huffman Date: Tue, 4 Aug 2026 15:07:00 -0400 Subject: [PATCH 1/4] spi-nor: run sclk at 62.5MHz Takes the flash clock from 20.83MHz (clk/6) to 62.5MHz (clk/2) on cosmo_seq and grapefruit. Every SP5 boot pulls its host image through this block over eSPI, so sclk sits directly on the boot path. clk/2 is the ceiling for this structure. The read sample point is fixed rather than trained, so the round trip out to the part and back has to land within half an sclk period of it; above this rate that window closes and it would take per-lane IDELAY read training to go further. Speed and sample point are generics on spi_nor_top so a project can dial back to clk/4 with one line. The register map is untouched, so hubris needs no change. Simply changing the divisor would not have worked. Three latent bugs were masked by clk/6 leaving spare clk cycles between sclk edges: - get_cur_io_mode reported the transaction's data mode during cs_assert, but sclk is already running by then. At clk/2 the first sclk edge lands inside cs_assert, so a quad read shifted the opcode out four bits at a time and the part decoded 0x60 instead of 0x6C. - io_oe followed the byte-aligned cur_io_mode, which still reads single for the first cycles of a dual or quad read -- long enough to re-enable io3 for HOLD avoidance exactly as the part takes the lane over. Direction now tracks the transaction directly, and release_lanes drops the lanes the part is about to drive an sclk cycle early. - Tearing down io_o and io_oe at the end of a write landed on the sclk edge the part samples the last bit on, because both are registered off a phase that changes on the falling edge. cs_deassert now counts as driving for anything that is not a read. Separately, the eSPI reader chains page reads and re-asserted cs_n the cycle after it rose: 8 to 16ns against a tSHSL of 30ns. That was already out of spec at 20MHz and is now enforced by cs_high_cnts. The transmit path also had to stop launching a cycle late. It shifted on an edge detector's view of the sclk falling edge, spending a whole clk period of the half-period budget, which caps sclk at clk/4 on its own. Both the shifter and a registered io_o now move on the same clk edge that drives sclk low. Read data is sampled at rx_sample_taps half-clks after the sclk rising edge, selected from a rising and a falling edge capture flop so the point can be placed on a 4ns grid without a faster clock. The sample pulse is qualified by in_rx_phases where it is generated, not where it is consumed, or the last dummy clock's pulse arrives after the phase has advanced and steals a sample. Getting this to close in Vivado needed the flash IO flops in the IOBs. Left in the fabric the placer put them 12 to 13ns of routing from their pins, varying by several ns between builds, which both blew the clock-to-data skew budget and pushed the round trip past every available sample point. sclk and cs_n cannot pack because they have internal fanout, so each gained a dedicated duplicate flop driven from the same next-state value: same edge, same value, no internal load. Flop to pin is now 3.3ns with 0.001ns of routing, and both projects meet timing with no failing endpoints. The old testbench pulled the bus to 'H' and checked nothing, so it could not have caught any of the above. It now drives a W25Q01JV target model that applies real tCLQV and tCLQX to read data and checks mosi setup and hold plus the chip select timing, and the harness models the FPGA's own flop-to-pin and pin-to-flop delays -- without those, simulation validates a regime that does not exist on hardware. Three testbenches cover the legacy configuration and the shipped fast one at both IO corners, and a margin test sweeps the round trip across the range the constraints allow. Still open: the board trace delays in both XDCs are placeholders pending measured lengths, and this wants a JEDEC ID read, a full image readback and a timed SP5 boot on real hardware. If reads come back marginal, sweep rx_sample_taps before suspecting anything else. --- hdl/ip/vhd/spi_nor_controller/BUCK | 1 + .../spi_nor_controller/link/spi_clk_gen.vhd | 37 +- .../vhd/spi_nor_controller/link/spi_link.vhd | 287 ++++++--- .../sims/spi_nor_fast_quick_io_tb.vhd | 91 +++ .../sims/spi_nor_fast_tb.vhd | 106 ++++ .../spi_nor_controller/sims/spi_nor_tb.vhd | 23 +- .../sims/spi_nor_tb_pkg.vhd | 221 +++++++ .../spi_nor_controller/sims/spi_nor_th.vhd | 71 ++- hdl/ip/vhd/spi_nor_controller/spi_nor_top.vhd | 49 +- .../spi_txn/spi_txn_mgr.vhd | 143 ++++- hdl/ip/vhd/vunit_components/BUCK | 7 + .../spi_nor_target/spi_nor_target_vc.vhd | 575 ++++++++++++++++++ .../spi_nor_target/spi_nor_target_vc_pkg.vhd | 227 +++++++ hdl/projects/cosmo_seq/cosmo_seq_pins.xdc | 2 +- hdl/projects/cosmo_seq/cosmo_timing.xdc | 114 +++- .../sp5_espi_flash_subsystem.vhd | 15 + hdl/projects/grapefruit/grapefruit_pins.xdc | 12 +- hdl/projects/grapefruit/grapefruit_timing.xdc | 32 + hdl/projects/grapefruit/grapefruit_top.vhd | 6 + 19 files changed, 1870 insertions(+), 149 deletions(-) create mode 100644 hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_quick_io_tb.vhd create mode 100644 hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_tb.vhd create mode 100644 hdl/ip/vhd/vunit_components/spi_nor_target/spi_nor_target_vc.vhd create mode 100644 hdl/ip/vhd/vunit_components/spi_nor_target/spi_nor_target_vc_pkg.vhd diff --git a/hdl/ip/vhd/spi_nor_controller/BUCK b/hdl/ip/vhd/spi_nor_controller/BUCK index 388feaca..c8b457bd 100644 --- a/hdl/ip/vhd/spi_nor_controller/BUCK +++ b/hdl/ip/vhd/spi_nor_controller/BUCK @@ -37,6 +37,7 @@ vunit_sim( ":spi_nor_top", "//hdl/ip/vhd/axi_blocks:axil_interconnect", "//hdl/ip/vhd/axi_blocks:axil8_resizer", + "//hdl/ip/vhd/vunit_components:spi_nor_target_vc", ], visibility = ["PUBLIC"], ) \ No newline at end of file diff --git a/hdl/ip/vhd/spi_nor_controller/link/spi_clk_gen.vhd b/hdl/ip/vhd/spi_nor_controller/link/spi_clk_gen.vhd index 3c262712..6b0e6698 100644 --- a/hdl/ip/vhd/spi_nor_controller/link/spi_clk_gen.vhd +++ b/hdl/ip/vhd/spi_nor_controller/link/spi_clk_gen.vhd @@ -13,7 +13,21 @@ entity spi_clk_gen is reset : in std_logic; divisor : in unsigned(15 downto 0); enable : in boolean; - sclk : out std_logic + -- For internal consumers: edge detection, phase counting, debug + sclk : out std_logic; + -- A second copy of the same flop, for the pin and nothing else. Both + -- change on the same clk edge with the same value, so this costs no + -- latency, but having no internal fanout is what lets it be packed into + -- the IOB. Without that the launch flop lands wherever the placer likes, + -- which measured 12ns of routing to the pin and put the read round trip + -- outside every available sample point. + sclk_pin : out std_logic; + -- True during the cycle whose clk edge will drive sclk low. Consumers + -- use this to move data on the *same* edge sclk moves, rather than a + -- cycle later after an edge detector has seen it. That distinction is + -- what sets the maximum sclk rate: a cycle-late launch has to fit inside + -- a half period, so it caps sclk at clk/4 rather than clk/2. + sclk_fall_now : out boolean ); end entity; @@ -23,9 +37,17 @@ architecture rtl of spi_clk_gen is signal strobe : boolean := false; signal internal_enable : boolean := false; signal enable_last : boolean := false; + signal sclk_int : std_logic := '0'; begin + sclk <= sclk_int; + + -- strobe is registered, so it is already asserted during the cycle that + -- precedes the toggling edge. That makes this safe to use as a synchronous + -- enable by anything that needs to change state exactly when sclk does. + sclk_fall_now <= internal_enable and strobe and sclk_int = '1'; + -- Pretty simple spi generator. -- start with a rising edge -- generate requested clock @@ -53,7 +75,8 @@ begin variable nxt_sclk : std_logic; begin if reset then - sclk <= '0'; + sclk_int <= '0'; + sclk_pin <= '0'; internal_enable <= false; enable_last <= false; elsif rising_edge(clk) then @@ -65,13 +88,15 @@ begin end if; if internal_enable then - nxt_sclk := sclk; + nxt_sclk := sclk_int; if strobe then - nxt_sclk := not sclk; + nxt_sclk := not sclk_int; end if; - sclk <= nxt_sclk; -- assign value to output + sclk_int <= nxt_sclk; -- assign value to output + sclk_pin <= nxt_sclk; -- IOB-resident duplicate, same edge else - sclk <= '0'; + sclk_int <= '0'; + sclk_pin <= '0'; end if; end if; end process; diff --git a/hdl/ip/vhd/spi_nor_controller/link/spi_link.vhd b/hdl/ip/vhd/spi_nor_controller/link/spi_link.vhd index aa7d0479..173a1cc0 100644 --- a/hdl/ip/vhd/spi_nor_controller/link/spi_link.vhd +++ b/hdl/ip/vhd/spi_nor_controller/link/spi_link.vhd @@ -9,6 +9,15 @@ use ieee.numeric_std_unsigned.all; use work.spi_nor_pkg.all; entity spi_link is + generic ( + -- Where to sample read data, in half-clk steps after the sclk rising + -- edge. The round trip (clk to sclk pin, flash tCLQV, data back to the + -- pin, pin to flop) does not scale with sclk, so at higher rates the + -- sample point has to be placed deliberately rather than left at + -- whatever the edge detector happens to produce. 2 reproduces the + -- original one-clk-after-the-edge behaviour. + rx_sample_taps : natural range 0 to 4 := 2 + ); port ( clk : in std_logic; reset : in std_logic; @@ -17,16 +26,27 @@ entity spi_link is divisor : in unsigned(15 downto 0); in_tx_phases : in boolean; in_rx_phases : in boolean; + -- Lanes to stop driving early, ahead of a controller-to-flash + -- turnaround, so the two ends are never enabled at once + release_lanes : in std_logic_vector(3 downto 0); rx_byte : out std_logic_vector(7 downto 0); rx_byte_done : out boolean; tx_byte : in std_logic_vector(7 downto 0); - tx_byte_done : out boolean; - sclk_redge : out boolean; - sclk_fedge : out boolean; + -- Asserted the cycle before the edge on which a new tx byte is + -- consumed. The transaction manager uses this both to advance its phase + -- and to present the next byte, so that the byte and the sclk edge that + -- launches it move together. + tx_byte_req : out boolean; + sclk_redge : out boolean; + sclk_fedge : out boolean; -- qspi interface cs_n : in std_logic; - sclk : out std_logic; + -- sclk as the internal logic sees it, for the transaction manager's + -- phase counting + sclk : out std_logic; + -- sclk for the pin only, so it can live in the IOB. See spi_clk_gen. + sclk_pin : out std_logic; io : in std_logic_vector(3 downto 0); io_o : out std_logic_vector(3 downto 0); io_oe : out std_logic_vector(3 downto 0) @@ -37,26 +57,77 @@ architecture rtl of spi_link is attribute mark_debug : string; + constant SENTINEL_AT_TOP : std_logic_vector(8 downto 0) := "100000000"; + + -- rx_sample_taps counts half-clks, but a capture flop only lets us pick a + -- source (rising or falling phase) and how many whole clks later to use it. + -- Odd taps come from the falling-edge capture, and the whole-clk part of + -- the delay is the number of pipeline stages the sample pulse walks. + constant SAMPLE_DELAY : natural := (rx_sample_taps + 1) / 2; + constant SAMPLE_ON_NEG : boolean := (rx_sample_taps mod 2) = 1; + signal tx_reg : std_logic_vector(8 downto 0); signal rx_reg : std_logic_vector(8 downto 0); attribute mark_debug of tx_reg : signal is "TRUE"; attribute mark_debug of rx_reg : signal is "TRUE"; - signal tx_byte_ack : boolean; signal sclk_last : std_logic; + signal sclk_int : std_logic; + signal sclk_fall_now : boolean; signal shift_amt : integer range 1 to 4; signal csn_last : std_logic; - signal is_last_bit : boolean; signal cur_io_mode : io_mode; signal dbg_sclk_cnts : unsigned(31 downto 0); attribute mark_debug of dbg_sclk_cnts : signal is "TRUE"; -begin + -- Dedicated input capture. These are the only loads on the io port pins, + -- which is what makes the pin-to-flop delay bound in the XDC meaningful. + signal io_cap_p : std_logic_vector(3 downto 0); + signal io_cap_n : std_logic_vector(3 downto 0); + signal io_n_q : std_logic_vector(3 downto 0); + signal io_tap : std_logic_vector(3 downto 0); + + -- Sample pulse pipeline. Stage 0 is combinational and fires on the clk edge + -- immediately after the sclk rising edge; each further stage is one clk + -- later. + signal sample_pipe : std_logic_vector(2 downto 0); + signal sample_now : boolean; - shift_amt <= 1 when cur_io_mode = SINGLE else - 2 when cur_io_mode = DUAL else - 4 when cur_io_mode = QUAD else - 1; + -- Resolve the shifter into the four io lanes for the current mode. Lane 3 + -- carries the most significant bit in quad, lane 1 in dual, and lanes 1/3 + -- have to sit high otherwise so the part doesn't see a HOLD request. + function io_out_bits ( + reg : std_logic_vector(8 downto 0); + mode : io_mode + ) return std_logic_vector is + begin + case mode is + when QUAD => + return reg(8 downto 5); + when DUAL => + return '1' & reg(7) & reg(8) & reg(7); + when SINGLE => + return '1' & reg(7) & '1' & reg(8); + end case; + end function; + + function shift_amt_of ( + mode : io_mode + ) return integer is + begin + case mode is + when QUAD => + return 4; + when DUAL => + return 2; + when SINGLE => + return 1; + end case; + end function; + +begin + sclk <= sclk_int; + shift_amt <= shift_amt_of(cur_io_mode); rx_byte_done <= rx_reg(rx_reg'high) = '1'; rx_byte <= rx_reg(7 downto 0); @@ -64,10 +135,9 @@ begin begin if reset then sclk_last <= '0'; - cur_io_mode <= single; dbg_sclk_cnts <= (others => '0'); elsif rising_edge(clk) then - sclk_last <= sclk; + sclk_last <= sclk_int; -- This is a simple sclk counter that is used for debugging -- purposes. I can be tricky to figure out where in the -- transaction you are, when using the ila, so this counter @@ -78,28 +148,22 @@ begin elsif cs_n = '1' then dbg_sclk_cnts <= (others => '0'); end if; - - if cs_n = '0' and sclk_fedge then - -- only update the io mode on the falling edge - -- so that we're done with the current bit - cur_io_mode <= req_io_mode; - elsif cs_n = '1' then - cur_io_mode <= req_io_mode; - end if; end if; end process; - sclk_redge <= sclk = '1' and sclk_last = '0'; - sclk_fedge <= sclk = '0' and sclk_last = '1'; + sclk_redge <= sclk_int = '1' and sclk_last = '0'; + sclk_fedge <= sclk_int = '0' and sclk_last = '1'; -- spi clock gen block clk_gen: entity work.spi_clk_gen port map ( - clk => clk, - reset => reset, - divisor => divisor, - enable => in_tx_phases or in_rx_phases, - sclk => sclk + clk => clk, + reset => reset, + divisor => divisor, + enable => in_tx_phases or in_rx_phases, + sclk => sclk_int, + sclk_pin => sclk_pin, + sclk_fall_now => sclk_fall_now ); -- This is the main "output" serializer. The internal @@ -108,115 +172,157 @@ begin -- We know we're done with a byte when the MSB is '1' -- and all the other bits are '0' b/c we've shifted the -- sentinel up 8x - -- There are two key indications that we need and we need them - -- at different times: - -- 1) We need to ack bytes from fifos/other data inputs. This happens - -- *before* the byte-shifting is done. - -- 2) We need to know when the last bit of the byte has been shifted + -- + -- Both the shifter and the io_o output register move on the same clk edge + -- that drives the sclk falling edge, so mosi and sclk leave the FPGA + -- together and the only skew left for the flash's setup time to absorb is + -- the IOB and routing difference between the pins. Doing this a cycle later + -- off an edge detector, as this used to, spends a whole clk period of the + -- half-period budget and caps sclk at clk/4. serializer: process(clk, reset) variable cs_n_assert_edge : boolean := false; + variable nxt_tx_reg : std_logic_vector(8 downto 0); + variable nxt_mode : io_mode; begin if reset then tx_reg <= (others => '0'); - tx_byte_ack <= false; csn_last <= '1'; - is_last_bit <= false; + cur_io_mode <= single; + io_o <= (others => '1'); elsif rising_edge(clk) then csn_last <= cs_n; cs_n_assert_edge := cs_n = '0' and csn_last = '1'; - -- clear single-cycle flags - tx_byte_ack <= false; - is_last_bit <= false; + + -- The io mode only changes on a byte boundary, which is also when + -- the shifter reloads, so the mode that applies to the bits going + -- out at this edge is the one selected here. + nxt_mode := cur_io_mode; + if (cs_n = '0' and sclk_fall_now) or cs_n = '1' then + nxt_mode := req_io_mode; + end if; + cur_io_mode <= nxt_mode; + + nxt_tx_reg := tx_reg; if cs_n_assert_edge then -- as the controller here, we need to pre-load data before the first -- clock - tx_reg(8 downto 1) <= tx_byte; - tx_reg(tx_reg'low) <= '1'; - tx_byte_ack <= true; - is_last_bit <= false; - - -- Main serializer logic, shift out on sclk_fedge - -- when we're chip-selected and not doing turnaround - elsif in_tx_phases and sclk_redge then - if shift_left(tx_reg, shift_amt) = "100000000" then - is_last_bit <= true; - end if; - elsif in_tx_phases and sclk_fedge then - -- if next-shift would be our sentinal value, load new data - if shift_left(tx_reg, shift_amt) = "100000000" then + nxt_tx_reg := tx_byte & '1'; + elsif in_tx_phases and sclk_fall_now then + if shift_left(tx_reg, shift_amt) = SENTINEL_AT_TOP then -- tx_register is "empty" load a new one - -- and the sentinal value - tx_reg(8 downto 1) <= tx_byte; - tx_reg(tx_reg'low) <= '1'; - tx_byte_ack <= true; - -- mid-byte, shift + -- and the sentinel value + nxt_tx_reg := tx_byte & '1'; else - tx_reg <= shift_left(tx_reg, shift_amt); + nxt_tx_reg := shift_left(tx_reg, shift_amt); end if; elsif not in_tx_phases then - tx_reg <= (others => '0'); - is_last_bit <= false; + nxt_tx_reg := (others => '0'); end if; + + tx_reg <= nxt_tx_reg; + io_o <= io_out_bits(nxt_tx_reg, nxt_mode); end if; end process; - tx_byte_done <= is_last_bit; + + -- Flag the reload a cycle ahead of the edge that consumes the byte so the + -- transaction manager can present the right one. This is the same condition + -- the serializer uses above, just not yet registered. + tx_byte_req <= in_tx_phases and sclk_fall_now and + shift_left(tx_reg, shift_amt) = SENTINEL_AT_TOP; -- Based on state and qspi mode, deal with the tri-state controls -- of the spi pins + -- Note this tracks req_io_mode, not the byte-aligned cur_io_mode. The + -- latched mode is deliberately held until a falling edge so a byte is never + -- split across two widths, but that leaves it reading `single` for the first + -- cycles of a dual/quad read -- long enough to re-enable io3 for HOLD + -- avoidance just as the part starts driving it. Direction has no reason to + -- wait for a byte boundary, so it follows the transaction directly. oe_control: process(clk, reset) + variable oe : std_logic_vector(3 downto 0); begin if reset then io_oe <= (others => '0'); elsif rising_edge(clk) then if in_tx_phases then - case cur_io_mode is + case req_io_mode is when single => -- data going out 0 port, but need 3 port to be high so -- chip doesn't see a HOLD operation - io_oe <= (0 => '1', 3 => '1', others => '0'); + oe := (0 => '1', 3 => '1', others => '0'); when dual => -- data going out 0 port, but need 3 port to be high so -- chip doesn't see a HOLD operation - io_oe <= (1 downto 0 => '1', 3 => '1', others => '0'); + oe := (1 downto 0 => '1', 3 => '1', others => '0'); when quad => - io_oe <= (others => '1'); + oe := (others => '1'); end case; else -- rx only in all rx phases - case cur_io_mode is + case req_io_mode is when single => -- data coming in 1 port, but need 3 port to be high so -- chip doesn't see a HOLD operation - io_oe <= (3 => '1', others => '0'); + oe := (3 => '1', others => '0'); when dual => -- data coming in 0, 1 ports, but need 3 port to be high so -- chip doesn't see a HOLD operation - io_oe <= ( 3 => '1', others => '0'); + oe := ( 3 => '1', others => '0'); when quad => -- data coming in all ports, no outputs - io_oe <= (others => '0'); + oe := (others => '0'); end case; end if; + + io_oe <= oe and not release_lanes; + end if; + end process; + + -- Input capture. Two flops, one per clk phase, so the sample point can be + -- placed on a half-clk grid without needing a faster clock. + capture_p: process(clk) + begin + if rising_edge(clk) then + io_cap_p <= io; + io_n_q <= io_cap_n; end if; end process; - -- Deal with output logic for the different modes - io_o(0) <= tx_reg(tx_reg'high-3) when cur_io_mode = QUAD else - tx_reg(tx_reg'high-1) when cur_io_mode = DUAL else - tx_reg(tx_reg'high); + capture_n: process(clk) + begin + if falling_edge(clk) then + io_cap_n <= io; + end if; + end process; - io_o(1) <= tx_reg(tx_reg'high) when cur_io_mode = DUAL else - tx_reg(tx_reg'high-2) when cur_io_mode = QUAD else - '1'; -- not used due to oe-gate - io_o(2) <= tx_reg(tx_reg'high - 1); -- only used in quad mode - io_o(3) <= tx_reg(tx_reg'high) when cur_io_mode = QUAD else - '1'; -- only used in quad mode, but need to not be 0 in - -- single or dual modes to not act as a HOLD + io_tap <= io_n_q when SAMPLE_ON_NEG else io_cap_p; + + -- Walk the sample pulse out to the requested whole-clk delay. + -- + -- in_rx_phases is sampled here, when the pulse is injected, rather than + -- where it is consumed. The phase advances on the clk edge right after the + -- sclk edge, so a pulse still in the pipeline would otherwise be validated + -- against a phase that has already moved on: the last dummy clock's pulse + -- lands once rdata is active and steals a sample, shifting every subsequent + -- byte by one group. + sample_pipe(0) <= '1' when sclk_redge and in_rx_phases else '0'; + + sample_pipeline: process(clk, reset) + begin + if reset then + sample_pipe(2 downto 1) <= (others => '0'); + elsif rising_edge(clk) then + sample_pipe(1) <= sample_pipe(0); + sample_pipe(2) <= sample_pipe(1); + end if; + end process; + + sample_now <= sample_pipe(SAMPLE_DELAY) = '1'; -- This is the main "input" deserializer. The internal -- register is 9 bits wide using a sentinel value in the -- LSB so that we don't need bit counters here. - -- We know we're done with a byte when the MSB is '1' + -- We know we're done shifting when the MSB is '1' -- This bit can also function as the valid flag deserializer: process(clk, reset) begin @@ -228,21 +334,22 @@ begin rx_reg <= (rx_reg'low => '1', others => '0'); elsif rising_edge(clk) then -- Do the sample/shift when requested and flag the - -- valid bytes once we have them - if in_rx_phases and sclk_redge then + -- valid bytes once we have them. sample_now is already qualified + -- by in_rx_phases at the point the pulse was generated. + if sample_now then -- Shift data by amount depending on mode rx_reg <= shift_left(rx_reg, shift_amt); -- Sample new data into vacated locations if cur_io_mode = SINGLE then - rx_reg(0) <= io(1); + rx_reg(0) <= io_tap(1); elsif cur_io_mode = DUAL then - rx_reg(0) <= io(0); - rx_reg(1) <= io(1); + rx_reg(0) <= io_tap(0); + rx_reg(1) <= io_tap(1); elsif cur_io_mode = QUAD then - rx_reg(0) <= io(0); - rx_reg(1) <= io(1); - rx_reg(2) <= io(2); - rx_reg(3) <= io(3); + rx_reg(0) <= io_tap(0); + rx_reg(1) <= io_tap(1); + rx_reg(2) <= io_tap(2); + rx_reg(3) <= io_tap(3); end if; elsif not in_rx_phases or rx_reg(rx_reg'high) = '1' then -- Reset shifter to sentinel value when we become diff --git a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_quick_io_tb.vhd b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_quick_io_tb.vhd new file mode 100644 index 00000000..fafe9b02 --- /dev/null +++ b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_quick_io_tb.vhd @@ -0,0 +1,91 @@ +-- This Source Code Form is subject to the terms of the Mozilla Public +-- License, v. 2.0. If a copy of the MPL was not distributed with this +-- file, You can obtain one at https://mozilla.org/MPL/2.0/. +-- +-- The shipped fast configuration at the *quick* IO corner. +-- +-- spi_nor_fast_tb runs the same RTL configuration with the slow-corner FPGA IO +-- delays, which is the corner that decides whether the sample point is late +-- enough. This one runs the fast corner, which decides whether it is early +-- enough: the sample point must land no later than half an sclk period past the +-- point the part stops holding the previous bit (tCLQX), and at the fast corner +-- everything arrives sooner, so that limit tightens. +-- +-- Both corners have to pass for rx_sample_taps to be the right choice, and at +-- 62.5MHz the margin at this corner is well under a nanosecond. That is the +-- headline number for anyone considering pushing sclk higher, or deciding +-- whether to fall back to sclk_divisor = 1. + +library ieee; +use ieee.std_logic_1164.all; +use ieee.numeric_std.all; + +library vunit_lib; + context vunit_lib.com_context; + context vunit_lib.vunit_context; +use work.spi_nor_tb_pkg.all; +use work.spi_nor_pkg.all; +use work.spi_nor_target_vc_pkg.all; + +entity spi_nor_fast_quick_io_tb is + generic ( + runner_cfg : string + ); +end entity; + +architecture tb of spi_nor_fast_quick_io_tb is + +begin + + th: entity work.spi_nor_th + generic map ( + sclk_divisor => 0, + rx_sample_taps => 2, + -- Fast corner of the delays the XDC bounds + out_delay => 1.6 ns, + in_delay => 0.5 ns + ); + + bench: process + alias reset is <>; + alias cs_n is <>; + constant flash : actor_t := find("spi_nor_target"); + begin + test_runner_setup(runner, runner_cfg); + + wait until reset = '0'; + wait for 500 ns; + + fill_pattern(net, flash); + + while test_suite loop + if run("read_quad_32addr_dummy") then + check_flash_read(net, FAST_READ_4BYTE_QUAD_OP, 8, 16#1200#, 64); + elsif run("read_dual_32addr_dummy") then + check_flash_read(net, FAST_READ_4BYTE_DUAL_OP, 8, 16#80#, 16); + elsif run("write_then_read_back") then + check_program_readback(net, flash, 16#2000#); + elsif run("back_to_back_reads") then + check_back_to_back_reads(net, 16#400#, 16#500#, 32); + elsif run("output_delay_margin") then + for clqv_ns in 2 to 7 loop + info("checking with tCLQV = " & to_string(clqv_ns) & " ns"); + set_output_valid_delay(net, flash, clqv_ns * 1 ns); + check_flash_read(net, FAST_READ_4BYTE_QUAD_OP, 8, 16#1200#, 32); + end loop; + end if; + end loop; + + wait for 1 us; + if cs_n = '0' then + wait until cs_n = '1' for 1 ms; + end if; + + wait for 1 us; + test_runner_cleanup(runner); + wait; + end process; + + test_runner_watchdog(runner, 10 ms); + +end tb; diff --git a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_tb.vhd b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_tb.vhd new file mode 100644 index 00000000..aec8c987 --- /dev/null +++ b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_tb.vhd @@ -0,0 +1,106 @@ +-- This Source Code Form is subject to the terms of the Mozilla Public +-- License, v. 2.0. If a copy of the MPL was not distributed with this +-- file, You can obtain one at https://mozilla.org/MPL/2.0/. +-- +-- The same checked scenarios as spi_nor_tb, run at the fast configuration the +-- projects actually ship: sclk_divisor 0, so sclk is clk/2 (62.5MHz off the +-- 125MHz system clock) instead of clk/6. +-- +-- This is the configuration where every clk edge is also an sclk edge, so there +-- are no spare cycles between sclk edges for phase transitions to settle in. +-- It is a genuinely different timing regime from clk/6, not just a faster one, +-- which is why it gets its own testbench rather than a comment. + +library ieee; +use ieee.std_logic_1164.all; +use ieee.numeric_std.all; + +library vunit_lib; + context vunit_lib.com_context; + context vunit_lib.vunit_context; +use work.spi_nor_tb_pkg.all; +use work.spi_nor_pkg.all; +use work.spi_nor_target_vc_pkg.all; + +entity spi_nor_fast_tb is + generic ( + runner_cfg : string + ); +end entity; + +architecture tb of spi_nor_fast_tb is + +begin + + th: entity work.spi_nor_th + generic map ( + sclk_divisor => 0, + rx_sample_taps => 2 + ); + + bench: process + alias reset is <>; + alias cs_n is <>; + constant flash : actor_t := find("spi_nor_target"); + begin + test_runner_setup(runner, runner_cfg); + + wait until reset = '0'; + wait for 500 ns; + + fill_pattern(net, flash); + + while test_suite loop + if run("jedec_id") then + check_jedec_id(net); + elsif run("read_quad_32addr_dummy") then + check_flash_read(net, FAST_READ_4BYTE_QUAD_OP, 8, 16#1200#, 64); + elsif run("read_single_32addr") then + -- Note: on a real W25Q01JV plain READ_DATA is limited to 50MHz, + -- so this opcode is out of spec at this rate. It is exercised + -- here for the datapath only; hubris uses the fast reads. + check_flash_read(net, READ_DATA_4BYTE_OP, 0, 16#40#, 16); + elsif run("read_dual_32addr_dummy") then + check_flash_read(net, FAST_READ_4BYTE_DUAL_OP, 8, 16#80#, 16); + elsif run("write_then_read_back") then + check_program_readback(net, flash, 16#2000#); + elsif run("back_to_back_reads") then + check_back_to_back_reads(net, 16#400#, 16#500#, 32); + elsif run("output_delay_margin") then + -- Sweep the part's clock-low-to-output-valid delay across the + -- datasheet range and well beyond it, and require correct data + -- throughout. + -- + -- The harness already models the FPGA's clock-to-pin and + -- pin-to-flop delays, so this sweeps only the part's own tCLQV, + -- across and a little beyond its datasheet range. Combined with + -- the harness delays this covers the whole read round trip that + -- the XDC allows at the slow corner. + -- + -- The sample point has to satisfy + -- round_trip_valid - half_period <= S <= half_period + round_trip_hold + -- and note the upper limit comes from tCLQX (output hold), not + -- tCLQV: sampling too late catches the *next* bit. That upper + -- limit is the binding one at the fast corner, which is what + -- spi_nor_fast_quick_io_tb covers. + for clqv_ns in 2 to 7 loop + info("checking with tCLQV = " & to_string(clqv_ns) & " ns"); + set_output_valid_delay(net, flash, clqv_ns * 1 ns); + check_flash_read(net, FAST_READ_4BYTE_QUAD_OP, 8, 16#1200#, 32); + end loop; + end if; + end loop; + + wait for 1 us; + if cs_n = '0' then + wait until cs_n = '1' for 1 ms; + end if; + + wait for 1 us; + test_runner_cleanup(runner); + wait; + end process; + + test_runner_watchdog(runner, 10 ms); + +end tb; diff --git a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb.vhd b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb.vhd index e28becf6..3662c867 100644 --- a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb.vhd +++ b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb.vhd @@ -11,6 +11,7 @@ library vunit_lib; context vunit_lib.vunit_context; use work.spi_nor_tb_pkg.all; use work.spi_nor_pkg.all; +use work.spi_nor_target_vc_pkg.all; entity spi_nor_tb is generic ( @@ -31,6 +32,7 @@ begin -- reset_b uses the relative path form of external naming for example purposes. alias reset is <>; alias cs_n is <>; + constant flash : actor_t := find("spi_nor_target"); begin -- Always the first thing in the process, set up things for the VUnit test runner test_runner_setup(runner, runner_cfg); @@ -40,12 +42,25 @@ begin wait until reset = '0'; wait for 500 ns; - -- I haven't built a flash model yet, so I'm just doing some basic - -- waveform inspection here. + -- Give the flash model known contents so read paths can be checked + -- rather than just watched. + fill_pattern(net, flash); + while test_suite loop if run("instr_only") then - write_data_size(net, 3); -- read out 3 bytes - write_instr(net, READ_JEDEC_ID_OP); + check_jedec_id(net); + elsif run("read_quad_32addr_dummy") then + -- The read path that actually matters: hubris and the eSPI flash + -- reader both use 4-byte-address quad fast read with 8 dummies. + check_flash_read(net, FAST_READ_4BYTE_QUAD_OP, 8, 16#1200#, 64); + elsif run("read_single_32addr") then + check_flash_read(net, READ_DATA_4BYTE_OP, 0, 16#40#, 16); + elsif run("read_dual_32addr_dummy") then + check_flash_read(net, FAST_READ_4BYTE_DUAL_OP, 8, 16#80#, 16); + elsif run("write_then_read_back") then + check_program_readback(net, flash, 16#2000#); + elsif run("back_to_back_reads") then + check_back_to_back_reads(net, 16#400#, 16#500#, 32); elsif run("write_24addr_no_dummy") then write_data(net, x"03020100"); -- do do words write_data(net, x"07060504"); -- do do words diff --git a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb_pkg.vhd b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb_pkg.vhd index 6403b635..2c0476fe 100644 --- a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb_pkg.vhd +++ b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb_pkg.vhd @@ -12,6 +12,8 @@ library vunit_lib; context vunit_lib.com_context; context vunit_lib.vc_context; use work.spi_nor_regs_pkg.all; +use work.spi_nor_pkg.all; +use work.spi_nor_target_vc_pkg.all; package spi_nor_tb_pkg is @@ -43,6 +45,65 @@ package spi_nor_tb_pkg is data : integer ); + procedure clear_fifos ( + signal net : inout network_t + ); + + -- Wait for a transaction to start and then finish. Polling only for "not + -- busy" races the start of the transaction: writing Instr queues the go + -- strobe, and a register read issued straight afterwards can easily land + -- before cs_n has fallen. + procedure wait_txn_done ( + signal net : inout network_t + ); + + procedure read_rx_word ( + signal net : inout network_t; + variable data : out std_logic_vector(31 downto 0) + ); + + -- Pop count bytes out of the RX FIFO and check them against the pattern the + -- flash model was preloaded with. This is what makes the rx sample point + -- observable: a sample taken outside the part's valid window shifts in 'X' + -- or a neighbouring bit, and lands here as a mismatch. + procedure check_read_pattern ( + signal net : inout network_t; + constant addr : natural; + constant count : natural + ); + + -- Checked scenarios, shared by the testbenches so the same coverage can be + -- run at more than one sclk rate without duplicating it. + + procedure check_jedec_id ( + signal net : inout network_t + ); + + -- opcode picks single / dual / quad and 3 or 4 byte addressing; dummies must + -- match what the part expects for that opcode. + procedure check_flash_read ( + signal net : inout network_t; + constant opcode : std_logic_vector(7 downto 0); + constant dummies : natural; + constant addr : natural; + constant count : natural + ); + + procedure check_program_readback ( + signal net : inout network_t; + constant flash : actor_t; + constant addr : natural + ); + + -- Two reads issued with no software delay between them, which is what the + -- eSPI reader does when it chains page reads. + procedure check_back_to_back_reads ( + signal net : inout network_t; + constant addr_a : natural; + constant addr_b : natural; + constant count : natural + ); + end package; package body spi_nor_tb_pkg is @@ -87,4 +148,164 @@ package body spi_nor_tb_pkg is write_bus(net, bus_handle, To_StdLogicVector(TX_FIFO_WDATA_OFFSET + 16#100#, bus_handle.p_address_length), resize(data, 32)); end; + procedure clear_fifos ( + signal net : inout network_t + ) is + begin + write_bus(net, bus_handle, To_StdLogicVector(SPICR_OFFSET + 16#100#, bus_handle.p_address_length), + SPICR_RX_FIFO_RESET_MASK or SPICR_TX_FIFO_RESET_MASK); + + -- The self-clearing reset bits produce a single-cycle pulse into the XPM + -- dual-clock FIFOs, which want their reset held longer than that and are + -- unusable for a number of cycles afterwards. Software gets away with it + -- because consecutive register accesses are far apart; a testbench + -- issuing back-to-back writes does not, and the FIFO data is dropped. + wait for 1 us; + end; + + procedure wait_txn_done ( + signal net : inout network_t + ) is + + variable status : std_logic_vector(31 downto 0); + variable started : boolean := false; + + begin + -- Wait for cs_n to fall. A register read takes several clocks and cs_n + -- asserts a handful of clocks after the go strobe, so this always + -- catches even the shortest transaction. + for i in 0 to 63 loop + read_bus(net, bus_handle, To_StdLogicVector(SPISR_OFFSET + 16#100#, bus_handle.p_address_length), status); + if (status and SPISR_BUSY_MASK) /= (status'range => '0') then + started := true; + exit; + end if; + end loop; + + check_true(started, "transaction never became busy"); + + loop + read_bus(net, bus_handle, To_StdLogicVector(SPISR_OFFSET + 16#100#, bus_handle.p_address_length), status); + exit when (status and SPISR_BUSY_MASK) = (status'range => '0'); + end loop; + end; + + procedure read_rx_word ( + signal net : inout network_t; + variable data : out std_logic_vector(31 downto 0) + ) is + + variable status : std_logic_vector(31 downto 0); + + begin + -- The RX FIFO is a dual-clock FIFO and the last partial word is only + -- pushed when cs_n rises, so a word can still be in flight when busy + -- clears. Wait for it to show up rather than reading a stale register. + loop + read_bus(net, bus_handle, To_StdLogicVector(SPISR_OFFSET + 16#100#, bus_handle.p_address_length), status); + exit when (status and SPISR_RX_EMPTY_MASK) = (status'range => '0'); + end loop; + + read_bus(net, bus_handle, To_StdLogicVector(RX_FIFO_RDATA_OFFSET + 16#100#, bus_handle.p_address_length), data); + end; + + procedure check_read_pattern ( + signal net : inout network_t; + constant addr : natural; + constant count : natural + ) is + + variable word : std_logic_vector(31 downto 0); + variable got : std_logic_vector(7 downto 0); + + begin + for i in 0 to count - 1 loop + -- The width adaptor packs bytes little-endian into each word, so a + -- fresh word is popped every fourth byte. + if i mod 4 = 0 then + read_rx_word(net, word); + end if; + + got := word(8 * (i mod 4) + 7 downto 8 * (i mod 4)); + check_equal(got, pattern_byte(addr + i), + "flash read data mismatch at offset " & to_string(i) & + " (flash address " & to_string(addr + i) & ")"); + end loop; + end; + + procedure check_jedec_id ( + signal net : inout network_t + ) is + + variable word : std_logic_vector(31 downto 0); + + begin + clear_fifos(net); + write_data_size(net, 3); + write_dummy(net, 0); + write_instr(net, READ_JEDEC_ID_OP); + wait_txn_done(net); + read_rx_word(net, word); + -- Winbond mfr id, W25Q01JV device id, lsb-packed by the width adaptor + check_equal(word(23 downto 0), std_logic_vector'(x"2140EF"), + "unexpected JEDEC id"); + end; + + procedure check_flash_read ( + signal net : inout network_t; + constant opcode : std_logic_vector(7 downto 0); + constant dummies : natural; + constant addr : natural; + constant count : natural + ) is + begin + clear_fifos(net); + write_dummy(net, dummies); + write_data_size(net, count); + write_addr(net, To_StdLogicVector(addr, 32)); + write_instr(net, opcode); + wait_txn_done(net); + check_read_pattern(net, addr, count); + end; + + procedure check_program_readback ( + signal net : inout network_t; + constant flash : actor_t; + constant addr : natural + ) is + + variable got : std_logic_vector(7 downto 0); + + begin + clear_fifos(net); + -- NOR can only clear bits, so start from erased + erase_flash(net, flash, addr, 16#1000#); + write_data(net, x"03020100"); + write_data(net, x"07060504"); + write_dummy(net, 0); + write_data_size(net, 8); + write_addr(net, To_StdLogicVector(addr, 32)); + write_instr(net, PAGE_PROGRAM_4BYTE_OP); + wait_txn_done(net); + + for i in 0 to 7 loop + read_flash_byte(net, flash, addr + i, got); + check_equal(got, std_logic_vector(to_unsigned(i, 8)), + "programmed byte mismatch at offset " & to_string(i)); + end loop; + end; + + procedure check_back_to_back_reads ( + signal net : inout network_t; + constant addr_a : natural; + constant addr_b : natural; + constant count : natural + ) is + begin + check_flash_read(net, FAST_READ_4BYTE_QUAD_OP, 8, addr_a, count); + -- No delay here on purpose: the flash needs a minimum cs_n high time + -- and the model enforces it. + check_flash_read(net, FAST_READ_4BYTE_QUAD_OP, 8, addr_b, count); + end; + end package body; diff --git a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_th.vhd b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_th.vhd index 4e275c1f..46750368 100644 --- a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_th.vhd +++ b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_th.vhd @@ -18,6 +18,20 @@ use work.axil26x32_pkg; use work.axilite_if_2k19_helper_pkg.all; entity spi_nor_th is + generic ( + -- Defaults match the shipped-for-years configuration: 125MHz / 6 = + -- 20.83MHz sclk with the sample point one clk after the rising edge. + sclk_divisor : natural := 2; + rx_sample_taps : natural := 2; + -- Delay from a launch flop to the pin at the flash, and from the pin back + -- to the capture flop, board trace included. RTL simulation has no notion + -- of either, but together they are a bigger share of a 16ns sclk period + -- than the flash's own tCLQV, so the sample point cannot be validated + -- without them. The XDC bounds both (via the output delay window and the + -- IOB packing); these defaults are the slow corner it allows. + out_delay : time := 3.7 ns; + in_delay : time := 1.5 ns + ); end entity; architecture th of spi_nor_th is @@ -30,6 +44,12 @@ architecture th of spi_nor_th is signal io : std_logic_vector(3 downto 0); signal io_o : std_logic_vector(3 downto 0); signal io_oe : std_logic_vector(3 downto 0); + signal flash_o : std_logic_vector(3 downto 0); + signal flash_oe : std_logic_vector(3 downto 0); + -- The bus and clock as seen at the part, i.e. after the outbound delay + signal io_flash : std_logic_vector(3 downto 0); + signal sclk_flash : std_logic; + signal csn_flash : std_logic; constant config_array : axil_responder_cfg_array_t(0 downto 0) := ( 0 => resp_cfg(base_addr => x"00000100", addr_span_bits => 8) @@ -80,6 +100,10 @@ begin resiser: entity work.axil8_resizer port map(fabric => responders(0), responder =>responders_8b(0)); spi_nor_top_inst: entity work.spi_nor_top + generic map ( + sclk_divisor => sclk_divisor, + rx_sample_taps => rx_sample_taps + ) port map ( clk => clk, reset => reset, @@ -89,6 +113,7 @@ begin io => io, io_o => io_o, io_oe => io_oe, + sp5_owns_flash => open, espi_cmd_fifo_rdata => (others => '0'), espi_cmd_fifo_rdack => open, espi_cmd_fifo_rempty => '1', @@ -96,10 +121,52 @@ begin espi_data_fifo_write => open ); - io_tris: process(all) + -- Everything the part sees is delayed by out_delay; everything the DUT + -- captures is delayed again by in_delay on the way back. + sclk_flash <= sclk after out_delay; + csn_flash <= cs_n after out_delay; + + flash: entity work.spi_nor_target_vc + generic map ( + actor_name => "spi_nor_target" + ) + port map ( + cs_n => csn_flash, + sclk => sclk_flash, + io => io_flash, + io_o => flash_o, + io_oe => flash_oe + ); + + -- Both ends contribute to the resolved bus at the part, plus a weak pull-up + -- standing in for the board's. If both ends drive the same lane the + -- resolution goes to 'X', which the DUT then shifts in and the data checks + -- catch. + bus_gen: for i in io_flash'range generate + io_flash(i) <= io_o(i) after out_delay when io_oe(i) = '1' else 'Z' after out_delay; + io_flash(i) <= flash_o(i) when flash_oe(i) = '1' else 'Z'; + io_flash(i) <= 'H'; + end generate; + + io <= io_flash after in_delay; + + -- Contention is a real bug that shows up as marginal reads on hardware, so + -- name it explicitly rather than leaving it to be inferred from an 'X'. + contention_check: process(all) + alias rel is << signal spi_nor_top_inst.release_lanes : std_logic_vector(3 downto 0) >>; + alias in_tx is << signal spi_nor_top_inst.in_tx_phases : boolean >>; + alias in_rx is << signal spi_nor_top_inst.in_rx_phases : boolean >>; begin for i in io'range loop - io(i) <= io_o(i) when io_oe(i) = '1' else 'H'; + assert not (io_oe(i) = '1' and flash_oe(i) = '1') + report "Bus contention: controller and flash both driving io(" & + to_string(i) & "), ctrl_oe=" & to_string(io_oe) & + " flash_oe=" & to_string(flash_oe) & + " cs_n=" & to_string(cs_n) & + " release=" & to_string(rel) & + " in_tx=" & to_string(in_tx) & + " in_rx=" & to_string(in_rx) + severity error; end loop; end process; diff --git a/hdl/ip/vhd/spi_nor_controller/spi_nor_top.vhd b/hdl/ip/vhd/spi_nor_controller/spi_nor_top.vhd index 0dc27f17..dd6cdd08 100644 --- a/hdl/ip/vhd/spi_nor_controller/spi_nor_top.vhd +++ b/hdl/ip/vhd/spi_nor_controller/spi_nor_top.vhd @@ -11,6 +11,18 @@ use work.spi_nor_regs_pkg.all; use work.axil8x32_pkg.all; entity spi_nor_top is + generic ( + -- sclk = clk / (2 * (sclk_divisor + 1)). 0 gives clk/2, which is the + -- fastest this structure supports. + sclk_divisor : natural := 2; + -- Read data sample point, in half-clk steps after the sclk rising edge. + -- 2 is the historical behaviour (one whole clk after the edge). See + -- spi_link for how the taps map onto capture flops. + rx_sample_taps : natural range 0 to 4 := 2; + -- Chip select timing, in clk cycles. Independent of sclk. + cs_setup_cnts : natural := 4; + cs_high_cnts : natural := 7 + ); port ( clk : in std_logic; reset : in std_logic; @@ -40,11 +52,17 @@ end entity; architecture rtl of spi_nor_top is - constant div_val : unsigned(15 downto 0) := to_unsigned(2, 16); + constant div_val : unsigned(15 downto 0) := to_unsigned(sclk_divisor, 16); signal rx_byte_done : boolean; - signal tx_byte_done : boolean; + signal tx_byte_req : boolean; signal in_rx_phases : boolean; signal in_tx_phases : boolean; + signal release_lanes : std_logic_vector(3 downto 0); + -- Internal copies of sclk and cs_n. The versions that leave the block are + -- duplicate flops with no internal fanout so they can be packed into the + -- IOBs; these are what the phase logic looks at. + signal sclk_internal : std_logic; + signal cs_n_internal : std_logic; signal link_rx_byte : std_logic_vector(7 downto 0); signal link_tx_byte : std_logic_vector(7 downto 0); signal cur_io_mode : io_mode; @@ -87,6 +105,9 @@ begin -- this does clock-gen, and has the serializer and -- deserializer that operate during tx and rx phases link: entity work.spi_link + generic map ( + rx_sample_taps => rx_sample_taps + ) port map ( clk => clk, reset => reset, @@ -95,15 +116,17 @@ begin divisor => div_val, in_tx_phases => in_tx_phases, in_rx_phases => in_rx_phases, + release_lanes => release_lanes, rx_byte => link_rx_byte, rx_byte_done => rx_byte_done, tx_byte => link_tx_byte, - tx_byte_done => tx_byte_done, + tx_byte_req => tx_byte_req, sclk_redge => open, sclk_fedge => open, -- spi link signals - sclk => sclk, - cs_n => cs_n, + sclk => sclk_internal, + sclk_pin => sclk, + cs_n => cs_n_internal, io => io, io_o => io_o, io_oe => io_oe @@ -113,20 +136,26 @@ begin -- feeds data to/from the serializer/deserializer -- to/from FIFOs spi_txn_mgr_inst: entity work.spi_txn_mgr + generic map ( + cs_setup_cnts => cs_setup_cnts, + cs_high_cnts => cs_high_cnts + ) port map ( clk => clk, reset => reset, -- registers i/f spi_cmd => spi_cmd_if, -- link i/f - cs_n => cs_n, - sclk => sclk, + cs_n => cs_n_internal, + cs_n_pin => cs_n, + sclk => sclk_internal, rx_byte_done => rx_byte_done, rx_link_byte => link_rx_byte, - tx_byte_done => tx_byte_done, + tx_byte_req => tx_byte_req, tx_link_byte => link_tx_byte, in_rx_phases => in_rx_phases, in_tx_phases => in_tx_phases, + release_lanes => release_lanes, cur_io_mode => cur_io_mode, tx_fifo_ack => tx_fifo_read8, tx_fifo_data => tx_fifo_data8, @@ -155,7 +184,7 @@ begin port map ( clk => clk, reset => reset, - txn_complete => cs_n, + txn_complete => cs_n_internal, -- TX FIFO Interface read_data => tx_fifo_data8, read_ack => tx_fifo_read8, @@ -237,7 +266,7 @@ begin spisr_reg.rx_full <= '1' when spisr_reg.rx_used_wds = 64 else '0'; - spisr_reg.busy <= '1' when cs_n = '0' else '0'; + spisr_reg.busy <= '1' when cs_n_internal = '0' else '0'; -- Hubris-interactable registers for control and status spi_nor_regs_inst: entity work.spi_nor_regs diff --git a/hdl/ip/vhd/spi_nor_controller/spi_txn/spi_txn_mgr.vhd b/hdl/ip/vhd/spi_nor_controller/spi_txn/spi_txn_mgr.vhd index 5936f356..f11882df 100644 --- a/hdl/ip/vhd/spi_nor_controller/spi_txn/spi_txn_mgr.vhd +++ b/hdl/ip/vhd/spi_nor_controller/spi_txn/spi_txn_mgr.vhd @@ -9,6 +9,15 @@ use ieee.numeric_std_unsigned.all; use work.spi_nor_pkg.all; entity spi_txn_mgr is + generic ( + -- clk cycles from cs_n asserting to the first sclk edge (tSLCH) + cs_setup_cnts : natural := 4; + -- Minimum clk cycles cs_n must stay high between transactions (tSHSL). + -- The eSPI read path chains page reads back to back and will otherwise + -- re-assert cs_n the cycle after it goes high, which the flash does not + -- allow. This is in clk cycles, so it does not scale with sclk. + cs_high_cnts : natural := 7 + ); port ( clk : in std_logic; reset : in std_logic; @@ -20,15 +29,20 @@ entity spi_txn_mgr is -- instr: in std_logic_vector(7 downto 0); -- go_flag: in std_logic; -- link interface - cs_n : out std_logic; + cs_n : out std_logic; + -- Second copy of the cs_n flop, for the pin only, so it can be packed + -- into the IOB. Same reasoning as spi_clk_gen's sclk_pin. + cs_n_pin : out std_logic; sclk : in std_logic; rx_byte_done : in boolean; rx_link_byte : in std_logic_vector(7 downto 0); - tx_byte_done : in boolean; + tx_byte_req : in boolean; tx_link_byte : out std_logic_vector(7 downto 0); in_rx_phases : out boolean; in_tx_phases : out boolean; - cur_io_mode : out io_mode; + -- Lanes to stop driving ahead of a turnaround, see below + release_lanes : out std_logic_vector(3 downto 0); + cur_io_mode : out io_mode; -- fifo interface rx_fifo_data : out std_logic_vector(7 downto 0); rx_fifo_write : out std_logic; @@ -42,7 +56,6 @@ architecture rtl of spi_txn_mgr is attribute mark_debug : string; constant BYTES_24BIT_ADDR : integer := 3; constant BYTES_32BIT_ADDR : integer := 4; - constant CS_CLK_DELAY_CNTS : integer := 4; type state_t is (idle, cs_assert, instruction, addr, dummy, wdata, rdata, cs_deassert); @@ -51,8 +64,11 @@ architecture rtl of spi_txn_mgr is txn : txn_info_t; csn : std_logic; counter : integer range 0 to 512; + -- Counts down the enforced cs_n high time. Separate from `counter` + -- because it has to keep running while we sit in idle. + cs_high : integer range 0 to 63; end record; - constant r_reset : reg_type := (idle, txn_info_t_reset, '1', 0); + constant r_reset : reg_type := (idle, txn_info_t_reset, '1', 0, 0); signal r, rin : reg_type; attribute mark_debug of r : signal is "TRUE"; @@ -66,11 +82,17 @@ architecture rtl of spi_txn_mgr is ) return io_mode is begin if state = idle or + state = cs_assert or state = instruction or state = addr or state = dummy then -- no matter what the transaction moves to, we're in single mode - -- for these phases + -- for these phases. cs_assert belongs here because the serializer is + -- already shifting the opcode out during it: sclk is enabled as soon + -- as we assert cs_n, and at clk/2 the first sclk edge lands inside + -- cs_assert rather than after it. Reporting the data mode here made + -- the opcode go out 4 bits at a time, so the part saw a different + -- instruction entirely. return single; else -- otherwise use the mode specified by the opcode @@ -79,12 +101,22 @@ architecture rtl of spi_txn_mgr is end; -- This function takes the state and returns if we're driving data lines - -- currently + -- currently. + -- + -- cs_deassert counts as driving for anything that is not a read: the part + -- samples our last bit on the sclk edge after we leave wdata, and io_oe is + -- registered, so releasing there would change mosi on the very edge being + -- sampled once a half period is a single clk cycle. Holding until cs_n rises + -- is what a controller should do anyway. Reads must not drive here, since + -- the part keeps driving until it is deselected. function is_in_tx_phases ( + txn: txn_info_t; state: state_t ) return boolean is begin - return state = cs_assert or state = wdata or state = dummy or state = addr or state = instruction; + return state = cs_assert or state = wdata or state = dummy or + state = addr or state = instruction or + (state = cs_deassert and txn.data_kind /= read); end; -- This function takes the state and returns if we're sampling data lines @@ -104,7 +136,7 @@ begin rx_fifo_data <= rx_link_byte; rx_fifo_write <= '1' when rx_byte_done else '0'; in_rx_phases <= is_in_rx_phases(r.state); - in_tx_phases <= is_in_tx_phases(r.state); + in_tx_phases <= is_in_tx_phases(r.txn, r.state); -- more complicated outputs @@ -114,14 +146,28 @@ begin cur_io_mode <= get_cur_io_mode(r.txn, r.state); end process; - -- Set up a bunch of muxes, and other signals that are used in the modules below - -- mux into the serializer: we are sending static data sometimes, and fifo data sometimes - tx_link_byte <= spi_cmd.instr when (r.state = instruction or r.state = cs_assert) else - spi_cmd.addr(8 * r.counter + 7 downto 8 * r.counter) when r.state = addr else - tx_fifo_data when r.state = wdata else - (others => '1'); - -- we only want to FIFO ack when we were reading from the fifo, not the static data - tx_fifo_ack <= '1' when tx_byte_done and r.state = wdata else '0'; + -- Stop driving the lanes the flash is about to drive, an sclk cycle before + -- it starts. Only multi-bit reads with dummy cycles need this: during the + -- dummy phase cur_io_mode is still single, so io0/io3 are being driven for + -- HOLD avoidance right up to the point a dual or quad read takes them over. + -- At 20MHz the registered io_oe happened to drop in time; at clk/2 it does + -- not, and the overlap shows up as marginal first bytes rather than an + -- obvious failure. Lanes the part never drives in this mode are left alone. + release_gen: process(all) + begin + release_lanes <= "0000"; + if r.state = dummy and r.txn.data_kind = read and r.counter <= 1 then + case r.txn.data_mode is + when quad => + release_lanes <= "1111"; + when dual => + release_lanes <= "0011"; + when single => + -- the part only drives io1, which we are not driving + release_lanes <= "0000"; + end case; + end if; + end process; -- main controller state machine controller: process(all) @@ -130,13 +176,20 @@ begin begin v := r; slk_redge := sclk = '1' and sclk_last = '0'; + + if r.cs_high > 0 then + v.cs_high := r.cs_high - 1; + end if; case r.state is when idle => - if spi_cmd.go_flag then + -- Hold off until the part's minimum cs_n high time has elapsed. + -- The eSPI manager re-asserts go_flag as soon as it sees us go + -- un-busy, which is the same cycle cs_n rises. + if spi_cmd.go_flag = '1' and r.cs_high = 0 then v.state := cs_assert; -- build up transaction info based on opcode v.txn := get_txn_info(spi_cmd.instr); - v.counter := CS_CLK_DELAY_CNTS; + v.counter := cs_setup_cnts; end if; when cs_assert => if r.counter = 0 then @@ -149,7 +202,7 @@ begin -- address phase, or issue dummy clocks, -- or go directly to a read/write phase -- so we check all the options here - if tx_byte_done then + if tx_byte_req then case r.txn.addr_kind is when bit24 => v.counter := BYTES_24BIT_ADDR - 1; -- zero indexed @@ -170,7 +223,7 @@ begin v.state := wdata; when none => v.state := cs_deassert; - v.counter := CS_CLK_DELAY_CNTS; + v.counter := cs_setup_cnts; end case; end if; end case; @@ -181,7 +234,7 @@ begin -- dummy clocks. I don't think there are commands -- that issue an address and then do nothing but -- we added the de-assert state for completeness - if tx_byte_done and r.counter = 0 then + if tx_byte_req and r.counter = 0 then if r.txn.uses_dummys then v.state := dummy; v.counter := to_integer(spi_cmd.dummy_cycles); @@ -193,9 +246,9 @@ begin v.counter := to_integer(spi_cmd.data_bytes); else v.state := cs_deassert; - v.counter := CS_CLK_DELAY_CNTS; + v.counter := cs_setup_cnts; end if; - elsif tx_byte_done then + elsif tx_byte_req then v.counter := r.counter - 1; end if; when dummy => @@ -208,7 +261,7 @@ begin v.counter := to_integer(spi_cmd.data_bytes); else v.state := cs_deassert; - v.counter := CS_CLK_DELAY_CNTS; + v.counter := cs_setup_cnts; end if; elsif slk_redge then v.counter := r.counter - 1; @@ -217,11 +270,11 @@ begin -- Data counter is 1 indexded to better align -- with sw expectations so we're done when -- r.counter = 1 - if tx_byte_done and r.counter = 1 then + if tx_byte_req and r.counter = 1 then -- We're done with the transaction v.state := cs_deassert; - v.counter := CS_CLK_DELAY_CNTS; - elsif tx_byte_done then + v.counter := cs_setup_cnts; + elsif tx_byte_req then v.counter := r.counter - 1; end if; when rdata => @@ -231,7 +284,7 @@ begin if rx_byte_done and r.counter = 1 then -- We're done with the transaction v.state := cs_deassert; - v.counter := CS_CLK_DELAY_CNTS; + v.counter := cs_setup_cnts; elsif rx_byte_done then v.counter := r.counter - 1; end if; @@ -248,6 +301,34 @@ begin v.csn := '0'; elsif v.state = idle then v.csn := '1'; + -- Arm the high-time counter on the edge that raises cs_n so the + -- next transaction cannot start too soon. + if r.csn = '0' then + v.cs_high := cs_high_cnts; + end if; + end if; + + -- The serializer consumes a byte on the same edge that this state + -- machine advances, so the byte it sees has to come from the state we + -- are moving *to*, not the one we are leaving. Driving this off `r` + -- works only while the phase advance lands a cycle before the shifter + -- reload, which stops being true once the sclk half period is a single + -- clk cycle. + if v.state = instruction or v.state = cs_assert then + tx_link_byte <= spi_cmd.instr; + elsif v.state = addr then + tx_link_byte <= spi_cmd.addr(8 * v.counter + 7 downto 8 * v.counter); + elsif v.state = wdata then + tx_link_byte <= tx_fifo_data; + else + tx_link_byte <= (others => '1'); + end if; + + -- Only ack the FIFO for bytes that actually came from it + if tx_byte_req and v.state = wdata then + tx_fifo_ack <= '1'; + else + tx_fifo_ack <= '0'; end if; rin <= v; @@ -258,9 +339,13 @@ begin if reset then r <= r_reset; sclk_last <= '0'; + cs_n_pin <= '1'; elsif rising_edge(clk) then sclk_last <= sclk; r <= rin; + -- Duplicate of r.csn, driven from the same next-state value so the + -- two flops always agree and change on the same edge. + cs_n_pin <= rin.csn; end if; end process; diff --git a/hdl/ip/vhd/vunit_components/BUCK b/hdl/ip/vhd/vunit_components/BUCK index 4560f006..17103cfd 100644 --- a/hdl/ip/vhd/vunit_components/BUCK +++ b/hdl/ip/vhd/vunit_components/BUCK @@ -20,6 +20,13 @@ vhdl_unit( visibility = ['PUBLIC'], ) +vhdl_unit( + name = "spi_nor_target_vc", + srcs = glob(["spi_nor_target/*.vhd"]), + standard = "2019", + visibility = ['PUBLIC'], +) + vhdl_unit( name = "i2c_cmd_vc", srcs = glob(["i2c_cmd/*.vhd"]), diff --git a/hdl/ip/vhd/vunit_components/spi_nor_target/spi_nor_target_vc.vhd b/hdl/ip/vhd/vunit_components/spi_nor_target/spi_nor_target_vc.vhd new file mode 100644 index 00000000..2aa00aa2 --- /dev/null +++ b/hdl/ip/vhd/vunit_components/spi_nor_target/spi_nor_target_vc.vhd @@ -0,0 +1,575 @@ +-- This Source Code Form is subject to the terms of the Mozilla Public +-- License, v. 2.0. If a copy of the MPL was not distributed with this +-- file, You can obtain one at https://mozilla.org/MPL/2.0/. +-- +-- Winbond W25Q-family QSPI NOR flash target model. +-- +-- This exists to answer two questions the previous "pull the bus to 'H' and +-- eyeball the waveform" harness could not: +-- +-- 1) does the controller sample read data at the right point, and +-- 2) does it meet the flash's AC timing at the sclk rate we want to run. +-- +-- So read data is driven with a real tCLQV output delay and is only held until +-- tCLQX after the falling edge, going to 'X' in between. A controller that +-- samples outside the valid window therefore shifts in 'X' and fails the +-- testbench's data check rather than quietly producing a plausible waveform. +-- MOSI setup/hold and the chip-select timing are checked directly. +-- +-- Bit ordering follows the datasheet: on multi-bit transfers the highest +-- numbered IO carries the most significant bit of each group. + +library ieee; + use ieee.std_logic_1164.all; + use ieee.numeric_std.all; + +library vunit_lib; + context vunit_lib.vunit_context; + context vunit_lib.com_context; + +use work.spi_nor_target_vc_pkg.all; + +entity spi_nor_target_vc is + generic ( + actor_name : string := "spi_nor_target"; + -- W25Q01JV AC characteristics. The read data path delays are the + -- interesting ones; they are what bounds the controller's usable + -- sample window. + t_clqv : time := 6 ns; -- clock low to output valid (max) + t_clqx : time := 1.5 ns; -- output hold after clock low (min) + t_dvch : time := 2 ns; -- data in setup to clock high + t_chdx : time := 3 ns; -- data in hold after clock high + t_slch : time := 5 ns; -- cs low to first clock high + t_shsl : time := 30 ns; -- cs high time between transactions + -- Winbond manufacturer id, W25Q01JV device id + jedec_id : std_logic_vector(23 downto 0) := x"EF4021"; + unique_id : std_logic_vector(63 downto 0) := x"0123456789ABCDEF" + ); + port ( + cs_n : in std_logic; + sclk : in std_logic; + -- Resolved bus as seen by the part + io : in std_logic_vector(3 downto 0); + -- Model's contribution to the bus, resolved externally so that + -- contention with the controller is visible + io_o : out std_logic_vector(3 downto 0) := (others => '0'); + io_oe : out std_logic_vector(3 downto 0) := (others => '0') + ); +end entity; + +architecture model of spi_nor_target_vc is + + -- Opcodes this model understands. Anything else is logged and treated as a + -- no-operand command. + constant OP_WRITE_ENABLE : std_logic_vector(7 downto 0) := x"06"; + constant OP_WRITE_DISABLE : std_logic_vector(7 downto 0) := x"04"; + constant OP_READ_STATUS_1 : std_logic_vector(7 downto 0) := x"05"; + constant OP_READ_STATUS_2 : std_logic_vector(7 downto 0) := x"35"; + constant OP_READ_STATUS_3 : std_logic_vector(7 downto 0) := x"15"; + constant OP_WRITE_STATUS_1 : std_logic_vector(7 downto 0) := x"01"; + constant OP_WRITE_STATUS_2 : std_logic_vector(7 downto 0) := x"31"; + constant OP_WRITE_STATUS_3 : std_logic_vector(7 downto 0) := x"11"; + constant OP_READ_DATA : std_logic_vector(7 downto 0) := x"03"; + constant OP_READ_DATA_4B : std_logic_vector(7 downto 0) := x"13"; + constant OP_FAST_READ : std_logic_vector(7 downto 0) := x"0B"; + constant OP_FAST_READ_4B : std_logic_vector(7 downto 0) := x"0C"; + constant OP_FAST_READ_DUAL : std_logic_vector(7 downto 0) := x"3B"; + constant OP_FAST_READ_DUAL_4B : std_logic_vector(7 downto 0) := x"3C"; + constant OP_FAST_READ_QUAD : std_logic_vector(7 downto 0) := x"6B"; + constant OP_FAST_READ_QUAD_4B : std_logic_vector(7 downto 0) := x"6C"; + constant OP_PAGE_PROGRAM : std_logic_vector(7 downto 0) := x"02"; + constant OP_PAGE_PROGRAM_4B : std_logic_vector(7 downto 0) := x"12"; + constant OP_QUAD_PAGE_PROGRAM : std_logic_vector(7 downto 0) := x"32"; + constant OP_QUAD_PAGE_PROGRAM_4B : std_logic_vector(7 downto 0) := x"34"; + constant OP_SECTOR_ERASE : std_logic_vector(7 downto 0) := x"20"; + constant OP_SECTOR_ERASE_4B : std_logic_vector(7 downto 0) := x"21"; + constant OP_BLOCK_ERASE_32K : std_logic_vector(7 downto 0) := x"52"; + constant OP_BLOCK_ERASE_64K : std_logic_vector(7 downto 0) := x"D8"; + constant OP_BLOCK_ERASE_64K_4B : std_logic_vector(7 downto 0) := x"DC"; + constant OP_READ_JEDEC_ID : std_logic_vector(7 downto 0) := x"9F"; + constant OP_READ_UNIQUE_ID : std_logic_vector(7 downto 0) := x"4B"; + constant OP_DIE_SELECT : std_logic_vector(7 downto 0) := x"C2"; + + constant SECTOR_BYTES : natural := 16#1000#; + constant BLOCK32_BYTES : natural := 16#8000#; + constant BLOCK64_BYTES : natural := 16#10000#; + constant PAGE_BYTES : natural := 256; + + type phase_t is (ph_cmd, ph_addr, ph_dummy, ph_rd, ph_wr, ph_done); + + -- Where read data comes from for the current opcode + type src_t is (src_mem, src_jedec, src_status, src_uid); + + shared variable mem : flash_mem_t; + + signal clqv : time := t_clqv; + + -- Snapshot of the bits sampled at the last input clock edge, published so + -- the hold checker can look at them tCHDX later. + signal sample_evt : std_logic := '0'; + signal sampled_data : std_logic_vector(3 downto 0) := (others => '0'); + signal sampled_lane : std_logic_vector(3 downto 0) := (others => '0'); + + signal dbg_phase : phase_t := ph_cmd; + signal dbg_opcode : std_logic_vector(7 downto 0) := (others => '0'); + + constant vc_logger : logger_t := get_logger("work:spi_nor_target_vc"); + +begin + + -- MOSI hold check. The main process publishes what it sampled and on which + -- lanes; tCHDX later those lanes must still be holding the same value. + hold_check : process is + + variable lanes : std_logic_vector(3 downto 0); + variable data : std_logic_vector(3 downto 0); + + begin + wait on sample_evt; + lanes := sampled_lane; + data := sampled_data; + wait for t_chdx; + + for i in lanes'range loop + if lanes(i) = '1' and cs_n = '0' then + if io(i) /= data(i) then + error(vc_logger, "Input hold (tCHDX) violated on io(" & to_string(i) & + "): sampled " & to_string(data(i)) & + " but bus is now " & to_string(io(i))); + end if; + end if; + end loop; + end process; + + -- Main protocol engine. Everything lives in one process so that the + -- rising-edge sampling and the falling-edge output presentation share + -- state with no inter-process delta delay. + proto : process (sclk, cs_n) is + + variable phase : phase_t := ph_cmd; + variable bit_cnt : natural := 0; + variable in_shifter : std_logic_vector(7 downto 0) := (others => '0'); + variable opcode : std_logic_vector(7 downto 0) := (others => '0'); + variable addr : unsigned(31 downto 0) := (others => '0'); + variable addr_left : natural := 0; + variable dummy_left : natural := 0; + variable in_width : natural := 1; + variable out_width : natural := 1; + variable out_src : src_t := src_mem; + variable rd_idx : natural := 0; + variable wr_idx : natural := 0; + variable out_shift : std_logic_vector(7 downto 0) := (others => '0'); + variable out_left : natural := 0; + variable status : std_logic_vector(7 downto 0) := x"00"; + variable erase_len : natural := 0; + variable do_erase : boolean := false; + variable cs_fall_at : time := 0 ps; + variable cs_rise_at : time := 0 ps; + variable first_clk : boolean := true; + variable t_invalid : time := t_clqx; + + -- The window is far smaller than the real part, so translate a flash + -- address into an index the backing store understands. Anything past + -- the window reads as erased. + impure function mem_addr (a : unsigned) return natural is + begin + if a < flash_window_bytes then + return to_integer(a); + else + return flash_window_bytes; + end if; + end function; + + -- Lanes the controller is expected to be driving in the current phase + impure function in_lanes return std_logic_vector is + begin + case in_width is + when 4 => + return "1111"; + when 2 => + return "0011"; + when others => + return "0001"; + end case; + end function; + + -- Setup check, split out so every call site uses a literal index and + -- the attribute prefix stays a static signal name. + procedure check_setup (constant idx : natural; constant since : time) is + begin + if since < t_dvch then + error(vc_logger, "Input setup (tDVCH) violated on io(" & to_string(idx) & + "): last changed " & to_string(since) & " before sclk"); + end if; + end procedure; + + -- Next byte the part would present, from whichever source the current + -- opcode selected + impure function next_out_byte return std_logic_vector is + + variable b : std_logic_vector(7 downto 0); + variable k : natural; + + begin + case out_src is + when src_jedec => + -- 3 id bytes, repeating for as long as the host clocks + k := rd_idx mod 3; + b := jedec_id(23 - 8 * k downto 16 - 8 * k); + when src_status => + b := status; + when src_uid => + if rd_idx < 8 then + b := unique_id(63 - 8 * rd_idx downto 56 - 8 * rd_idx); + else + b := x"FF"; + end if; + when src_mem => + b := mem.get(mem_addr(addr + rd_idx)); + end case; + + rd_idx := rd_idx + 1; + return b; + end function; + + -- Decode the opcode into the phase sequence and bus widths + procedure decode is + begin + in_width := 1; + out_width := 1; + out_src := src_mem; + rd_idx := 0; + wr_idx := 0; + addr := (others => '0'); + addr_left := 0; + dummy_left := 0; + do_erase := false; + erase_len := 0; + + case opcode is + when OP_READ_JEDEC_ID => + out_src := src_jedec; + phase := ph_rd; + when OP_READ_UNIQUE_ID => + out_src := src_uid; + dummy_left := 32; + phase := ph_dummy; + when OP_READ_STATUS_1 | OP_READ_STATUS_2 | OP_READ_STATUS_3 => + out_src := src_status; + phase := ph_rd; + when OP_WRITE_STATUS_1 | OP_WRITE_STATUS_2 | OP_WRITE_STATUS_3 => + phase := ph_wr; + when OP_WRITE_ENABLE => + status := status or x"02"; + phase := ph_done; + when OP_WRITE_DISABLE => + status := status and x"FD"; + phase := ph_done; + when OP_DIE_SELECT => + phase := ph_wr; + when OP_READ_DATA => + addr_left := 3; + phase := ph_addr; + when OP_READ_DATA_4B => + addr_left := 4; + phase := ph_addr; + when OP_FAST_READ => + addr_left := 3; + dummy_left := 8; + phase := ph_addr; + when OP_FAST_READ_4B => + addr_left := 4; + dummy_left := 8; + phase := ph_addr; + when OP_FAST_READ_DUAL => + addr_left := 3; + dummy_left := 8; + out_width := 2; + phase := ph_addr; + when OP_FAST_READ_DUAL_4B => + addr_left := 4; + dummy_left := 8; + out_width := 2; + phase := ph_addr; + when OP_FAST_READ_QUAD => + addr_left := 3; + dummy_left := 8; + out_width := 4; + phase := ph_addr; + when OP_FAST_READ_QUAD_4B => + addr_left := 4; + dummy_left := 8; + out_width := 4; + phase := ph_addr; + when OP_PAGE_PROGRAM => + addr_left := 3; + phase := ph_addr; + when OP_PAGE_PROGRAM_4B => + addr_left := 4; + phase := ph_addr; + when OP_QUAD_PAGE_PROGRAM => + addr_left := 3; + phase := ph_addr; + when OP_QUAD_PAGE_PROGRAM_4B => + addr_left := 4; + phase := ph_addr; + when OP_SECTOR_ERASE => + addr_left := 3; + erase_len := SECTOR_BYTES; + phase := ph_addr; + when OP_SECTOR_ERASE_4B => + addr_left := 4; + erase_len := SECTOR_BYTES; + phase := ph_addr; + when OP_BLOCK_ERASE_32K => + addr_left := 3; + erase_len := BLOCK32_BYTES; + phase := ph_addr; + when OP_BLOCK_ERASE_64K => + addr_left := 3; + erase_len := BLOCK64_BYTES; + phase := ph_addr; + when OP_BLOCK_ERASE_64K_4B => + addr_left := 4; + erase_len := BLOCK64_BYTES; + phase := ph_addr; + when others => + info(vc_logger, "Unmodelled opcode 0x" & to_hstring(opcode) & + ", treating as no-operand"); + phase := ph_done; + end case; + end procedure; + + -- What follows the address phase for this opcode + procedure after_addr is + begin + if dummy_left > 0 then + phase := ph_dummy; + elsif erase_len > 0 then + do_erase := true; + phase := ph_done; + elsif opcode = OP_READ_DATA or opcode = OP_READ_DATA_4B then + phase := ph_rd; + elsif opcode = OP_PAGE_PROGRAM or opcode = OP_PAGE_PROGRAM_4B then + phase := ph_wr; + elsif opcode = OP_QUAD_PAGE_PROGRAM or opcode = OP_QUAD_PAGE_PROGRAM_4B then + in_width := 4; + phase := ph_wr; + else + phase := ph_done; + end if; + end procedure; + + -- Programs wrap within the 256 byte page rather than running on + procedure program_byte (constant data : std_logic_vector(7 downto 0)) is + + variable page_base : unsigned(31 downto 0); + variable offset : natural; + + begin + page_base := addr(31 downto 8) & x"00"; + offset := (to_integer(addr(7 downto 0)) + wr_idx) mod PAGE_BYTES; + mem.set(mem_addr(page_base + offset), data); + wr_idx := wr_idx + 1; + end procedure; + + begin + if cs_n /= '0' then + -- Deselected: finish any pending erase, then reset per-transaction + -- state. Erases are modelled as instantaneous on deselect. + if do_erase then + mem.erase((mem_addr(addr) / erase_len) * erase_len, erase_len); + do_erase := false; + end if; + + if cs_n = '1' and cs_n'event then + cs_rise_at := now; + end if; + + phase := ph_cmd; + bit_cnt := 0; + in_width := 1; + out_width := 1; + out_left := 0; + first_clk := true; + in_shifter := (others => '0'); + io_oe <= (others => '0'); + io_o <= (others => '0'); + else + if cs_n = '0' and cs_n'event then + cs_fall_at := now; + + -- tSHSL: the part needs the select to stay high for a while + -- between transactions. Back to back page reads are the case + -- that trips this. + if cs_rise_at > 0 ps and now - cs_rise_at < t_shsl then + error(vc_logger, "cs_n high time (tSHSL) violated: " & + to_string(now - cs_rise_at) & " < " & to_string(t_shsl)); + end if; + end if; + + if rising_edge(sclk) then + if first_clk then + first_clk := false; + if now - cs_fall_at < t_slch then + error(vc_logger, "cs_n to first sclk (tSLCH) violated: " & + to_string(now - cs_fall_at) & " < " & to_string(t_slch)); + end if; + end if; + + case phase is + when ph_cmd | ph_addr | ph_wr => + -- Setup check on the lanes the controller owns, then + -- shift the group in. Highest lane is the msb. + check_setup(0, io(0)'last_event); + if in_width >= 2 then + check_setup(1, io(1)'last_event); + end if; + if in_width = 4 then + check_setup(2, io(2)'last_event); + check_setup(3, io(3)'last_event); + end if; + + sampled_data <= io; + sampled_lane <= in_lanes; + sample_evt <= not sample_evt; + + if in_width = 4 then + in_shifter := in_shifter(3 downto 0) & io(3) & io(2) & io(1) & io(0); + elsif in_width = 2 then + in_shifter := in_shifter(5 downto 0) & io(1) & io(0); + else + in_shifter := in_shifter(6 downto 0) & io(0); + end if; + bit_cnt := bit_cnt + in_width; + + if bit_cnt = 8 then + bit_cnt := 0; + + case phase is + when ph_cmd => + opcode := in_shifter; + decode; + when ph_addr => + addr := addr(23 downto 0) & unsigned(in_shifter); + addr_left := addr_left - 1; + if addr_left = 0 then + after_addr; + end if; + when ph_wr => + if opcode = OP_DIE_SELECT or + opcode = OP_WRITE_STATUS_1 or + opcode = OP_WRITE_STATUS_2 or + opcode = OP_WRITE_STATUS_3 then + status := in_shifter; + else + program_byte(in_shifter); + end if; + when others => + null; + end case; + end if; + + when ph_dummy => + dummy_left := dummy_left - 1; + if dummy_left = 0 then + -- Data starts on the next falling edge + phase := ph_rd; + out_left := 0; + end if; + + when ph_rd | ph_done => + null; + end case; + elsif falling_edge(sclk) then + if phase = ph_rd then + if out_left = 0 then + out_shift := next_out_byte; + out_left := 8; + end if; + + -- Hold the previous value until tCLQX, go invalid, then + -- present the new group at tCLQV. The invalid gap is what + -- makes the controller's sample point matter. + t_invalid := t_clqx; + if t_invalid >= clqv then + t_invalid := clqv / 2; + end if; + + case out_width is + when 4 => + io_o <= "XXXX" after t_invalid, + out_shift(7 downto 4) after clqv; + io_oe <= "1111" after t_invalid; + out_shift := out_shift(3 downto 0) & x"F"; + out_left := out_left - 4; + when 2 => + io_o(1 downto 0) <= "XX" after t_invalid, + out_shift(7 downto 6) after clqv; + io_oe <= "0011" after t_invalid; + out_shift := out_shift(5 downto 0) & "11"; + out_left := out_left - 2; + when others => + io_o(1) <= 'X' after t_invalid, + out_shift(7) after clqv; + io_oe <= "0010" after t_invalid; + out_shift := out_shift(6 downto 0) & '1'; + out_left := out_left - 1; + end case; + else + io_oe <= (others => '0'); + end if; + end if; + end if; + + dbg_phase <= phase; + dbg_opcode <= opcode; + end process; + + -- Backdoor access to the memory array for preload and readback + msg_handler : process is + + variable self : actor_t; + variable msg_type : msg_type_t; + variable request_msg : msg_t; + variable reply_msg : msg_t; + variable addr : natural; + variable len : natural; + variable data : std_logic_vector(7 downto 0); + + begin + self := new_actor(actor_name); + + loop + receive(net, self, request_msg); + msg_type := message_type(request_msg); + reply_msg := new_msg; + + if msg_type = flash_fill_pattern_msg then + mem.fill_pattern; + elsif msg_type = flash_write_byte_msg then + addr := pop(request_msg); + data := pop(request_msg); + -- Backdoor writes are absolute, not and-ed like a real program + mem.erase(addr, 1); + mem.set(addr, data); + elsif msg_type = flash_read_byte_msg then + addr := pop(request_msg); + push(reply_msg, mem.get(addr)); + elsif msg_type = flash_erase_msg then + addr := pop(request_msg); + len := pop(request_msg); + mem.erase(addr, len); + elsif msg_type = flash_set_clqv_msg then + len := pop(request_msg); + clqv <= len * 1 ps; + info(vc_logger, "tCLQV set to " & to_string(len * 1 ps)); + else + unexpected_msg_type(msg_type); + end if; + + reply(net, request_msg, reply_msg); + end loop; + end process; + +end model; diff --git a/hdl/ip/vhd/vunit_components/spi_nor_target/spi_nor_target_vc_pkg.vhd b/hdl/ip/vhd/vunit_components/spi_nor_target/spi_nor_target_vc_pkg.vhd new file mode 100644 index 00000000..5372e866 --- /dev/null +++ b/hdl/ip/vhd/vunit_components/spi_nor_target/spi_nor_target_vc_pkg.vhd @@ -0,0 +1,227 @@ +-- This Source Code Form is subject to the terms of the Mozilla Public +-- License, v. 2.0. If a copy of the MPL was not distributed with this +-- file, You can obtain one at https://mozilla.org/MPL/2.0/. +-- +-- Message definitions and backing store for the SPI NOR flash target VC. +-- +-- The VC models a Winbond W25Q-family QSPI NOR flash closely enough to check +-- both data integrity and the AC timing relationships that bound how fast the +-- controller's sclk can run. The interesting knob for margin testing is +-- set_output_valid_delay(), which walks the modelled tCLQV so a testbench can +-- prove the controller samples correctly across the whole datasheet range +-- rather than just at one nominal value. + +library ieee; + use ieee.std_logic_1164.all; + use ieee.numeric_std.all; + +library vunit_lib; + context vunit_lib.vunit_context; + context vunit_lib.com_context; + +package spi_nor_target_vc_pkg is + + -- Size of the modelled memory window. The real part is 1Gbit, which we + -- have no interest in allocating, so reads outside the window return + -- erased (0xFF) data and writes outside it are dropped. + constant flash_window_bytes : natural := 16#10000#; + + -- Deterministic address -> data mapping used by fill_pattern. Multiplying + -- the low address byte by an odd constant makes this a bijection over any + -- aligned 256 byte run, so an off-by-one address or a dropped bit always + -- shows up as a different byte rather than aliasing to the same value. + function pattern_byte (addr : natural) return std_logic_vector; + + type flash_mem_t is protected + + procedure set (addr : natural; data : std_logic_vector(7 downto 0)); + impure function get (addr : natural) return std_logic_vector; + procedure erase (addr : natural; len : natural); + procedure fill_pattern; + + end protected; + + -- Message types + constant flash_fill_pattern_msg : msg_type_t := new_msg_type("flash_fill_pattern"); + constant flash_write_byte_msg : msg_type_t := new_msg_type("flash_write_byte"); + constant flash_read_byte_msg : msg_type_t := new_msg_type("flash_read_byte"); + constant flash_erase_msg : msg_type_t := new_msg_type("flash_erase"); + constant flash_set_clqv_msg : msg_type_t := new_msg_type("flash_set_clqv"); + + -- Fill the whole modelled window with pattern_byte(addr). + procedure fill_pattern ( + signal net : inout network_t; + constant actor : actor_t + ); + + procedure write_flash_byte ( + signal net : inout network_t; + constant actor : actor_t; + constant addr : natural; + constant data : std_logic_vector(7 downto 0) + ); + + procedure read_flash_byte ( + signal net : inout network_t; + constant actor : actor_t; + constant addr : natural; + variable data : out std_logic_vector(7 downto 0) + ); + + procedure erase_flash ( + signal net : inout network_t; + constant actor : actor_t; + constant addr : natural; + constant len : natural + ); + + -- Override the modelled clock-low-to-output-valid delay (tCLQV). Used to + -- sweep the controller's rx sample point margin at a fixed sclk rate. + procedure set_output_valid_delay ( + signal net : inout network_t; + constant actor : actor_t; + constant delay : time + ); + +end package; + +package body spi_nor_target_vc_pkg is + + function pattern_byte (addr : natural) return std_logic_vector is + + variable a : unsigned(31 downto 0); + variable v : unsigned(7 downto 0); + + begin + a := to_unsigned(addr, 32); + v := resize(a(7 downto 0) * 7, 8) xor + a(15 downto 8) xor + resize(a(23 downto 16) * 3, 8) xor + x"A5"; + return std_logic_vector(v); + end function; + + type flash_mem_t is protected body + + type mem_arr_t is array (0 to flash_window_bytes - 1) of std_logic_vector(7 downto 0); + + variable mem : mem_arr_t := (others => x"FF"); + + procedure set (addr : natural; data : std_logic_vector(7 downto 0)) is + begin + if addr < flash_window_bytes then + -- Real NOR can only clear bits on a program, it cannot set + -- them. Modelling that catches a missing erase. + mem(addr) := mem(addr) and data; + end if; + end procedure; + + impure function get (addr : natural) return std_logic_vector is + begin + if addr < flash_window_bytes then + return mem(addr); + else + return x"FF"; + end if; + end function; + + procedure erase (addr : natural; len : natural) is + begin + for i in addr to addr + len - 1 loop + if i < flash_window_bytes then + mem(i) := x"FF"; + end if; + end loop; + end procedure; + + procedure fill_pattern is + begin + for i in mem'range loop + mem(i) := pattern_byte(i); + end loop; + end procedure; + + end protected body; + + procedure fill_pattern ( + signal net : inout network_t; + constant actor : actor_t + ) is + + variable request_msg : msg_t := new_msg(flash_fill_pattern_msg); + variable reply_msg : msg_t; + + begin + request(net, actor, request_msg, reply_msg); + delete(reply_msg); + end procedure; + + procedure write_flash_byte ( + signal net : inout network_t; + constant actor : actor_t; + constant addr : natural; + constant data : std_logic_vector(7 downto 0) + ) is + + variable request_msg : msg_t := new_msg(flash_write_byte_msg); + variable reply_msg : msg_t; + + begin + push(request_msg, addr); + push(request_msg, data); + request(net, actor, request_msg, reply_msg); + delete(reply_msg); + end procedure; + + procedure read_flash_byte ( + signal net : inout network_t; + constant actor : actor_t; + constant addr : natural; + variable data : out std_logic_vector(7 downto 0) + ) is + + variable request_msg : msg_t := new_msg(flash_read_byte_msg); + variable reply_msg : msg_t; + + begin + push(request_msg, addr); + request(net, actor, request_msg, reply_msg); + data := pop(reply_msg); + delete(reply_msg); + end procedure; + + procedure erase_flash ( + signal net : inout network_t; + constant actor : actor_t; + constant addr : natural; + constant len : natural + ) is + + variable request_msg : msg_t := new_msg(flash_erase_msg); + variable reply_msg : msg_t; + + begin + push(request_msg, addr); + push(request_msg, len); + request(net, actor, request_msg, reply_msg); + delete(reply_msg); + end procedure; + + procedure set_output_valid_delay ( + signal net : inout network_t; + constant actor : actor_t; + constant delay : time + ) is + + variable request_msg : msg_t := new_msg(flash_set_clqv_msg); + variable reply_msg : msg_t; + + begin + -- Passed as an integer count of ps rather than a time so we don't + -- depend on queue_pkg's time overloads being present. + push(request_msg, delay / 1 ps); + request(net, actor, request_msg, reply_msg); + delete(reply_msg); + end procedure; + +end package body; diff --git a/hdl/projects/cosmo_seq/cosmo_seq_pins.xdc b/hdl/projects/cosmo_seq/cosmo_seq_pins.xdc index c34a107f..56d12080 100644 --- a/hdl/projects/cosmo_seq/cosmo_seq_pins.xdc +++ b/hdl/projects/cosmo_seq/cosmo_seq_pins.xdc @@ -226,7 +226,7 @@ set_property -dict { PACKAGE_PIN N1 IOSTANDARD LVCMOS18 } [get_ports { spi0_sp5_ set_property -dict { PACKAGE_PIN P3 IOSTANDARD LVCMOS18 } [get_ports { spi1_sp5_to_fpga1_cs_l }]; set_property -dict { PACKAGE_PIN P2 IOSTANDARD LVCMOS18 } [get_ports { spi2_sp5_to_fpga1_cs_l }]; set_property -dict { PACKAGE_PIN E15 IOSTANDARD LVCMOS33 SLEW FAST} [get_ports { spi_fpga1_to_flash_clk }]; -set_property -dict { PACKAGE_PIN C17 IOSTANDARD LVCMOS33 } [get_ports { spi_fpga1_to_flash_cs_l }]; +set_property -dict { PACKAGE_PIN C17 IOSTANDARD LVCMOS33 SLEW FAST} [get_ports { spi_fpga1_to_flash_cs_l }]; set_property -dict { PACKAGE_PIN E16 IOSTANDARD LVCMOS33 SLEW FAST} [get_ports { spi_fpga1_to_flash_dat[0] }]; set_property -dict { PACKAGE_PIN B14 IOSTANDARD LVCMOS33 SLEW FAST} [get_ports { spi_fpga1_to_flash_dat[1] }]; set_property -dict { PACKAGE_PIN A14 IOSTANDARD LVCMOS33 SLEW FAST} [get_ports { spi_fpga1_to_flash_dat[2] }]; diff --git a/hdl/projects/cosmo_seq/cosmo_timing.xdc b/hdl/projects/cosmo_seq/cosmo_timing.xdc index 92a42b73..74661558 100644 --- a/hdl/projects/cosmo_seq/cosmo_timing.xdc +++ b/hdl/projects/cosmo_seq/cosmo_timing.xdc @@ -171,4 +171,116 @@ set_false_path -from [get_ports {*}] -to [get_ports {fpga1_spare_v1p8[*]}] # This is a stop-gap to provide some kind of output timing constraints per the eSPI base spec set_max_delay -to [get_ports espi0_sp5_to_fpga1_dat[*]] 6 -set_min_delay -to [get_ports espi0_sp5_to_fpga1_dat[*]] 0 \ No newline at end of file +set_min_delay -to [get_ports espi0_sp5_to_fpga1_dat[*]] 0 + +# ####################### +# SPI NOR flash interface (Winbond W25Q01JV) +# ####################### +# sclk is toggled by fabric logic off clk_125m at clk/2, so 62.5MHz, a 16ns +# period with an 8ns half period. Nothing inside the FPGA is clocked by it, so +# there is deliberately no create_generated_clock here: what actually has to be +# bounded is the clock-to-data skew leaving the FPGA and the pin-to-flop delay +# coming back, and both are directly constrainable. +# +# Trace delays are short and local; using the same 6.8ns/m as the FMC block +# above. TODO: replace with the measured lengths off 913-0000023. +set flash_trace_max 0.40 +set flash_trace_min 0.10 + +# Pull the launch flops into the IOBs. Every one of these is a dedicated +# duplicate whose only load is its pin (see spi_clk_gen's sclk_pin and +# spi_txn_mgr's cs_n_pin), which is what makes packing legal. It matters a lot: +# left in the fabric the placer put them wherever it liked and measured 12 to 13 +# ns of routing to the pin, which both blew the clock-to-data skew budget and +# pushed the read round trip past every available sample point. In the IOB the +# delay is small, deterministic, and the same for all four. +set_property IOB TRUE [get_cells -hier -filter {NAME =~ *spi_nor_top_inst/link/clk_gen/sclk_pin_reg}] +set_property IOB TRUE [get_cells -hier -filter {NAME =~ *spi_nor_top_inst/spi_txn_mgr_inst/cs_n_pin_reg}] +set_property IOB TRUE [get_cells -hier -filter {NAME =~ *spi_nor_top_inst/link/io_o_reg[*]}] +set_property IOB TRUE [get_cells -hier -filter {NAME =~ *spi_nor_top_inst/link/io_oe_reg[*]}] +set_property IOB TRUE [get_cells -hier -filter {NAME =~ *spi_nor_top_inst/link/io_cap_*_reg[*]}] + +# ################# +# Outputs: sclk, and dat[] during the instruction, address and write phases. +# cs_n is handled separately below. +# +# The part samples mosi on the sclk rising edge, and the FPGA launches both mosi +# and the sclk falling edge from the same clk edge. So the flash sees a full half +# period of setup, less whatever skew the IOBs and routing add between the clock +# pin and the data pins: +# +# skew_budget = half_period - tDVCH = 8.0 - 2.0 = 6.0 ns +# +# Constraining all of these pins into one delay window makes the worst-case skew +# the difference between the two bounds, which is the quantity that matters here: +# the same clock insertion delay applies to every one of these launch flops, so it +# cancels out of the skew and only the window width has to fit the budget. +# +# Note the window looks wide (4.5ns against a 6ns budget) for four pins that are +# all IOB-packed and launched off the same clk edge. That is because max and min +# delay checks compare the slow corner of one path against the fast corner of +# another, so most of the width is process/voltage/temperature spread rather than +# pin-to-pin skew, which is well under a nanosecond here. The bound is a tripwire +# against a pin losing its IOB or picking up extra logic, not a skew estimate. +# +# Hold is not a concern for the part: mosi is held until the following falling +# edge, 8ns after the sampling edge, against a tCHDX of 3ns. +# +# These numbers assume the IOB packing above. Packed, the flop-to-pin delay is +# about 3.3ns and essentially all of it is logic -- 0.001ns of routing -- so a +# tight window is both meetable and meaningful. Left in the fabric the same paths +# measured 12 to 13ns of routing and varied by several ns between builds. +set_max_delay 5.0 -to [get_ports {spi_fpga1_to_flash_clk \ + spi_fpga1_to_flash_dat[*]}] +set_min_delay 0.5 -to [get_ports {spi_fpga1_to_flash_clk \ + spi_fpga1_to_flash_dat[*]}] + +# Two things are deliberately outside that window, because pulling them into it +# would make the placer work hard on paths that have an order of magnitude more +# real slack than the data pins do: +# +# cs_n only has to be low before the first sclk edge and stay low after +# the last. spi_txn_mgr spends cs_setup_cnts = 4 clk cycles, 32ns, +# on each, against tSLCH/tCHSH of 5ns. +# the tristate carries no data and only has to have settled before the part +# enable starts driving, which release_lanes gives it a full sclk cycle +# to do. +set_max_delay 16.0 -to [get_ports spi_fpga1_to_flash_cs_l] +set_max_delay 16.0 -from [get_cells -hier -filter {NAME =~ *spi_nor_top_inst/link/io_oe_reg[*]}] \ + -to [get_ports spi_fpga1_to_flash_dat[*]] + +# ################# +# Inputs: dat[] during read phases. +# +# The controller samples read data at a fixed point S after the sclk rising edge, +# set by the rx_sample_taps generic on spi_nor_top. S has to satisfy +# +# round_trip_valid - half_period <= S <= half_period + round_trip_hold +# +# where round_trip_valid is built from the part's tCLQV and round_trip_hold from +# its tCLQX. Note the upper limit comes from tCLQX, not tCLQV: sampling too late +# catches the next bit rather than the current one. +# +# With the IOB packing and the output bounds above: +# flop to sclk pin 3.30 max 1.50 min +# sclk trace 0.40 0.10 +# flash tCLQV / tCLQX 6.00 1.50 +# data trace back 0.40 0.10 +# pin to capture flop 1.50 0.50 +# ----- ----- +# round_trip_valid max 11.60 round_trip_hold min 3.70 +# +# so at an 8ns half period S has to land in 3.6 .. 11.7ns. rx_sample_taps = 2 puts +# it at 8ns, about 4ns clear of either limit. spi_nor_fast_tb and +# spi_nor_fast_quick_io_tb model these delays and check both corners. +# +# If reads are marginal on hardware, sweep rx_sample_taps before assuming anything +# else is wrong; taps are 4ns apart so 1 and 3 bracket the shipped value. +# +# -datapath_only because this is a pin to flop propagation bound, not a +# synchronous transfer: without it Vivado charges the MMCM's clock insertion +# delay against the budget and the check becomes meaningless. +# +# The dedicated capture flops in spi_link are the only loads on these pins, so +# these paths are exactly the pin-to-flop delay. +set_max_delay 3.0 -datapath_only -from [get_ports spi_fpga1_to_flash_dat[*]] \ No newline at end of file diff --git a/hdl/projects/cosmo_seq/sp5_espi_flash_subsystem/sp5_espi_flash_subsystem.vhd b/hdl/projects/cosmo_seq/sp5_espi_flash_subsystem/sp5_espi_flash_subsystem.vhd index 8154fdb3..87a33bf0 100644 --- a/hdl/projects/cosmo_seq/sp5_espi_flash_subsystem/sp5_espi_flash_subsystem.vhd +++ b/hdl/projects/cosmo_seq/sp5_espi_flash_subsystem/sp5_espi_flash_subsystem.vhd @@ -152,6 +152,21 @@ begin spi_nor_top_inst: entity work.spi_nor_top + generic map( + -- 125MHz / 2 = 62.5MHz sclk. This is as fast as the block goes off + -- clk_125m, and it is the ceiling for a fixed sample point: the + -- round trip out to the flash and back has to land within half an + -- sclk period of rx_sample_taps, and above this rate that window + -- closes. Faster would need per-lane IDELAY read training. + sclk_divisor => 0, + -- Sample 8ns after the sclk rising edge. Taps are in half-clk (4ns) + -- steps. With the flash IO flops packed into the IOBs the round trip + -- out and back is bounded to roughly 3.7..11.6ns, which puts the + -- usable sample window at 3.6..11.7ns; 8ns sits about 4ns clear of + -- either end. cosmo_timing.xdc carries the arithmetic. Sweep this on + -- hardware if reads come back corrupted. + rx_sample_taps => 2 + ) port map( clk => clk_125m, reset => reset_125m, diff --git a/hdl/projects/grapefruit/grapefruit_pins.xdc b/hdl/projects/grapefruit/grapefruit_pins.xdc index ecee16c9..730a1831 100644 --- a/hdl/projects/grapefruit/grapefruit_pins.xdc +++ b/hdl/projects/grapefruit/grapefruit_pins.xdc @@ -88,12 +88,12 @@ set_property -dict { PACKAGE_PIN C15 IOSTANDARD LVCMOS33 } [get_ports { sp_to_se set_property -dict { PACKAGE_PIN B16 IOSTANDARD LVCMOS33 } [get_ports { seq_rev_id[2] }]; set_property -dict { PACKAGE_PIN B15 IOSTANDARD LVCMOS33 } [get_ports { seq_rev_id[1] }]; set_property -dict { PACKAGE_PIN C16 IOSTANDARD LVCMOS33 } [get_ports { seq_rev_id[0] }]; -set_property -dict { PACKAGE_PIN C17 IOSTANDARD LVCMOS33 } [get_ports { spi_fpga_to_flash_cs_l }]; -set_property -dict { PACKAGE_PIN E15 IOSTANDARD LVCMOS33 } [get_ports { spi_fpga_to_flash_clk }]; -set_property -dict { PACKAGE_PIN A16 IOSTANDARD LVCMOS33 } [get_ports { spi_fpga_to_flash_dat[3] }]; -set_property -dict { PACKAGE_PIN A14 IOSTANDARD LVCMOS33 } [get_ports { spi_fpga_to_flash_dat[2] }]; -set_property -dict { PACKAGE_PIN B14 IOSTANDARD LVCMOS33 } [get_ports { spi_fpga_to_flash_dat[1] }]; -set_property -dict { PACKAGE_PIN E16 IOSTANDARD LVCMOS33 } [get_ports { spi_fpga_to_flash_dat[0] }]; +set_property -dict { PACKAGE_PIN C17 IOSTANDARD LVCMOS33 SLEW FAST} [get_ports { spi_fpga_to_flash_cs_l }]; +set_property -dict { PACKAGE_PIN E15 IOSTANDARD LVCMOS33 SLEW FAST} [get_ports { spi_fpga_to_flash_clk }]; +set_property -dict { PACKAGE_PIN A16 IOSTANDARD LVCMOS33 SLEW FAST} [get_ports { spi_fpga_to_flash_dat[3] }]; +set_property -dict { PACKAGE_PIN A14 IOSTANDARD LVCMOS33 SLEW FAST} [get_ports { spi_fpga_to_flash_dat[2] }]; +set_property -dict { PACKAGE_PIN B14 IOSTANDARD LVCMOS33 SLEW FAST} [get_ports { spi_fpga_to_flash_dat[1] }]; +set_property -dict { PACKAGE_PIN E16 IOSTANDARD LVCMOS33 SLEW FAST} [get_ports { spi_fpga_to_flash_dat[0] }]; set_property -dict { PACKAGE_PIN A18 IOSTANDARD LVCMOS33 } [get_ports { spi_fpga_to_flash2_cs_l }]; set_property -dict { PACKAGE_PIN A19 IOSTANDARD LVCMOS33 } [get_ports { spi_fpga_to_flash2_clk }]; set_property -dict { PACKAGE_PIN A20 IOSTANDARD LVCMOS33 } [get_ports { spi_fpga_to_flash2_dat[3] }]; diff --git a/hdl/projects/grapefruit/grapefruit_timing.xdc b/hdl/projects/grapefruit/grapefruit_timing.xdc index f6c31359..395863df 100644 --- a/hdl/projects/grapefruit/grapefruit_timing.xdc +++ b/hdl/projects/grapefruit/grapefruit_timing.xdc @@ -69,4 +69,36 @@ set_multicycle_path -from [get_pins {stm32h7_fmc_target_inst/data_out*/C}] -to [ set_multicycle_path -from [get_pins {stm32h7_fmc_target_inst/data_out_en_reg*/C}] -to [get_ports {fmc_sp_to_fpga_da[*]}] -setup 2 set_multicycle_path -from [get_pins {stm32h7_fmc_target_inst/data_out_en_reg*/C}] -to [get_ports {fmc_sp_to_fpga_da[*]}] -hold 1 +# ####################### +# SPI NOR flash interface (Winbond W25Q01JV) +# ####################### +# sclk is fabric-toggled off clk_125m at clk/2, so 62.5MHz with an 8ns half +# period. See cosmo_timing.xdc for the full derivation; the short version is that +# the outputs are bound to a common delay window so the clock-to-data skew at the +# part stays inside half_period - tDVCH = 6ns, and the return path is bound so +# the round trip stays inside the sample window that rx_sample_taps picks. +# +# TODO: replace the trace numbers with measured lengths for this board. + +# Launch and capture flops into the IOBs; see cosmo_timing.xdc for why this is +# load-bearing rather than a nicety. +set_property IOB TRUE [get_cells -hier -filter {NAME =~ *spi_nor_top_inst/link/clk_gen/sclk_pin_reg}] +set_property IOB TRUE [get_cells -hier -filter {NAME =~ *spi_nor_top_inst/spi_txn_mgr_inst/cs_n_pin_reg}] +set_property IOB TRUE [get_cells -hier -filter {NAME =~ *spi_nor_top_inst/link/io_o_reg[*]}] +set_property IOB TRUE [get_cells -hier -filter {NAME =~ *spi_nor_top_inst/link/io_oe_reg[*]}] +set_property IOB TRUE [get_cells -hier -filter {NAME =~ *spi_nor_top_inst/link/io_cap_*_reg[*]}] + +set_max_delay 5.0 -to [get_ports {spi_fpga_to_flash_clk \ + spi_fpga_to_flash_dat[*]}] +set_min_delay 0.5 -to [get_ports {spi_fpga_to_flash_clk \ + spi_fpga_to_flash_dat[*]}] + +# cs_n and the tristate enable are exempted from the data window; both have an +# order of magnitude more real slack than the data pins. See cosmo_timing.xdc. +set_max_delay 16.0 -to [get_ports spi_fpga_to_flash_cs_l] +set_max_delay 16.0 -from [get_cells -hier -filter {NAME =~ *spi_nor_top_inst/link/io_oe_reg[*]}] \ + -to [get_ports spi_fpga_to_flash_dat[*]] + +set_max_delay 3.0 -datapath_only -from [get_ports spi_fpga_to_flash_dat[*]] + diff --git a/hdl/projects/grapefruit/grapefruit_top.vhd b/hdl/projects/grapefruit/grapefruit_top.vhd index 5d70aad9..c0eaf1a1 100644 --- a/hdl/projects/grapefruit/grapefruit_top.vhd +++ b/hdl/projects/grapefruit/grapefruit_top.vhd @@ -338,6 +338,12 @@ begin resize_axil(fabric_responders(1), responders_8b(1)); spi_nor_top_inst: entity work.spi_nor_top + generic map( + -- 125MHz / 2 = 62.5MHz sclk; see sp5_espi_flash_subsystem for why this + -- is the ceiling and how rx_sample_taps relates to the board round trip. + sclk_divisor => 0, + rx_sample_taps => 2 + ) port map( clk => clk_125m, reset => reset_125m, From e048697d344ae445d4700c0e502a0b6b00e1b67c Mon Sep 17 00:00:00 2001 From: Nathanael Huffman Date: Thu, 6 Aug 2026 12:15:26 -0400 Subject: [PATCH 2/4] Increase timing margin and fix a number of implementation bugs discovered when testing in hardware --- .../spi_nor_controller/link/spi_clk_gen.vhd | 12 +- .../vhd/spi_nor_controller/link/spi_link.vhd | 99 +++++++----- .../sims/spi_nor_fast_quick_io_tb.vhd | 4 + .../sims/spi_nor_fast_tb.vhd | 7 + .../spi_nor_controller/sims/spi_nor_tb.vhd | 4 + .../sims/spi_nor_tb_pkg.vhd | 96 ++++++++++++ hdl/ip/vhd/spi_nor_controller/spi_nor_top.vhd | 6 + .../spi_txn/spi_txn_mgr.vhd | 146 +++++++++++++++--- .../spi_nor_target/spi_nor_target_vc.vhd | 50 +++++- 9 files changed, 357 insertions(+), 67 deletions(-) diff --git a/hdl/ip/vhd/spi_nor_controller/link/spi_clk_gen.vhd b/hdl/ip/vhd/spi_nor_controller/link/spi_clk_gen.vhd index 6b0e6698..e11fa293 100644 --- a/hdl/ip/vhd/spi_nor_controller/link/spi_clk_gen.vhd +++ b/hdl/ip/vhd/spi_nor_controller/link/spi_clk_gen.vhd @@ -38,15 +38,23 @@ architecture rtl of spi_clk_gen is signal internal_enable : boolean := false; signal enable_last : boolean := false; signal sclk_int : std_logic := '0'; + signal running : boolean; begin sclk <= sclk_int; + -- internal_enable is registered, so on its own it lingers for a cycle after + -- enable drops -- long enough to emit one more toggle. For a command that + -- ends on an exact bit count, an erase or a program, that stray edge is a + -- whole extra bit and the part throws the instruction away. Qualifying with + -- the live enable stops the clock on the same edge the caller asked it to. + running <= internal_enable and enable; + -- strobe is registered, so it is already asserted during the cycle that -- precedes the toggling edge. That makes this safe to use as a synchronous -- enable by anything that needs to change state exactly when sclk does. - sclk_fall_now <= internal_enable and strobe and sclk_int = '1'; + sclk_fall_now <= running and strobe and sclk_int = '1'; -- Pretty simple spi generator. -- start with a rising edge @@ -87,7 +95,7 @@ begin internal_enable <= false; end if; - if internal_enable then + if running then nxt_sclk := sclk_int; if strobe then nxt_sclk := not sclk_int; diff --git a/hdl/ip/vhd/spi_nor_controller/link/spi_link.vhd b/hdl/ip/vhd/spi_nor_controller/link/spi_link.vhd index 173a1cc0..c03edd0a 100644 --- a/hdl/ip/vhd/spi_nor_controller/link/spi_link.vhd +++ b/hdl/ip/vhd/spi_nor_controller/link/spi_link.vhd @@ -26,16 +26,25 @@ entity spi_link is divisor : in unsigned(15 downto 0); in_tx_phases : in boolean; in_rx_phases : in boolean; + -- Runs the sclk generator. Not the same as in_tx_phases: the bus is + -- still driven while cs_n is being torn down, but the part must not see + -- any clock edges there. + sclk_running : in boolean; -- Lanes to stop driving early, ahead of a controller-to-flash -- turnaround, so the two ends are never enabled at once release_lanes : in std_logic_vector(3 downto 0); rx_byte : out std_logic_vector(7 downto 0); rx_byte_done : out boolean; + -- The next byte to shift out. Must be held ready ahead of the reload + -- edge rather than produced in response to tx_byte_req: it lands + -- directly on the io_o launch flops, so anything combinational behind it + -- is in series with the shifter's own empty detect. tx_byte : in std_logic_vector(7 downto 0); - -- Asserted the cycle before the edge on which a new tx byte is - -- consumed. The transaction manager uses this both to advance its phase - -- and to present the next byte, so that the byte and the sclk edge that - -- launches it move together. + -- The io mode tx_byte belongs to, adopted at the reload edge + tx_byte_mode : in io_mode; + -- Asserted during the cycle whose clk edge consumes a tx byte. The + -- transaction manager advances its phase on this, so the byte and the + -- sclk edge that launches it move together. tx_byte_req : out boolean; sclk_redge : out boolean; sclk_fedge : out boolean; @@ -160,7 +169,7 @@ begin clk => clk, reset => reset, divisor => divisor, - enable => in_tx_phases or in_rx_phases, + enable => sclk_running, sclk => sclk_int, sclk_pin => sclk_pin, sclk_fall_now => sclk_fall_now @@ -183,12 +192,15 @@ begin variable cs_n_assert_edge : boolean := false; variable nxt_tx_reg : std_logic_vector(8 downto 0); variable nxt_mode : io_mode; + variable oe_mode : io_mode; + variable oe : std_logic_vector(3 downto 0); begin if reset then tx_reg <= (others => '0'); csn_last <= '1'; cur_io_mode <= single; io_o <= (others => '1'); + io_oe <= (others => '0'); elsif rising_edge(clk) then csn_last <= cs_n; cs_n_assert_edge := cs_n = '0' and csn_last = '1'; @@ -196,11 +208,17 @@ begin -- The io mode only changes on a byte boundary, which is also when -- the shifter reloads, so the mode that applies to the bits going -- out at this edge is the one selected here. + -- + -- req_io_mode comes off the registered transaction state, which has + -- not advanced yet on the edge that reloads the shifter. That is + -- fine while we are mid-phase, and it is what keeps reads right + -- (their phase change lands on a rising edge, away from any reload), + -- but the byte being loaded here may belong to the *next* phase. So + -- a reload takes the mode that travelled with the byte instead. nxt_mode := cur_io_mode; if (cs_n = '0' and sclk_fall_now) or cs_n = '1' then nxt_mode := req_io_mode; end if; - cur_io_mode <= nxt_mode; nxt_tx_reg := tx_reg; @@ -208,11 +226,13 @@ begin -- as the controller here, we need to pre-load data before the first -- clock nxt_tx_reg := tx_byte & '1'; + nxt_mode := tx_byte_mode; elsif in_tx_phases and sclk_fall_now then if shift_left(tx_reg, shift_amt) = SENTINEL_AT_TOP then -- tx_register is "empty" load a new one -- and the sentinel value nxt_tx_reg := tx_byte & '1'; + nxt_mode := tx_byte_mode; else nxt_tx_reg := shift_left(tx_reg, shift_amt); end if; @@ -220,46 +240,39 @@ begin nxt_tx_reg := (others => '0'); end if; - tx_reg <= nxt_tx_reg; - io_o <= io_out_bits(nxt_tx_reg, nxt_mode); - end if; - end process; + cur_io_mode <= nxt_mode; - -- Flag the reload a cycle ahead of the edge that consumes the byte so the - -- transaction manager can present the right one. This is the same condition - -- the serializer uses above, just not yet registered. - tx_byte_req <= in_tx_phases and sclk_fall_now and - shift_left(tx_reg, shift_amt) = SENTINEL_AT_TOP; + -- Direction is decided here, alongside the data, so the two can + -- never disagree about which mode is in force. Computing it in its + -- own process off req_io_mode put the quad lanes' output enable one + -- clk behind the first quad data nibble, which at clk/2 is the edge + -- the part samples on. + -- + -- Turning *on* follows nxt_mode so it lands with the data; turning + -- *off* follows req_io_mode, which moves as soon as the phase does, + -- because releasing early is what keeps us clear of the part on a + -- read turnaround. + if in_tx_phases then + oe_mode := nxt_mode; + else + oe_mode := req_io_mode; + end if; - -- Based on state and qspi mode, deal with the tri-state controls - -- of the spi pins - -- Note this tracks req_io_mode, not the byte-aligned cur_io_mode. The - -- latched mode is deliberately held until a falling edge so a byte is never - -- split across two widths, but that leaves it reading `single` for the first - -- cycles of a dual/quad read -- long enough to re-enable io3 for HOLD - -- avoidance just as the part starts driving it. Direction has no reason to - -- wait for a byte boundary, so it follows the transaction directly. - oe_control: process(clk, reset) - variable oe : std_logic_vector(3 downto 0); - begin - if reset then - io_oe <= (others => '0'); - elsif rising_edge(clk) then if in_tx_phases then - case req_io_mode is + case oe_mode is when single => -- data going out 0 port, but need 3 port to be high so -- chip doesn't see a HOLD operation oe := (0 => '1', 3 => '1', others => '0'); when dual => - -- data going out 0 port, but need 3 port to be high so - -- chip doesn't see a HOLD operation + -- data going out 0 and 1 ports, 3 port high so the chip + -- doesn't see a HOLD operation oe := (1 downto 0 => '1', 3 => '1', others => '0'); when quad => oe := (others => '1'); end case; else -- rx only in all rx phases - case req_io_mode is + case oe_mode is when single => -- data coming in 1 port, but need 3 port to be high so -- chip doesn't see a HOLD operation @@ -267,17 +280,33 @@ begin when dual => -- data coming in 0, 1 ports, but need 3 port to be high so -- chip doesn't see a HOLD operation - oe := ( 3 => '1', others => '0'); + oe := (3 => '1', others => '0'); when quad => -- data coming in all ports, no outputs oe := (others => '0'); end case; end if; - io_oe <= oe and not release_lanes; + tx_reg <= nxt_tx_reg; + io_o <= io_out_bits(nxt_tx_reg, nxt_mode); + io_oe <= oe and not release_lanes; end if; end process; + -- Flag the reload a cycle ahead of the edge that consumes the byte so the + -- transaction manager can present the right one. This is the same condition + -- the serializer uses above, just not yet registered. + tx_byte_req <= in_tx_phases and sclk_fall_now and + shift_left(tx_reg, shift_amt) = SENTINEL_AT_TOP; + + -- Based on state and qspi mode, deal with the tri-state controls + -- of the spi pins + -- Note this tracks req_io_mode, not the byte-aligned cur_io_mode. The + -- latched mode is deliberately held until a falling edge so a byte is never + -- split across two widths, but that leaves it reading `single` for the first + -- cycles of a dual/quad read -- long enough to re-enable io3 for HOLD + -- avoidance just as the part starts driving it. Direction has no reason to + -- wait for a byte boundary, so it follows the transaction directly. -- Input capture. Two flops, one per clk phase, so the sample point can be -- placed on a half-clk grid without needing a faster clock. capture_p: process(clk) diff --git a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_quick_io_tb.vhd b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_quick_io_tb.vhd index fafe9b02..da48a5c9 100644 --- a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_quick_io_tb.vhd +++ b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_quick_io_tb.vhd @@ -67,6 +67,10 @@ begin check_program_readback(net, flash, 16#2000#); elsif run("back_to_back_reads") then check_back_to_back_reads(net, 16#400#, 16#500#, 32); + elsif run("block_erase") then + check_block_erase(net, flash, 16#30000#); + elsif run("quad_page_program") then + check_quad_page_program(net, flash, 16#5000#); elsif run("output_delay_margin") then for clqv_ns in 2 to 7 loop info("checking with tCLQV = " & to_string(clqv_ns) & " ns"); diff --git a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_tb.vhd b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_tb.vhd index aec8c987..200dfa9e 100644 --- a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_tb.vhd +++ b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_fast_tb.vhd @@ -66,6 +66,13 @@ begin check_program_readback(net, flash, 16#2000#); elsif run("back_to_back_reads") then check_back_to_back_reads(net, 16#400#, 16#500#, 32); + elsif run("block_erase") then + -- Commands with an address but no data phase leave the sclk + -- generator enabled a moment longer than the part allows if + -- the drive and clock-enable conditions are conflated. + check_block_erase(net, flash, 16#30000#); + elsif run("quad_page_program") then + check_quad_page_program(net, flash, 16#5000#); elsif run("output_delay_margin") then -- Sweep the part's clock-low-to-output-valid delay across the -- datasheet range and well beyond it, and require correct data diff --git a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb.vhd b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb.vhd index 3662c867..3cf94b66 100644 --- a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb.vhd +++ b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb.vhd @@ -61,6 +61,10 @@ begin check_program_readback(net, flash, 16#2000#); elsif run("back_to_back_reads") then check_back_to_back_reads(net, 16#400#, 16#500#, 32); + elsif run("block_erase") then + check_block_erase(net, flash, 16#30000#); + elsif run("quad_page_program") then + check_quad_page_program(net, flash, 16#5000#); elsif run("write_24addr_no_dummy") then write_data(net, x"03020100"); -- do do words write_data(net, x"07060504"); -- do do words diff --git a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb_pkg.vhd b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb_pkg.vhd index 2c0476fe..ca31c41e 100644 --- a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb_pkg.vhd +++ b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb_pkg.vhd @@ -104,6 +104,28 @@ package spi_nor_tb_pkg is constant count : natural ); + -- Erase a block and prove the part actually did it. Commands with an address + -- but no data phase are the ones that expose trailing-clock bugs: the part + -- discards the instruction rather than reporting anything, so nothing but a + -- readback notices. + procedure check_block_erase ( + signal net : inout network_t; + constant flash : actor_t; + constant addr : natural + ); + + -- A full 256 byte page programmed in quad mode, which is exactly what the + -- hubris driver does. This is the hardest thing the tx path has to do: quad + -- data is two sclk cycles per byte, so at clk/2 a byte leaves every four clk + -- cycles, and every fourth byte crosses a 32 bit boundary in the tx FIFO. + -- A short single-mode write has eight times the slack and proves nothing + -- about it. + procedure check_quad_page_program ( + signal net : inout network_t; + constant flash : actor_t; + constant addr : natural + ); + end package; package body spi_nor_tb_pkg is @@ -295,6 +317,80 @@ package body spi_nor_tb_pkg is end loop; end; + procedure check_block_erase ( + signal net : inout network_t; + constant flash : actor_t; + constant addr : natural + ) is + + variable got : std_logic_vector(7 downto 0); + + begin + -- Put something other than 0xFF in the way so a no-op erase cannot pass + for i in 0 to 7 loop + write_flash_byte(net, flash, addr + i, pattern_byte(addr + i)); + end loop; + + clear_fifos(net); + write_dummy(net, 0); + write_data_size(net, 0); + + -- Issue the write enable the way the driver does. It carries neither an + -- address nor data, so it is the shortest command the block emits and + -- the least forgiving about trailing clocks -- and every erase and + -- program on real hardware is preceded by one, so a discarded write + -- enable looks exactly like a discarded erase. + write_instr(net, WRITE_ENABLE_OP); + wait_txn_done(net); + + write_addr(net, To_StdLogicVector(addr, 32)); + write_instr(net, BLOCK_ERASE_64K_4BYTE_OP); + wait_txn_done(net); + + -- The model only applies the erase if the instruction was well formed, + -- so this catches a discarded command as well as a wrong address. + for i in 0 to 7 loop + read_flash_byte(net, flash, addr + i, got); + check_equal(got, std_logic_vector'(x"FF"), + "block erase left byte " & to_string(i) & " unerased"); + end loop; + end; + + procedure check_quad_page_program ( + signal net : inout network_t; + constant flash : actor_t; + constant addr : natural + ) is + + variable word : std_logic_vector(31 downto 0); + variable got : std_logic_vector(7 downto 0); + + begin + clear_fifos(net); + erase_flash(net, flash, addr, 256); + + write_dummy(net, 0); + write_data_size(net, 256); + write_addr(net, To_StdLogicVector(addr, 32)); + + -- 64 words fill the tx FIFO exactly, the same way the driver does it + for w in 0 to 63 loop + for b in 0 to 3 loop + word(8 * b + 7 downto 8 * b) := pattern_byte(addr + 4 * w + b); + end loop; + write_data(net, word); + end loop; + + write_instr(net, QUAD_INPUT_PAGE_PROGRAM_4BYTE_OP); + wait_txn_done(net); + + for i in 0 to 255 loop + read_flash_byte(net, flash, addr + i, got); + check_equal(got, pattern_byte(addr + i), + "quad page program mismatch at offset " & to_string(i)); + end loop; + end; + procedure check_back_to_back_reads ( signal net : inout network_t; constant addr_a : natural; diff --git a/hdl/ip/vhd/spi_nor_controller/spi_nor_top.vhd b/hdl/ip/vhd/spi_nor_controller/spi_nor_top.vhd index dd6cdd08..cf5dd8df 100644 --- a/hdl/ip/vhd/spi_nor_controller/spi_nor_top.vhd +++ b/hdl/ip/vhd/spi_nor_controller/spi_nor_top.vhd @@ -58,6 +58,7 @@ architecture rtl of spi_nor_top is signal in_rx_phases : boolean; signal in_tx_phases : boolean; signal release_lanes : std_logic_vector(3 downto 0); + signal sclk_running : boolean; -- Internal copies of sclk and cs_n. The versions that leave the block are -- duplicate flops with no internal fanout so they can be packed into the -- IOBs; these are what the phase logic looks at. @@ -65,6 +66,7 @@ architecture rtl of spi_nor_top is signal cs_n_internal : std_logic; signal link_rx_byte : std_logic_vector(7 downto 0); signal link_tx_byte : std_logic_vector(7 downto 0); + signal link_tx_mode : io_mode; signal cur_io_mode : io_mode; signal rx_fifo_write8 : std_logic; signal tx_fifo_read8 : std_logic; @@ -116,10 +118,12 @@ begin divisor => div_val, in_tx_phases => in_tx_phases, in_rx_phases => in_rx_phases, + sclk_running => sclk_running, release_lanes => release_lanes, rx_byte => link_rx_byte, rx_byte_done => rx_byte_done, tx_byte => link_tx_byte, + tx_byte_mode => link_tx_mode, tx_byte_req => tx_byte_req, sclk_redge => open, sclk_fedge => open, @@ -153,8 +157,10 @@ begin rx_link_byte => link_rx_byte, tx_byte_req => tx_byte_req, tx_link_byte => link_tx_byte, + tx_link_mode => link_tx_mode, in_rx_phases => in_rx_phases, in_tx_phases => in_tx_phases, + sclk_running => sclk_running, release_lanes => release_lanes, cur_io_mode => cur_io_mode, tx_fifo_ack => tx_fifo_read8, diff --git a/hdl/ip/vhd/spi_nor_controller/spi_txn/spi_txn_mgr.vhd b/hdl/ip/vhd/spi_nor_controller/spi_txn/spi_txn_mgr.vhd index f11882df..aff99f20 100644 --- a/hdl/ip/vhd/spi_nor_controller/spi_txn/spi_txn_mgr.vhd +++ b/hdl/ip/vhd/spi_nor_controller/spi_txn/spi_txn_mgr.vhd @@ -38,8 +38,14 @@ entity spi_txn_mgr is rx_link_byte : in std_logic_vector(7 downto 0); tx_byte_req : in boolean; tx_link_byte : out std_logic_vector(7 downto 0); + -- The io mode the byte above belongs to. Travels with it because the + -- serializer needs both on the same edge; see next_tx. + tx_link_mode : out io_mode; in_rx_phases : out boolean; in_tx_phases : out boolean; + -- Gates the sclk generator. Narrower than in_tx_phases on purpose, see + -- is_sclk_running below. + sclk_running : out boolean; -- Lanes to stop driving ahead of a turnaround, see below release_lanes : out std_logic_vector(3 downto 0); cur_io_mode : out io_mode; @@ -74,6 +80,15 @@ architecture rtl of spi_txn_mgr is attribute mark_debug of r : signal is "TRUE"; signal sclk_last : std_logic; + -- The byte the serializer will load at its next reload, and the io mode it + -- belongs to, held ready ahead of the edge that consumes them. See next_tx. + type tx_prefetch_t is record + byte : std_logic_vector(7 downto 0); + mode : io_mode; + end record; + + signal tx_pre : tx_prefetch_t; + -- Some helper functions -- This block gets the expected IO mode given the state and the transaction we're running function get_cur_io_mode ( @@ -100,6 +115,20 @@ architecture rtl of spi_txn_mgr is end if; end; + -- The phases sclk is allowed to run in. This is deliberately *not* the same + -- question as "are we driving the bus": cs_deassert drives but must not + -- clock. The part counts sclk edges to decide where an instruction ends, and + -- an erase or a write enable that gets even one trailing edge before cs_n + -- rises is discarded outright, with no error anywhere -- it just silently + -- does not happen. Conflating the two conditions is what broke erase. + function is_sclk_running ( + state: state_t + ) return boolean is + begin + return state = cs_assert or state = instruction or state = addr or + state = dummy or state = wdata or state = rdata; + end; + -- This function takes the state and returns if we're driving data lines -- currently. -- @@ -128,15 +157,91 @@ architecture rtl of spi_txn_mgr is return state = rdata; end; + -- The byte the serializer will load at its *next* reload. + -- + -- The serializer reloads on the same clk edge this state machine advances a + -- phase, so the byte has to already be sitting there when that edge arrives + -- rather than be selected by it. Driving the mux from `v` satisfied that by + -- hanging the entire selection off tx_reg in one combinational path: + -- shifter-empty detect, then the next-state decode, then a byte select off + -- the decremented counter, then the lane mux into io_o. At clk/6 there was + -- room for it; at clk/2 it was the design's critical path, eight levels of + -- LUT spending 6.9ns of an 8ns period to reach io_o. + -- + -- *Which* byte comes next does not depend on *when* the request arrives, + -- though -- it is a function of the registered state alone. So compute it + -- from `r` and register it, leaving the reload edge nothing to do but copy a + -- flop. The prefetch settles two clks after each phase advance and byte + -- slots are at least four clks apart (quad data at clk/2, the fastest this + -- block runs), so it is always in place in time. + -- The prefetch carries the io mode with the byte, and it has to: the mode is + -- a property of the phase the byte belongs to, and the serializer needs both + -- at the same instant. Taking the mode from the registered state instead + -- means the first data byte of a dual or quad write is loaded while the + -- state still reads `addr`, so it goes out with single-bit lane assignment + -- and, worse, gets shifted by one instead of four. That leaves the shifter's + -- sentinel on an odd bit where the byte-complete compare can never match it, + -- so the byte counter stops advancing and the transaction never ends. + function next_tx ( + r : reg_type; + cmd : spi_nor_cmd_t; + fifo : std_logic_vector(7 downto 0) + ) return tx_prefetch_t is + variable idx : integer range 0 to 3; + begin + case r.state is + when idle => + -- The opcode is the first byte out. The serializer preloads it + -- on the cs_n assert edge, before the first sclk edge. + return (byte => cmd.instr, mode => single); + when cs_assert | instruction => + -- Whatever follows the opcode: the top address byte, or the + -- first data byte for an opcode that writes without an address. + case r.txn.addr_kind is + when bit32 => + return (byte => cmd.addr(31 downto 24), mode => single); + when bit24 => + return (byte => cmd.addr(23 downto 16), mode => single); + when none => + if not r.txn.uses_dummys and r.txn.data_kind = write then + return (byte => fifo, mode => r.txn.data_mode); + end if; + end case; + when addr => + if r.counter > 0 then + idx := r.counter - 1; + return (byte => cmd.addr(8 * idx + 7 downto 8 * idx), + mode => single); + elsif not r.txn.uses_dummys and r.txn.data_kind = write then + return (byte => fifo, mode => r.txn.data_mode); + end if; + when wdata => + -- The fifo was acked on the edge that consumed the previous + -- byte, so its head is already the one after it. The tail of the + -- phase keeps reporting the data mode so the lane assignment + -- does not twitch on the last edge. + return (byte => fifo, mode => r.txn.data_mode); + when others => + null; + end case; + + -- Nothing of ours to send: dummy cycles or a read, both of which shift + -- ones out a bit at a time. + return (byte => (others => '1'), mode => single); + end; + begin -- "Simple outputs" cs_n <= r.csn; + tx_link_byte <= tx_pre.byte; + tx_link_mode <= tx_pre.mode; -- rx fifo is just a pass through here, no need for any muxing rx_fifo_data <= rx_link_byte; rx_fifo_write <= '1' when rx_byte_done else '0'; in_rx_phases <= is_in_rx_phases(r.state); in_tx_phases <= is_in_tx_phases(r.txn, r.state); + sclk_running <= is_sclk_running(r.state); -- more complicated outputs @@ -295,33 +400,24 @@ begin v.counter := r.counter - 1; end if; end case; - -- Deal with the chip selects once we've figured - -- out what state we're going to be in next - if v.state = cs_assert then + -- cs_n is derived from `r` rather than from the v.state we just decoded, + -- even though the two agree. cs_n_pin is an IOB flop, so whatever feeds + -- its D lands on the same kind of long launch path io_o does, and going + -- through v.state put the shifter's empty detect and the entire phase + -- decode on it for no reason: cs_n only moves on the idle -> cs_assert + -- and cs_deassert -> idle edges, and neither involves tx_byte_req. + -- Staying in cs_assert or idle leaves it where it already is. + if r.state = idle and spi_cmd.go_flag = '1' and r.cs_high = 0 then v.csn := '0'; - elsif v.state = idle then + elsif r.state = cs_deassert and r.counter = 0 then v.csn := '1'; - -- Arm the high-time counter on the edge that raises cs_n so the - -- next transaction cannot start too soon. - if r.csn = '0' then - v.cs_high := cs_high_cnts; - end if; end if; - -- The serializer consumes a byte on the same edge that this state - -- machine advances, so the byte it sees has to come from the state we - -- are moving *to*, not the one we are leaving. Driving this off `r` - -- works only while the phase advance lands a cycle before the shifter - -- reload, which stops being true once the sclk half period is a single - -- clk cycle. - if v.state = instruction or v.state = cs_assert then - tx_link_byte <= spi_cmd.instr; - elsif v.state = addr then - tx_link_byte <= spi_cmd.addr(8 * v.counter + 7 downto 8 * v.counter); - elsif v.state = wdata then - tx_link_byte <= tx_fifo_data; - else - tx_link_byte <= (others => '1'); + -- Arm the high-time counter on the edge that raises cs_n so the next + -- transaction cannot start too soon. Off v.state, since cs_high feeds + -- nothing but internal logic. + if v.state = idle and r.csn = '0' then + v.cs_high := cs_high_cnts; end if; -- Only ack the FIFO for bytes that actually came from it @@ -340,9 +436,13 @@ begin r <= r_reset; sclk_last <= '0'; cs_n_pin <= '1'; + tx_pre <= (byte => (others => '1'), mode => single); elsif rising_edge(clk) then sclk_last <= sclk; r <= rin; + -- Off `r`, not `rin`: the point of the prefetch is to give this + -- selection a full clk period of its own, clear of the reload edge. + tx_pre <= next_tx(r, spi_cmd, tx_fifo_data); -- Duplicate of r.csn, driven from the same next-state value so the -- two flops always agree and change on the same edge. cs_n_pin <= rin.csn; diff --git a/hdl/ip/vhd/vunit_components/spi_nor_target/spi_nor_target_vc.vhd b/hdl/ip/vhd/vunit_components/spi_nor_target/spi_nor_target_vc.vhd index 2aa00aa2..662b56d6 100644 --- a/hdl/ip/vhd/vunit_components/spi_nor_target/spi_nor_target_vc.vhd +++ b/hdl/ip/vhd/vunit_components/spi_nor_target/spi_nor_target_vc.vhd @@ -163,6 +163,12 @@ begin variable status : std_logic_vector(7 downto 0) := x"00"; variable erase_len : natural := 0; variable do_erase : boolean := false; + -- Set when the host clocks the part in a way that makes the instruction + -- invalid. The real part discards such a command silently, so anything + -- it would have done has to be suppressed too or the model is more + -- forgiving than the hardware and the testbench proves nothing. + variable cmd_invalid : boolean := false; + variable modelled : boolean := true; variable cs_fall_at : time := 0 ps; variable cs_rise_at : time := 0 ps; variable first_clk : boolean := true; @@ -336,7 +342,9 @@ begin when others => info(vc_logger, "Unmodelled opcode 0x" & to_hstring(opcode) & ", treating as no-operand"); - phase := ph_done; + -- Don't police the clock count for something we can't decode + modelled := false; + phase := ph_done; end case; end procedure; @@ -375,17 +383,32 @@ begin begin if cs_n /= '0' then + if cs_n = '1' and cs_n'event then + cs_rise_at := now; + + -- The part latches an instruction only if cs_n rises on a byte + -- boundary. Trailing clocks that leave a partial byte make the + -- whole command invalid, and it is discarded without any + -- indication -- an erase simply does not happen. + if bit_cnt /= 0 then + cmd_invalid := true; + error(vc_logger, "cs_n rose " & to_string(bit_cnt) & + " bits into a byte, instruction 0x" & + to_hstring(opcode) & " is discarded"); + end if; + end if; + -- Deselected: finish any pending erase, then reset per-transaction -- state. Erases are modelled as instantaneous on deselect. if do_erase then - mem.erase((mem_addr(addr) / erase_len) * erase_len, erase_len); + if not cmd_invalid then + mem.erase((mem_addr(addr) / erase_len) * erase_len, erase_len); + end if; do_erase := false; end if; - if cs_n = '1' and cs_n'event then - cs_rise_at := now; - end if; - + cmd_invalid := false; + modelled := true; phase := ph_cmd; bit_cnt := 0; in_width := 1; @@ -478,8 +501,21 @@ begin out_left := 0; end if; - when ph_rd | ph_done => + when ph_rd => null; + + when ph_done => + -- The instruction and its operands are complete, so the + -- host should have raised cs_n by now. Clocking on makes + -- the command invalid: an erase or a write enable that + -- gets trailing clocks is thrown away, which shows up + -- much later as "the sector did not erase". + if modelled and not cmd_invalid then + cmd_invalid := true; + error(vc_logger, "sclk continued after instruction 0x" & + to_hstring(opcode) & + " completed, command is discarded"); + end if; end case; elsif falling_edge(sclk) then if phase = ph_rd then From f6accaa8703981ae6ac42a3e06721d03e3242fc1 Mon Sep 17 00:00:00 2001 From: Nathanael Huffman Date: Tue, 11 Aug 2026 19:22:59 -0400 Subject: [PATCH 3/4] Add some spi block docs --- hdl/ip/vhd/spi_nor_controller/docs/README.md | 140 ++++++++++++++---- .../docs/spi_nor_block.drawio.svg | 95 +++++++++++- .../docs/spi_nor_edges.drawio.svg | 58 ++++++++ 3 files changed, 267 insertions(+), 26 deletions(-) create mode 100644 hdl/ip/vhd/spi_nor_controller/docs/spi_nor_edges.drawio.svg diff --git a/hdl/ip/vhd/spi_nor_controller/docs/README.md b/hdl/ip/vhd/spi_nor_controller/docs/README.md index d851550c..29e7cc82 100644 --- a/hdl/ip/vhd/spi_nor_controller/docs/README.md +++ b/hdl/ip/vhd/spi_nor_controller/docs/README.md @@ -1,25 +1,115 @@ -This controller aims to provide an interface from software and the espi blocks - -From the software side, we support read and writes. - -The sw interface is intended to be rather simple: -There are registers for a 256byte tx/rx fifo, and various fifo flags in the status register. -There are fifo reset signals in the control register. - -The software interface to issue commands is as follows: -If doing a data write: -- (Optional: clean out any data in FIFOs using control register to reset them) -- Write up to 256 data bytes into TX FIFO, each write is 4 bytes due to 32bit access. -- Set data size register to the number of data bytes to send. This does not have to be 4 byte multiple -- Write number of dummy *clocks* into the dummy register as required for the instruction according to flash datasheeet -- Write instruction into the instruction register. Write-side effect will begin the transaction. -- Wait until status shows not busy - -If doing a data read -- (Optional: clean out any data in FIFOs using control register to reset them) -- Set data size register to the number of data bytes to send. This does not have to be 4 byte multiple -- Write number of dummy *clocks* into the dummy register as required for the instruction according to flash datasheeet -- Write instruction into the instruction register. Write-side effect will begin the transaction. -- You can either wait until status shows not busy, or poll on the rx fifo used wds and start consuming data -- as it becomes available -- Wait until status shows not busy \ No newline at end of file +# SPI NOR controller + +Drives a Winbond W25Q01JV QSPI NOR flash on behalf of two masters: the SP over +AXI-Lite, and the eSPI block fetching the host image. `SPICR.sp5_owns_flash` +picks which one owns the part. + +![block diagram](spi_nor_block.drawio.svg) + +## Software interface + +Registers are generated from `spi_nor_regs.rdl`. Writing `Instr` starts a +transaction; reading `rx_fifo_rdata` pops the RX FIFO. + +Write: + +- (optional) reset the FIFOs via the control register +- write up to 256 data bytes into the TX FIFO, 4 bytes per access +- set the data size register to the byte count (need not be a multiple of 4) +- set the dummy register to the clock count the instruction needs +- write the instruction; the write side effect starts the transaction +- poll `SPISR.busy` + +Read: as above without the FIFO fill. Either wait on `busy`, or poll +`rx_used_wds` and drain as data arrives. + +The opcode alone determines the phase sequence and bus width, via +`get_txn_info` in `spi_nor_pkg`. Software supplies only the dummy count. + +## Structure + +| unit | role | +| --- | --- | +| `spi_nor_regs` | AXI-Lite target, command and FIFO registers | +| `espi_flash_txn_mgr` | turns eSPI read requests into commands, remaps host and APOB addresses | +| `spi_txn_mgr` | phase FSM, chip select, byte and io-mode prefetch | +| `spi_link` | serializer, deserializer, rx capture and sample-point mux | +| `spi_clk_gen` | sclk divider | +| `mixed_width_adaptor` | 32-bit FIFO side to 8-bit link side | + +## Clocking and edges + +`sclk = clk / (2 * (sclk_divisor + 1))`. Both projects run `sclk_divisor => 0` +off `clk_125m`, so **sclk is 62.5MHz**, a 16ns period. + +![edge relationships](spi_nor_edges.drawio.svg) + +Three things follow from a half period being one clk cycle, and all three are +load-bearing: + +**Launch on the falling edge itself.** The shifter, `io_o`, `io_oe` and the io +mode all move on the clk edge that drives sclk low, so mosi and sclk leave the +FPGA together. Reacting to an edge detector instead spends a whole clk of the +half-period budget and caps sclk at clk/4. + +**The byte and its io mode are prefetched together.** The serializer reloads on +the same edge the FSM advances a phase, so both must already be sitting there. +Taking the mode from the registered state instead means the first data byte of a +dual or quad write is loaded while the state still reads `addr`: it goes out +with single-bit lane assignment and gets shifted by one instead of four, which +leaves the shifter's sentinel where the byte-complete compare can never match +and the transaction never ends. + +**Read data is sampled at a placed point, not on an edge.** The round trip out +to the part and back does not shrink with sclk, so `rx_sample_taps` selects the +sample point in half-clk steps after the sclk rising edge (default 2 = 8ns), +sourced from a rising- and a falling-edge capture flop. The point must satisfy + +``` +round_trip_valid - half_period <= S <= half_period + round_trip_hold +``` + +Note the upper limit comes from the part's tCLQX, not tCLQV: sampling too late +catches the next bit. The project XDC carries the arithmetic for the delays it +bounds. + +`sclk_running` is deliberately narrower than `in_tx_phases`: `cs_deassert` still +drives the bus, so mosi is not torn down on the edge the part samples it, but it +must not clock. The part counts edges to find the end of an instruction, and a +single trailing edge makes an erase or a write enable be discarded silently. + +## Physical + +Every pin-facing flop is a dedicated duplicate with no internal fanout +(`sclk_pin`, `cs_n_pin`, `io_o`, `io_oe`, the rx capture) so it can be packed +into its IOB. That is what makes the round trip bounded and repeatable: left in +the fabric the placer put these 12-13ns of routing from their pins and varied by +several ns between builds, which both blew the clock-to-data skew budget and +pushed the round trip outside every available sample point. + +62.5MHz is the ceiling for this structure. The sample point is placed, not +trained, so the round trip has to fit within half a period of it; above this +rate that window closes and it would take per-lane `IDELAY` read training to go +further. + +## Simulation + +`spi_nor_target_vc` models the part with datasheet AC timing (tCLQV, tCLQX, +tDVCH, tCHDX, tSLCH, tSHSL) and discards malformed commands the way silicon +does, so a trailing clock or a bad sample point fails a test rather than +producing a plausible waveform. The harness also models the FPGA's own +flop-to-pin and pin-to-flop delays; without them simulation validates a regime +that does not exist on hardware. + +| testbench | configuration | +| --- | --- | +| `spi_nor_tb` | clk/6, the historical setting | +| `spi_nor_fast_tb` | clk/2, slow IO corner | +| `spi_nor_fast_quick_io_tb` | clk/2, fast IO corner | + +The fast benches also sweep the part's output delay across its datasheet range +to show the sample point has margin at both ends. + +``` +buck2 run //hdl/ip/vhd/spi_nor_controller:spi_nor_top_sim +``` diff --git a/hdl/ip/vhd/spi_nor_controller/docs/spi_nor_block.drawio.svg b/hdl/ip/vhd/spi_nor_controller/docs/spi_nor_block.drawio.svg index 16ddae42..686d63fa 100644 --- a/hdl/ip/vhd/spi_nor_controller/docs/spi_nor_block.drawio.svg +++ b/hdl/ip/vhd/spi_nor_controller/docs/spi_nor_block.drawio.svg @@ -1,4 +1,97 @@ -
SPI/QSPI
Link
(serializer, deserializer, clockgen)
SPI/QSPI...
AXI Register I/F
AXI Register I/F
Transaction Manager
Transaction Manager
TX DATA FIFO
256Byte
TX DATA FIFO...
RX DATA FIFO
256Byte
RX DATA FIFO...
8b i/f here
8b i/f here
32bit i/f here
32bit i/f here
ESPI Adaptor
ESPI Adaptor
To/From ESPI core
To/From ESPI core
To/From SP
To/From SPText is not SVG - cannot display \ No newline at end of file + + + + +spi_nor_top + +AXI-Lite +from SP + +spi_nor_regs +cmd + FIFO registers + +spi_cmd mux +sp5_owns_flash + +spi_txn_mgr +phase FSM, cs_n +byte + mode prefetch + +eSPI cmd +FIFO + +espi_flash_txn_mgr +address remap + +eSPI data +FIFO + +TX FIFO 64 x 32 + +RX FIFO 64 x 32 + +mixed_width_adaptor +32 <-> 8 + +spi_link +serializer / deserializer +rx capture + tap mux + +spi_clk_gen +sclk = clk / 2(div+1) + +IOB flops +sclk_pin, cs_n_pin +io_o, io_oe, capture + +QSPI pins +to W25Q01JV + + + +spi_cmd + + + + +tx data + + +tx_byte + mode + + + + +rx data + +phases, sclk_running, +release_lanes + +tx_byte_req +rx_byte_done + +sclk_fall_now + + +Orange: reworked for clk/2 operation. The transaction manager decides what goes out and when the clock may run; +spi_link turns that into bits and edges; every pin-facing flop is duplicated into its IOB so its delay is fixed. + diff --git a/hdl/ip/vhd/spi_nor_controller/docs/spi_nor_edges.drawio.svg b/hdl/ip/vhd/spi_nor_controller/docs/spi_nor_edges.drawio.svg new file mode 100644 index 00000000..6d33befc --- /dev/null +++ b/hdl/ip/vhd/spi_nor_controller/docs/spi_nor_edges.drawio.svg @@ -0,0 +1,58 @@ + + + + + + + +sclk_divisor = 0: sclk is clk/2, so one sclk half period is a single clk cycle (8ns at 125MHz) +clk + +sclk (internal) + + + + + + + + + + +io_o (write) + +bit n + +bit n+1 + +bit n+2 + +bit n+3 +sclk falls: shifter, io_o, io_oe and the io mode all move on this edge +sclk rises: the part samples io_o here +io (read, at +capture flop) + +bit n + +bit n+1 + +bit n+2 + +bit n+3 + +round trip: clk to sclk pin, tCLQV, back to the flop + + + + + + + + +rx sample point = rx_sample_taps x (clk/2) after the sclk rising edge; taps = 2 is 8ns. +It must land inside the shifted cell, so it is placed explicitly rather than taken on the edge. +sclk runs only in cs_assert / instruction / addr / dummy / wdata / rdata. cs_deassert still drives the bus but must not clock: +the part counts edges to find the end of an instruction, so a single trailing edge makes an erase or a write enable be discarded. +Every pin-facing flop is duplicated with no internal fanout and packed into its IOB, which is what makes the round trip bounded and repeatable. + From 41691f8cc020bb23d1b5a71aab7567821e454f78 Mon Sep 17 00:00:00 2001 From: Nathanael Huffman Date: Mon, 17 Aug 2026 12:26:24 -0400 Subject: [PATCH 4/4] review feedback --- hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb.vhd | 8 +++++++- hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb_pkg.vhd | 1 - hdl/projects/cosmo_seq/cosmo_timing.xdc | 9 ++++++--- .../sp5_espi_flash_subsystem.vhd | 4 +++- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb.vhd b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb.vhd index 3cf94b66..c4a05aee 100644 --- a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb.vhd +++ b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb.vhd @@ -23,7 +23,13 @@ architecture tb of spi_nor_tb is begin - th: entity work.spi_nor_th; + th: entity work.spi_nor_th + generic map( + sclk_divisor => 2, + rx_sample_taps => 2, + out_delay => 3.7 ns, + in_delay => 1.5 ns + ); bench: process -- Note: External names are broken in GHDL llvm backends https://github.com/ghdl/ghdl/issues/2610 diff --git a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb_pkg.vhd b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb_pkg.vhd index ca31c41e..c444a273 100644 --- a/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb_pkg.vhd +++ b/hdl/ip/vhd/spi_nor_controller/sims/spi_nor_tb_pkg.vhd @@ -74,7 +74,6 @@ package spi_nor_tb_pkg is -- Checked scenarios, shared by the testbenches so the same coverage can be -- run at more than one sclk rate without duplicating it. - procedure check_jedec_id ( signal net : inout network_t ); diff --git a/hdl/projects/cosmo_seq/cosmo_timing.xdc b/hdl/projects/cosmo_seq/cosmo_timing.xdc index 74661558..8dae48f1 100644 --- a/hdl/projects/cosmo_seq/cosmo_timing.xdc +++ b/hdl/projects/cosmo_seq/cosmo_timing.xdc @@ -183,9 +183,12 @@ set_min_delay -to [get_ports espi0_sp5_to_fpga1_dat[*]] 0 # coming back, and both are directly constrainable. # # Trace delays are short and local; using the same 6.8ns/m as the FMC block -# above. TODO: replace with the measured lengths off 913-0000023. -set flash_trace_max 0.40 -set flash_trace_min 0.10 +# above. +# On cosmo flash trace min is 31.982mm (Dat0), and max is 33.436 (Dat3) +# so min = 0.031982m * 6.8ns/m = 0.217 +# max = 0.033436 * 6.8ns/m = 0.227 +set flash_trace_max 0.227 +set flash_trace_min 0.217 # Pull the launch flops into the IOBs. Every one of these is a dedicated # duplicate whose only load is its pin (see spi_clk_gen's sclk_pin and diff --git a/hdl/projects/cosmo_seq/sp5_espi_flash_subsystem/sp5_espi_flash_subsystem.vhd b/hdl/projects/cosmo_seq/sp5_espi_flash_subsystem/sp5_espi_flash_subsystem.vhd index 87a33bf0..47c167a4 100644 --- a/hdl/projects/cosmo_seq/sp5_espi_flash_subsystem/sp5_espi_flash_subsystem.vhd +++ b/hdl/projects/cosmo_seq/sp5_espi_flash_subsystem/sp5_espi_flash_subsystem.vhd @@ -165,7 +165,9 @@ begin -- usable sample window at 3.6..11.7ns; 8ns sits about 4ns clear of -- either end. cosmo_timing.xdc carries the arithmetic. Sweep this on -- hardware if reads come back corrupted. - rx_sample_taps => 2 + rx_sample_taps => 2, + cs_setup_cnts => 4, + cs_high_cnts => 7 ) port map( clk => clk_125m,