build.{dsl,res,plat}: apply clock constraints to signals, not resources.

This adds the Clock() build DSL element, and adds a resource manager
function add_clock_constraint() that takes a Pin or a Signal.
Note that not all platforms, in particular not any nextpnr platforms
at the moment, can add constraints on arbitrary signals.

Fixes #86.

nmigen/build/__init__.py

@@ -1,3 +1,3 @@
-from .dsl import Pins, DiffPairs, Attrs, Subsignal, Resource, Connector
+from .dsl import *
 from .res import ResourceError
-from .plat import Platform, TemplatedPlatform
+from .plat import *

nmigen/build/dsl.py

@@ -1,7 +1,7 @@
 from collections import OrderedDict
 
 
-__all__ = ["Pins", "DiffPairs", "Attrs", "Subsignal", "Resource", "Connector"]
+__all__ = ["Pins", "DiffPairs", "Attrs", "Clock", "Subsignal", "Resource", "Connector"]
 
 
 class Pins:
@@ -83,11 +83,27 @@ class Attrs(OrderedDict):
                                     for k, v in self.items()))
 
 
+class Clock:
+    def __init__(self, frequency):
+        if not isinstance(frequency, (float, int)):
+            raise TypeError("Clock frequency must be a number")
+
+        self.frequency = float(frequency)
+
+    @property
+    def period(self):
+        return 1 / self.frequency
+
+    def __repr__(self):
+        return "(clock {})".format(self.frequency)
+
+
 class Subsignal:
     def __init__(self, name, *args):
         self.name  = name
         self.ios   = []
         self.attrs = Attrs()
+        self.clock = None
 
         if not args:
             raise ValueError("Missing I/O constraints")
@@ -108,15 +124,33 @@ class Subsignal:
                                     .format(arg, self.ios[-1]))
             elif isinstance(arg, Attrs):
                 self.attrs.update(arg)
+            elif isinstance(arg, Clock):
+                if self.ios and isinstance(self.ios[-1], (Pins, DiffPairs)):
+                    if self.clock is None:
+                        self.clock = arg
+                    else:
+                        raise ValueError("Clock constraint can be applied only once")
+                else:
+                    raise TypeError("Clock constraint can only be applied to Pins or DiffPairs, "
+                                    "not {!r}"
+                                    .format(self.ios[-1]))
             else:
-                raise TypeError("I/O constraint must be one of Pins, DiffPairs, Subsignal, "
+                raise TypeError("Constraint must be one of Pins, DiffPairs, Subsignal, Attrs, "
-                                "or Attrs, not {!r}"
+                                "or Clock, not {!r}"
                                 .format(arg))
 
+    def _content_repr(self):
+        parts = []
+        for io in self.ios:
+            parts.append(repr(io))
+        if self.clock is not None:
+            parts.append(repr(self.clock))
+        if self.attrs:
+            parts.append(repr(self.attrs))
+        return " ".join(parts)
+
     def __repr__(self):
-        return "(subsignal {} {} {})".format(self.name,
+        return "(subsignal {} {})".format(self.name, self._content_repr())
-                                             " ".join(map(repr, self.ios)),
-                                             repr(self.attrs))
 
 
 class Resource(Subsignal):
@@ -126,9 +160,7 @@ class Resource(Subsignal):
         self.number = number
 
     def __repr__(self):
-        return "(resource {} {} {} {})".format(self.name, self.number,
+        return "(resource {} {} {})".format(self.name, self.number, self._content_repr())
-                                               " ".join(map(repr, self.ios)),
-                                               repr(self.attrs))
 
 
 class Connector:

nmigen/build/plat.py

@@ -20,10 +20,9 @@ __all__ = ["Platform", "TemplatedPlatform"]
 class Platform(ResourceManager, metaclass=ABCMeta):
     resources  = abstractproperty()
     connectors = abstractproperty()
-    clocks     = abstractproperty()
 
     def __init__(self):
-        super().__init__(self.resources, self.connectors, self.clocks)
+        super().__init__(self.resources, self.connectors)
 
         self.extra_files = OrderedDict()
 
@@ -267,7 +266,7 @@ class TemplatedPlatform(Platform):
         plan = BuildPlan(script="build_{}".format(name))
         for filename_tpl, content_tpl in self.file_templates.items():
             plan.add_file(render(filename_tpl, origin=filename_tpl),
-                          render(content_tpl, origin=filename_tpl))
+                          render(content_tpl, origin=content_tpl))
         for filename, content in self.extra_files.items():
             plan.add_file(filename, content)
         return plan

