Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

selftest loopback fails with USB EP8IN timeout #161

Closed
whitequark opened this issue Sep 28, 2019 · 3 comments
Closed

selftest loopback fails with USB EP8IN timeout #161

whitequark opened this issue Sep 28, 2019 · 3 comments
Labels
upstream Meta: something we can't change ourselves

Comments

@whitequark
Copy link
Member

This appears to be a crossbar issue. It is caused by migration to nMigen. Good commit: 80aef21. Bad commit: 376422a.

Changing the order of EPs like this:

diff --git a/software/glasgow/applet/internal/selftest/__init__.py b/software/glasgow/applet/internal/selftest/__init__.py
index 6307f88..09914d7 100644
--- a/software/glasgow/applet/internal/selftest/__init__.py
+++ b/software/glasgow/applet/internal/selftest/__init__.py
@@ -220,8 +220,8 @@ class SelfTestApplet(GlasgowApplet, name="selftest"):
                 data_2 = bytes(reversed(data_1))
 
                 for iface, data, ep_out, ep_in in (
-                    (iface_1, data_1, "EP2OUT", "EP6IN"),
                     (iface_2, data_2, "EP4OUT", "EP8IN"),
+                    (iface_1, data_1, "EP2OUT", "EP6IN"),
                 ):
                     try:
                         await iface.write(data)

makes both EP8IN and EP6IN timeout.

Changing the crossbar to eliminate nMigen's builtin XDR logic like this:

diff --git a/software/glasgow/gateware/fx2_crossbar.py b/software/glasgow/gateware/fx2_crossbar.py
index 79b86f9..c17d6dc 100644
--- a/software/glasgow/gateware/fx2_crossbar.py
+++ b/software/glasgow/gateware/fx2_crossbar.py
@@ -262,6 +262,40 @@ class _UnimplementedINFIFO(Module, _FIFOInterface):
         self.flushed  = Signal()
 
 
+class _RegisteredTristate(Module):
+    def __init__(self, io):
+        self.oe = Signal()
+        self.o  = Signal.like(io)
+        self.i  = Signal.like(io)
+
+        def get_bit(signal, bit):
+            return signal[bit] if len(signal) > 0 else signal
+
+        for bit in range(len(io)):
+            # The FX2 output valid window starts well after (5.4 ns past) the iCE40 input
+            # capture window for the rising edge. However, the input capture for
+            # the falling edge is just right.
+            #
+            # We carefully use DDR input and fabric registers to capture the FX2 output in
+            # the valid window and prolong its validity to 1 IFCLK cycle. The output is
+            # not DDR and is handled the straightforward way.
+            #
+            # See https://github.com/GlasgowEmbedded/Glasgow/issues/89 for details.
+            d_in_1 = Signal()
+            self.specials += \
+                Instance("SB_IO",
+                    # PIN_INPUT_DDR|PIN_OUTPUT_REGISTERED_ENABLE_REGISTERED
+                    p_PIN_TYPE=C(0b110100, 6),
+                    io_PACKAGE_PIN=get_bit(io, bit),
+                    i_INPUT_CLK=ClockSignal(),
+                    i_OUTPUT_CLK=ClockSignal(),
+                    i_OUTPUT_ENABLE=self.oe,
+                    i_D_OUT_0=get_bit(self.o, bit),
+                    o_D_IN_1=d_in_1,
+                )
+            self.sync += get_bit(self.i, bit).eq(d_in_1)
+
+
 class _FX2Bus(Module):
     def __init__(self, pads):
         self.flag = Signal(4)
@@ -284,6 +318,14 @@ class _FX2Bus(Module):
 
         ###
 
+        self.submodules._fifoadr_t = _RegisteredTristate(pads.fifoadr)
+        self.submodules._flag_t    = _RegisteredTristate(pads.flag)
+        self.submodules._fd_t      = _RegisteredTristate(pads.fd)
+        self.submodules._sloe_t    = _RegisteredTristate(pads.sloe)
+        self.submodules._slrd_t    = _RegisteredTristate(pads.slrd)
+        self.submodules._slwr_t    = _RegisteredTristate(pads.slwr)
+        self.submodules._pktend_t  = _RegisteredTristate(pads.pktend)
+
         # The FX2 output valid window starts well after (5.4 ns past) the iCE40 input capture
         # window for the rising edge. However, the input capture for the falling edge is
         # just right.
