lib.fifo: fix reset handling of asynchronous FIFOs. #591
Conversation
Codecov Report
@@ Coverage Diff @@
## master #591 +/- ##
==========================================
+ Coverage 81.50% 81.58% +0.07%
==========================================
Files 49 49
Lines 6461 6467 +6
Branches 1287 1288 +1
==========================================
+ Hits 5266 5276 +10
+ Misses 1007 1005 -2
+ Partials 188 186 -2
Continue to review full report at Codecov.
|
jfng
force-pushed
the
fix-async-fifo-reset
branch
from
0dcf329
to
b8dd8a1
Compare
There was a problem hiding this comment.
After discussing with @whitequark, it is no longer clear whether the current approach of propagating write domain resets to the read side is correct; doing so empties the FIFO, which can violate the invariant of a stream interface (valid must not be deasserted on it own).
Ideally, there would be a configurable "primary" and "secondary" side, where the primary side controls the reset of the secondary side.
In any case, further iterations of this change will require thorough testing and would benefit from (multi-clock) BMC or fuzzing.
| # write domain. | ||
| with m.If(w_rst): | ||
| m.d[self._w_domain] += produce_w_gry.eq(consume_w_gry) | ||
| m.d[self._w_domain] += produce_w_bin.eq(_gray_decode(consume_w_gry)) |
There was a problem hiding this comment.
This does not work: if a write domain reset happens one clock cycle after a read, then the next value of consume_w_gry is still inside an FFSynchronizer.
Asynchronous FIFOs do not properly handle a write domain reset (i.e.
w_levelis not reset to 0). This PR attempts to fix their behaviour.