nmigen/build/res.py

@@ -1,6 +1,6 @@
 from collections import OrderedDict
 
-from .. import *
+from ..hdl.ast import *
 from ..hdl.rec import *
 from ..lib.io import *
 
@@ -15,24 +15,19 @@ class ResourceError(Exception):
 
 
 class ResourceManager:
-    def __init__(self, resources, connectors, clocks):
+    def __init__(self, resources, connectors):
         self.resources  = OrderedDict()
         self._requested = OrderedDict()
 
         self.connectors = OrderedDict()
         self._conn_pins = OrderedDict()
 
-        self.clocks     = OrderedDict()
-
         # Constraint lists
         self._ports     = []
+        self._clocks    = SignalDict()
 
         self.add_resources(resources)
         self.add_connectors(connectors)
-        for name_number, frequency in clocks:
-            if not isinstance(name_number, tuple):
-                name_number = (name_number, 0)
-            self.add_clock(*name_number, frequency)
 
     def add_resources(self, resources):
         for res in resources:
@@ -56,19 +51,6 @@ class ResourceManager:
                 assert conn_pin not in self._conn_pins
                 self._conn_pins[conn_pin] = plat_pin
 
-    def add_clock(self, name, number, frequency):
-        resource = self.lookup(name, number)
-        if isinstance(resource.ios[0], Subsignal):
-            raise TypeError("Cannot constrain frequency of resource {}#{} because it has "
-                            "subsignals"
-                            .format(resource.name, resource.number, frequency))
-        if (resource.name, resource.number) in self.clocks:
-            other = self.clocks[resource.name, resource.number]
-            raise ResourceError("Resource {}#{} is already constrained to a frequency of "
-                                "{:f} MHz"
-                                .format(resource.name, resource.number, other / 1e6))
-        self.clocks[resource.name, resource.number] = frequency
-
     def lookup(self, name, number=0):
         if (name, number) not in self.resources:
             raise ResourceError("Resource {}#{} does not exist"
@@ -138,12 +120,15 @@ class ResourceManager:
                     port = Record([("p", len(phys)),
                                    ("n", len(phys))], name=name)
                 if dir == "-":
-                    self._ports.append((resource, None, port, attrs))
+                    pin = None
-                    return port
                 else:
-                    pin  = Pin(len(phys), dir, xdr, name=name)
+                    pin = Pin(len(phys), dir, xdr, name=name)
-                    self._ports.append((resource, pin, port, attrs))
+                self._ports.append((resource, pin, port, attrs))
-                    return pin
+
+                if pin is not None and resource.clock is not None:
+                    self.add_clock_constraint(pin, resource.clock.frequency)
+
+                return pin if pin is not None else port
 
             else:
                 assert False # :nocov:
@@ -206,19 +191,33 @@ class ResourceManager:
                 for bit, pin_name in enumerate(pin_names):
                     yield "{}[{}]".format(port_name, bit), pin_name, attrs
 
-    def iter_clock_constraints(self):
+    def add_clock_constraint(self, clock, frequency):
-        for name, number in self.clocks.keys() & self._requested.keys():
+        if not isinstance(clock, (Signal, Pin)):
-            resource = self.resources[name, number]
+            raise TypeError("Object {!r} is not a Signal or Pin".format(clock))
-            period   = self.clocks[name, number]
+        if not isinstance(frequency, (int, float)):
-            pin      = self._requested[name, number]
+            raise TypeError("Frequency must be a number, not {!r}".format(frequency))
-            if pin.dir == "io":
+
-                raise ResourceError("Cannot constrain frequency of resource {}#{} because "
+        if isinstance(clock, Pin):
-                                    "it has been requested as a tristate buffer"
+            for res, pin, port, attrs in self._ports:
-                                    .format(name, number))
+                if clock is pin:
-            if isinstance(resource.ios[0], Pins):
+                    if isinstance(res.ios[0], Pins):
-                port_name = "{}__io".format(pin.name)
+                        clock = port.io
-            elif isinstance(resource.ios[0], DiffPairs):
+                    elif isinstance(res.ios[0], DiffPairs):
-                port_name = "{}__p".format(pin.name)
+                        clock = port.p
+                    else:
+                        assert False
+                    break
             else:
-                assert False
+                raise ValueError("Cannot add clock constraint on a Pin {!r} that is not "
-            yield (port_name, period)
+                                 "a previously requested resource"
+                                 .format(clock))
+
+        if clock in self._clocks:
+            raise ValueError("Cannot add clock constraint on {!r}, which is already constrained "
+                             "to {} Hz"
+                             .format(clock, self._clocks[clock]))
+
+        self._clocks[clock] = float(frequency)
+
+    def iter_clock_constraints(self):
+        return iter(self._clocks.items())

nmigen/test/test_build_dsl.py

@@ -96,6 +96,14 @@ class AttrsTestCase(FHDLTestCase):
             a = Attrs(FOO=1)
 
 
+class ClockTestCase(FHDLTestCase):
+    def test_basic(self):
+        c = Clock(1_000_000)
+        self.assertEqual(c.frequency, 1e6)
+        self.assertEqual(c.period, 1e-6)
+        self.assertEqual(repr(c), "(clock 1000000.0)")
+
+
 class SubsignalTestCase(FHDLTestCase):
     def test_basic_pins(self):
         s = Subsignal("a", Pins("A0"), Attrs(IOSTANDARD="LVCMOS33"))
@@ -105,15 +113,15 @@ class SubsignalTestCase(FHDLTestCase):
     def test_basic_diffpairs(self):
         s = Subsignal("a", DiffPairs("A0", "B0"))
         self.assertEqual(repr(s),
-            "(subsignal a (diffpairs io (p A0) (n B0)) (attrs ))")
+            "(subsignal a (diffpairs io (p A0) (n B0)))")
 
     def test_basic_subsignals(self):
         s = Subsignal("a",
                 Subsignal("b", Pins("A0")),
                 Subsignal("c", Pins("A1")))
         self.assertEqual(repr(s),
-            "(subsignal a (subsignal b (pins io A0) (attrs )) "
+            "(subsignal a (subsignal b (pins io A0)) "
-                         "(subsignal c (pins io A1) (attrs )) (attrs ))")
+                         "(subsignal c (pins io A1)))")
 
     def test_attrs(self):
         s = Subsignal("a",
@@ -128,13 +136,17 @@ class SubsignalTestCase(FHDLTestCase):
         s = Subsignal("a", Pins("A0"), Attrs(SLEW="FAST"), Attrs(PULLUP="1"))
         self.assertEqual(s.attrs, {"SLEW": "FAST", "PULLUP": "1"})
 
+    def test_clock(self):
+        s = Subsignal("a", Pins("A0"), Clock(1e6))
+        self.assertEqual(s.clock.frequency, 1e6)
+
     def test_wrong_empty_io(self):
         with self.assertRaises(ValueError, msg="Missing I/O constraints"):
             s = Subsignal("a")
 
     def test_wrong_io(self):
         with self.assertRaises(TypeError,
-                msg="I/O constraint must be one of Pins, DiffPairs, Subsignal, or Attrs, "
+                msg="Constraint must be one of Pins, DiffPairs, Subsignal, Attrs, or Clock, "
                     "not 'wrong'"):
             s = Subsignal("a", "wrong")
 
@@ -153,10 +165,20 @@ class SubsignalTestCase(FHDLTestCase):
     def test_wrong_subsignals(self):
         with self.assertRaises(TypeError,
                 msg="Pins and DiffPairs are incompatible with other location or subsignal "
-                    "constraints, but (pins io B0) appears after (subsignal b (pins io A0) "
+                    "constraints, but (pins io B0) appears after (subsignal b (pins io A0))"):
-                    "(attrs ))"):
             s = Subsignal("a", Subsignal("b", Pins("A0")), Pins("B0"))
 