@@ -295,24 +337,20 @@ class _FX2Bus(Module):
         #
         # See https://github.com/GlasgowEmbedded/Glasgow/issues/89 for details.
         self.comb += [
-            pads.flag.i_clk.eq(ClockSignal()),
-            self.flag.eq(pads.flag.i1),
-            pads.fifoadr.o_clk.eq(ClockSignal()),
-            pads.fifoadr.o.eq(self.addr),
-            pads.fd.o_clk.eq(ClockSignal()),
-            pads.fd.oe.eq(self.data.oe),
-            pads.fd.o0.eq(self.data.o),
-            pads.fd.o1.eq(self.data.o),
-            pads.fd.i_clk.eq(ClockSignal()),
-            self.data.i.eq(pads.fd.i1),
-            pads.sloe.o_clk.eq(ClockSignal()),
-            pads.sloe.o.eq(~self.sloe),
-            pads.slrd.o_clk.eq(ClockSignal()),
-            pads.slrd.o.eq(~self.slrd),
-            pads.slwr.o_clk.eq(ClockSignal()),
-            pads.slwr.o.eq(~self.slwr),
-            pads.pktend.o_clk.eq(ClockSignal()),
-            pads.pktend.o.eq(~self.pend),
+            self.flag.eq(self._flag_t.i),
+            self._fifoadr_t.oe.eq(1),
+            self._fifoadr_t.o.eq(self.addr),
+            self._fd_t.oe.eq(self.data.oe),
+            self._fd_t.o.eq(self.data.o),
+            self.data.i.eq(self._fd_t.i),
+            self._sloe_t.oe.eq(1),
+            self._sloe_t.o.eq(~self.sloe),
+            self._slrd_t.oe.eq(1),
+            self._slrd_t.o.eq(~self.slrd),
+            self._slwr_t.oe.eq(1),
+            self._slwr_t.o.eq(~self.slwr),
+            self._pktend_t.oe.eq(1),
+            self._pktend_t.o.eq(~self.pend),
         ]
 
         # Delay the FX2 bus control signals, taking into account the roundtrip latency.
diff --git a/software/glasgow/target/hardware.py b/software/glasgow/target/hardware.py
index db40f51..a2fba5c 100644
--- a/software/glasgow/target/hardware.py
+++ b/software/glasgow/target/hardware.py
@@ -44,8 +44,9 @@ class GlasgowHardwareTarget(Module):
         self.submodules.registers = I2CRegisters(self.i2c_slave)
         self.comb += self.i2c_slave.address.eq(0b0001000)
 
-        self.submodules.fx2_crossbar = FX2Crossbar(self.platform.request("fx2", xdr={
-            "sloe": 1, "slrd": 1, "slwr": 1, "pktend": 1, "fifoadr": 1, "flag": 2, "fd": 2
+        self.submodules.fx2_crossbar = FX2Crossbar(self.platform.request("fx2", dir={
+            "sloe": '-', "slrd": '-', "slwr": '-', "pktend": '-', "fifoadr": '-',
+            "flag": '-', "fd": '-'
         }))
 
         self.ports = {

does absolutely nothing. There were no other changes to the crossbar, or the selftest applet, that touch the USB data path.

Of note is that benchmark runs just fine, and it stresses the crossbar more. This is a strange issue.

@whitequark whitequark added selftest Applet: selftest upstream Meta: something we can't change ourselves and removed selftest Applet: selftest labels Sep 28, 2019
@whitequark
Copy link
Member Author

This is a toolchain bug: YosysHQ/yosys#1427

@whitequark
Copy link
Member Author

This is not just a toolchain bug, it still fails with that issue fixed.

@whitequark
Copy link
Member Author

Was also m-labs/nmigen@d139f34.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream Meta: something we can't change ourselves
Projects
None yet
Development

No branches or pull requests

1 participant