First comments.

git-svn-id: http://svn.ohwr.org/cern-fip/trunk/hdl/design@133 7f0067c9-7624-46c7-bd39-3fb5400c0213

First comments.
git-svn-id: http://svn.ohwr.org/cern-fip/trunk/hdl/design@133 7f0067c9-7624-46c7-bd39-3fb5400c0213
cfcc5c6c · serrano · e42a61d3 · cfcc5c6c
Commit cfcc5c6c authored Jan 24, 2011 by serrano
Hide whitespace changes
Inline Side-by-side

Showing with 29 additions and 0 deletions

javier.txt CodeReview_24Jan11/javier.txt +29 -0

No files found.
--- a/CodeReview_24Jan11/javier.txt
+++ b/CodeReview_24Jan11/javier.txt
+Comments on NanoFIP VHDL code from Javier Serrano
+
+General comments
+----------------
+
+None so far. Enjoying looking at code after quite a while, sipping a
+bio mango juice from Burkina Faso.
+
+wf_inputs_synchronizer.vhd
+--------------------------
+
+Line 250 onwards. Cosmetics: the vector notation is shorter, so I'd
+stick to it for all cases, including the varX_access.
+
+Line 326. I don't think it's a good idea to synchronize the data from
+the user in slone mode. If there is an incoherent state (i.e. some
+bits have flipped and others haven't) this state will propagate
+through the pipeline. Only control signals need to be synchronized (as
+is done for the wishbone). The user *must* ensure the data are stable
+by the time VAR3_RDY goes to zero, so the 3*16 FFs spent here can only
+get us trouble. Incidentally, I think we should change the spec so it
+does not say we sample the data on the first clock tick after VAR3_RDY
+goes down. We can also sample a couple of ticks later, the user should
+not care.
+
+Line 347. One could argue about the need for these. We know these
+inputs are static. While I can't really think how these FFs could get
+us in trouble, they are certainly not needed, so why risk it?
+