+    def test_wrong_clock(self):
+        with self.assertRaises(TypeError,
+                msg="Clock constraint can only be applied to Pins or DiffPairs, not "
+                    "(subsignal b (pins io A0))"):
+            s = Subsignal("a", Subsignal("b", Pins("A0")), Clock(1e6))
+
+    def test_wrong_clock_many(self):
+        with self.assertRaises(ValueError,
+                msg="Clock constraint can be applied only once"):
+            s = Subsignal("a", Pins("A0"), Clock(1e6), Clock(1e7))
+
 
 class ResourceTestCase(FHDLTestCase):
     def test_basic(self):
@@ -165,8 +187,8 @@ class ResourceTestCase(FHDLTestCase):
                 Subsignal("rx", Pins("A1", dir="i")),
                 Attrs(IOSTANDARD="LVCMOS33"))
         self.assertEqual(repr(r), "(resource serial 0"
-                                  " (subsignal tx (pins o A0) (attrs ))"
+                                  " (subsignal tx (pins o A0))"
-                                  " (subsignal rx (pins i A1) (attrs ))"
+                                  " (subsignal rx (pins i A1))"
                                   " (attrs IOSTANDARD=LVCMOS33))")
 
 

nmigen/test/test_build_res.py

@@ -9,8 +9,8 @@ from .tools import *
 class ResourceManagerTestCase(FHDLTestCase):
     def setUp(self):
         self.resources = [
-            Resource("clk100", 0, DiffPairs("H1", "H2", dir="i")),
+            Resource("clk100", 0, DiffPairs("H1", "H2", dir="i"), Clock(100e6)),
-            Resource("clk50", 0, Pins("K1")),
+            Resource("clk50", 0, Pins("K1"), Clock(50e6)),
             Resource("user_led", 0, Pins("A0", dir="o")),
             Resource("i2c", 0,
                 Subsignal("scl", Pins("N10", dir="o")),
@@ -20,14 +20,10 @@ class ResourceManagerTestCase(FHDLTestCase):
         self.connectors = [
             Connector("pmod", 0, "B0 B1 B2 B3 - -"),
         ]
-        self.cm = ResourceManager(self.resources, self.connectors, [])
+        self.cm = ResourceManager(self.resources, self.connectors)
 
     def test_basic(self):
-        self.clocks = [
+        self.cm = ResourceManager(self.resources, self.connectors)
-            ("clk100",      100),
-            (("clk50", 0),  50),
-        ]
-        self.cm = ResourceManager(self.resources, self.connectors, self.clocks)
         self.assertEqual(self.cm.resources, {
             ("clk100",   0): self.resources[0],
             ("clk50",    0): self.resources[1],
@@ -37,10 +33,6 @@ class ResourceManagerTestCase(FHDLTestCase):
         self.assertEqual(self.cm.connectors, {
             ("pmod", 0): self.connectors[0],
         })
-        self.assertEqual(self.cm.clocks, {
-            ("clk100", 0): 100,
-            ("clk50",  0): 50,
-        })
 
     def test_add_resources(self):
         new_resources = [
@@ -152,23 +144,28 @@ class ResourceManagerTestCase(FHDLTestCase):
             )
         ])
         spi0 = self.cm.request("spi", 0)
-        self.assertEqual(list(sorted(self.cm.iter_port_constraints())), [
+        self.assertEqual(list(self.cm.iter_port_constraints()), [
+            ("spi_0__ss__io",   ["B0"], {}),
             ("spi_0__clk__io",  ["B1"], {}),
             ("spi_0__miso__io", ["B2"], {}),
             ("spi_0__mosi__io", ["B3"], {}),
-            ("spi_0__ss__io",   ["B0"], {}),
+        ])
+
+    def test_request_clock(self):
+        clk100 = self.cm.request("clk100", 0)
+        clk50 = self.cm.request("clk50", 0, dir="i")
+        clk100_port_p, clk100_port_n, clk50_port = self.cm.iter_ports()
+        self.assertEqual(list(self.cm.iter_clock_constraints()), [
+            (clk100_port_p, 100e6),
+            (clk50_port, 50e6)
         ])
 
     def test_add_clock(self):
-        self.cm.add_clock("clk100", 0, 10e6)
+        i2c = self.cm.request("i2c")
-        self.assertEqual(self.cm.clocks["clk100", 0], 10e6)
+        self.cm.add_clock_constraint(i2c.scl, 100e3)
-        self.cm.add_clock("clk50", 0, 5e6)
+        scl_port, sda_port = self.cm.iter_ports()
-
+        self.assertEqual(list(self.cm.iter_clock_constraints()), [
-        clk100 = self.cm.request("clk100", 0)
+            (scl_port, 100e3)
-        clk50 = self.cm.request("clk50", 0, dir="i")
-        self.assertEqual(list(sorted(self.cm.iter_clock_constraints())), [
-            ("clk100_0__p", 10e6),
-            ("clk50_0__io", 5e6)
         ])
 
     def test_wrong_resources(self):
@@ -177,8 +174,8 @@ class ResourceManagerTestCase(FHDLTestCase):
 
     def test_wrong_resources_duplicate(self):
         with self.assertRaises(NameError,
-                msg="Trying to add (resource user_led 0 (pins o A1) (attrs )), but "
+                msg="Trying to add (resource user_led 0 (pins o A1)), but "
-                    "(resource user_led 0 (pins o A0) (attrs )) has the same name and number"):
+                    "(resource user_led 0 (pins o A0)) has the same name and number"):
             self.cm.add_resources([Resource("user_led", 0, Pins("A1", dir="o"))])
 
     def test_wrong_connectors(self):
@@ -196,25 +193,15 @@ class ResourceManagerTestCase(FHDLTestCase):
                 msg="Resource user_led#1 does not exist"):
             r = self.cm.lookup("user_led", 1)
 
-    def test_wrong_frequency_subsignals(self):
+    def test_wrong_clock_signal(self):
         with self.assertRaises(TypeError,
-                msg="Cannot constrain frequency of resource i2c#0 because "
+                msg="Object None is not a Signal or Pin"):
-                    "it has subsignals"):
+            self.cm.add_clock_constraint(None, 10e6)
-            self.cm.add_clock("i2c", 0, 10e6)
 
-    def test_wrong_frequency_tristate(self):
+    def test_wrong_clock_frequency(self):
-        with self.assertRaises(ResourceError,
+        with self.assertRaises(TypeError,
-                msg="Cannot constrain frequency of resource clk50#0 because "
+                msg="Frequency must be a number, not None"):
-                    "it has been requested as a tristate buffer"):
+            self.cm.add_clock_constraint(Signal(), None)
-            self.cm.add_clock("clk50", 0, 20e6)
-            clk50 = self.cm.request("clk50", 0)
-            list(self.cm.iter_clock_constraints())
-
-    def test_wrong_frequency_duplicate(self):
-        with self.assertRaises(ResourceError,
-                msg="Resource clk100#0 is already constrained to a frequency of 10.000000 MHz"):
-            self.cm.add_clock("clk100", 0, 10e6)
-            self.cm.add_clock("clk100", 0, 5e6)
 
     def test_wrong_request_duplicate(self):
         with self.assertRaises(ResourceError,
@@ -238,7 +225,7 @@ class ResourceManagerTestCase(FHDLTestCase):
     def test_wrong_request_with_dir_dict(self):
         with self.assertRaises(TypeError,
                 msg="Directions must be a dict, not 'i', because (resource i2c 0 (subsignal scl "
-                    "(pins o N10) (attrs )) (subsignal sda (pins io N11) (attrs )) (attrs )) "
+                    "(pins o N10)) (subsignal sda (pins io N11))) "
                     "has subsignals"):
             i2c = self.cm.request("i2c", 0, dir="i")
 
@@ -250,6 +237,13 @@ class ResourceManagerTestCase(FHDLTestCase):
     def test_wrong_request_with_xdr_dict(self):
         with self.assertRaises(TypeError,
                 msg="Data rate must be a dict, not 2, because (resource i2c 0 (subsignal scl "
-                    "(pins o N10) (attrs )) (subsignal sda (pins io N11) (attrs )) (attrs )) "
+                    "(pins o N10)) (subsignal sda (pins io N11))) "
                     "has subsignals"):
             i2c = self.cm.request("i2c", 0, xdr=2)
+
+    def test_wrong_clock_constraint_twice(self):
+        clk100 = self.cm.request("clk100")
+        with self.assertRaises(ValueError,
+                msg="Cannot add clock constraint on (sig clk100_0__p), which is already "
+                    "constrained to 100000000.0 Hz"):
+            self.cm.add_clock_constraint(clk100, 1e6)

nmigen/vendor/lattice_ice40.py

@@ -76,9 +76,9 @@ class LatticeICE40Platform(TemplatedPlatform):
         """,
         "{{name}}_pre_pack.py": r"""
             # {{autogenerated}}
-            {% for port_name, frequency in platform.iter_clock_constraints() -%}
+            {% for signal, frequency in platform.iter_clock_constraints() -%}
             {# Clock in MHz #}
-            ctx.addClock("{{port_name}}", {{frequency/1000000}})
+            ctx.addClock("{{signal.name}}", {{frequency/1000000}})
             {% endfor%}
         """,
     }