Ndh/speed up spi nor - #514
Open
nathanaelhuffman wants to merge 2 commits into
Open
Conversation
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.
…ered when testing in hardware
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I've stacked this for now on the axi-pipes branch, but will rebase this against main once axi-pipes lands. This allows review of just this change.
Here we increase the SPI nor frequency to 62.5MHz (up from ~20MHz). This required some link layer optimization and the addition of some timing constraints for this. Simulation coverage has also increased.
At least when benchmark over usb, this basically make 0 difference as the usb transfers dominate the performance there.
Given what we know about flash and the system performance, it is unclear whether this will speed anything up in product but it gets the FPGA shift speed out of the way for any future performance investigation. There is probably additional optimization to be had around queueing up more data or other interface changes here should any hubris benchmarking suggest additional optimization opportunity.
This has been tested in hardware on a cosmo with the following somewhat adhoc tests via humility: