BusSynchronizer: lengthen request path, rather than shortening data path, so that data path benefits from MCP/falsepath constraints from MultiReg.

Author: Wren6991

nmigen/lib/cdc.py

@@ -229,8 +229,9 @@ def elaborate(self, platform):
         ack_o = Signal()
         ack_i = Signal()
 
+        # Extra flop on i->o to avoid race between data and request
         sync_io = m.submodules.sync_io = \
-            PulseSynchronizer(self.idomain, self.odomain, self.sync_stages)
+            PulseSynchronizer(self.idomain, self.odomain, self.sync_stages + 1)
         sync_oi = m.submodules.sync_oi = \
             PulseSynchronizer(self.odomain, self.idomain, self.sync_stages)
 
@@ -256,7 +257,7 @@ def elaborate(self, platform):
         with m.If(ack_i):
             m.d[self.idomain] += buf_i.eq(self.i)
         sync_data = m.submodules.sync_data = \
-            MultiReg(buf_i, buf_o, odomain=self.odomain, n=self.sync_stages - 1)
+            MultiReg(buf_i, buf_o, odomain=self.odomain, n=self.sync_stages)
         with m.If(ack_o):
             m.d[self.odomain] += self.o.eq(buf_o)
 

nmigen/test/test_lib_cdc.py

@@ -193,8 +193,8 @@ def process():
                 for i in range(10):
                     testval = i % (2 ** width)
                     yield bs.i.eq(testval)
-                    # 6-cycle round trip, and if one in progress, must complete first:
-                    for j in range(11):
+                    # 7-cycle round trip, and if one in progress, must complete first:
+                    for j in range(13):
                         yield Tick()
                     self.assertEqual((yield bs.o), testval)
             sim.add_process(process)