vendor.xilinx_{spartan_3_6,7series}: reconsider default reset logic.

On Xilinx devices, flip-flops are reset to their initial state with
an internal global reset network, but this network is deasserted
asynchronously to user clocks. Use BUFGCE and STARTUP to hold default
clock low until after GWE is deasserted.

diff --git a/nmigen/build/plat.py b/nmigen/build/plat.py
index 94fd79fb0d3d0d7c44d5654c4eb450704304f8a3..63458500b2a2530fa52820765ccd9fa9a7d80902 100644 (file)
--- a/nmigen/build/plat.py
+++ b/nmigen/build/plat.py
@@ -74,6 +74,10 @@ class Platform(ResourceManager, metaclass=ABCMeta):
 
     @abstractmethod
     def create_missing_domain(self, name):
+        # Simple instantiation of a clock domain driven directly by the board clock and reset.
+        # Because of device-specific considerations, this implementation generally does NOT provide
+        # reliable power-on/post-configuration reset, and the logic should be replaced with family
+        # specific logic based on vendor recommendations.
         if name == "sync" and self.default_clk is not None:
             clk_i = self.request(self.default_clk).i
             if self.default_rst is not None:

diff --git a/nmigen/vendor/xilinx_7series.py b/nmigen/vendor/xilinx_7series.py
index 34452ef68de40c756b870ecea24668c7b1382eae..02cfee9103c2a36cea87df656697b127b73b0d78 100644 (file)
--- a/nmigen/vendor/xilinx_7series.py
+++ b/nmigen/vendor/xilinx_7series.py
@@ -1,8 +1,6 @@
 from abc import abstractproperty
 
-from ..hdl.ast import *
-from ..hdl.dsl import *
-from ..hdl.ir import *
+from ..hdl import *
 from ..build import *
 
 
@@ -123,8 +121,27 @@ class Xilinx7SeriesPlatform(TemplatedPlatform):
     ]
 
     def create_missing_domain(self, name):
-        # No additional reset logic needed.
-        csuper().create_missing_domain(name)
+        # Xilinx devices have a global write enable (GWE) signal that asserted during configuraiton
+        # and deasserted once it ends. Because it is an asynchronous signal (GWE is driven by logic
+        # syncronous to configuration clock, which is not used by most designs), even though it is
+        # a low-skew global network, its deassertion may violate a setup/hold constraint with
+        # relation to a user clock. The recommended solution is to use a BUFGCE driven by the EOS
+        # signal. For details, see:
+        #   * https://www.xilinx.com/support/answers/44174.html
+        #   * https://www.xilinx.com/support/documentation/white_papers/wp272.pdf
+        if name == "sync" and self.default_clk is not None:
+            clk_i = self.request(self.default_clk).i
+            if self.default_rst is not None:
+                rst_i = self.request(self.default_rst).i
+
+            m = Module()
+            ready = Signal()
+            m.submodules += Instance("STARTUPE3", o_EOS=ready)
+            m.domains += ClockDomain("sync", reset_less=self.default_rst is None)
+            m.submodules += Instance("BUFGCE", i_CE=ready, i_I=clk_i, o_O=ClockSignal("sync"))
+            if self.default_rst is not None:
+                m.d.comb += ResetSignal("sync").eq(rst_i)
+            return m
 
     def _get_xdr_buffer(self, m, pin, i_invert=None, o_invert=None):
         def get_dff(clk, d, q):

diff --git a/nmigen/vendor/xilinx_spartan_3_6.py b/nmigen/vendor/xilinx_spartan_3_6.py
index 3b3b9bfde37f433d0c810dd0c66ab1c811283ae9..2f93e455db9b4a84f119bcb9a1162795cb363c15 100644 (file)
--- a/nmigen/vendor/xilinx_spartan_3_6.py
+++ b/nmigen/vendor/xilinx_spartan_3_6.py
@@ -1,18 +1,15 @@
 from abc import abstractproperty
 
-from ..hdl.ast import *
-from ..hdl.dsl import *
-from ..hdl.ir import *
+from ..hdl import *
 from ..build import *
 
 
 __all__ = ["XilinxSpartan3APlatform", "XilinxSpartan6Platform"]
 
+
 # The interface to Spartan 3 and 6 are substantially the same. Handle
 # differences internally using one class and expose user-aliases for
 # convenience.
-
-
 class XilinxSpartan3Or6Platform(TemplatedPlatform):
     """
     Required tools:
@@ -164,8 +161,31 @@ class XilinxSpartan3Or6Platform(TemplatedPlatform):
     ]
 
     def create_missing_domain(self, name):
-        # No additional reset logic needed.
-        return super().create_missing_domain(name)
+        # Xilinx devices have a global write enable (GWE) signal that asserted during configuraiton
+        # and deasserted once it ends. Because it is an asynchronous signal (GWE is driven by logic
+        # syncronous to configuration clock, which is not used by most designs), even though it is
+        # a low-skew global network, its deassertion may violate a setup/hold constraint with
+        # relation to a user clock. The recommended solution is to use a BUFGCE driven by the EOS
+        # signal (if available). For details, see:
+        #   * https://www.xilinx.com/support/answers/44174.html
+        #   * https://www.xilinx.com/support/documentation/white_papers/wp272.pdf
+        if name == "sync" and self.default_clk is not None:
+            clk_i = self.request(self.default_clk).i
+            if self.default_rst is not None:
+                rst_i = self.request(self.default_rst).i
+
+            m = Module()
+            ready = Signal()
+            if self.family == "6":
+                m.submodules += Instance("STARTUP_SPARTAN6", o_EOS=ready)
+            else:
+                raise NotImplementedError("Spartan 3 devices lack an end-of-startup signal; "
+                                          "ensure the design has an appropriate reset")
+            m.domains += ClockDomain("sync", reset_less=self.default_rst is None)
+            m.submodules += Instance("BUFGCE", i_CE=ready, i_I=clk_i, o_O=ClockSignal("sync"))
+            if self.default_rst is not None:
+                m.d.comb += ResetSignal("sync").eq(rst_i)
+            return m
 
     def _get_xdr_buffer(self, m, pin, i_invert=None, o_invert=None):
         def get_dff(clk, d, q):