Fix i2c muxing between fpga and SP5 #443
Open
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.
Fixes #434
In #436 and its corresponding hubris PR, we worked around the fact that the FPGA wasn't properly muxing between the SP5 and FPGA when doing i2c transactions during boot, resulting in missing DIMM SPD information and generally unhappy DDR presence detection. We discovered during that investigation that we were making some assumptions about when we could abort an i2c transaction that were invalid: notably any time we have ACK'd a read, any time we're expecting a target ACK, and after we've set up a read and the target has ACK'd, the target owns the bus and we can't safely generate stop conditions which require an SDA rising transition from the controller and the controller doesn't own the bus.
Because START conditions are totally asynchronous, commanded by the SP5, and can come at arbitrary times, we need a system to abort an in-process FPGA transaction safely. As described above, there are certain transaction phases where we can't safely abort, so we need to begin storing the SP5's commanded i2c transaction until we can abort the FPGA transaction safely, abort it, and then play back the portion of the stored SP5 transaction at a higher frequency until we catch-up with the actual bits being transmitted. To make things simple, we assume that it is possible to safely abort any in-process FPGA transaction and play back the data before we get to the first ACK phase of the SP5's transaction. This assumption allows us not have to generate ACK's back to the SP5 as we'll have handed off the bus by that time, avoiding our need to synthesize an ACK assuming the target will give one and then having to somehow wind that back if the target doesn't actually ACK.
Since we now abort FPGA-based transactions, I've implemented a hardware-retry that will re-issue the transaction once the bus opens back up rather than just dropping them on the floor like we did previously. This potentially induces a bit of jitter into the data, but since we re-issue the whole transaction, the response data is fresh rather than cached.
Additionally, I was unhappy with how these blocks were organized, and how they interacted with requesting bus arbitration so I re-structured the design and we have a priority arbiter giving the SP5 bus priority and then a downstream round-robin arbiter that handles the SPD engine and any requests from hubris.
Over the course of implementing and testing this I also discovered a number of bugs:
#440 is the result of priority arbitration not waiting for the currently granted actor to release the bus.
There were a couple of bugs that are fixed in how the fpga's transaction and link layer i2c blocks interacted that caused an extra sclk to be inserted before a restart. That's mostly benign but wasn't correct so it was fixed.
There is a lingering issue where we get an extra clock before a STOP that I haven't fixed yet #442