From 8050cfaa986793433c52efcbe505ca57a92cd5e2 Mon Sep 17 00:00:00 2001 From: whitequark Date: Sat, 21 Sep 2019 14:12:29 +0000 Subject: [PATCH] build.res: simplify clock constraints. Before this commit, it was possible to set and get clock constraints placed on Pin objects. This was not a very good implementation, since it relied on matching the identity of the provided Pin object to a previously requested one. The only reason it worked like that is deficiencies in nextpnr. Since then, nextpnr has been fixed to allow setting constraints on arbitrary nets. Correspondingly, backends that are using Synplify were changed to use [get_nets] instead of [get_ports] in SDC files. However, in some situations, Synplify does not allow specifying ports in [get_nets]. (In fact, nextpnr had a similar problem, but it has also been fixed.) The simplest way to address this is to refer to the interior net (after the input buffer), which always works. The only downside of this is that requesting a clock as a raw pin using platform.request("clk", dir="-") and directly applying a constraint to it could fail in some cases. This is not a significant issue. --- nmigen/build/res.py | 30 +++--------------------------- nmigen/test/test_build_res.py | 27 +++++++-------------------- 2 files changed, 10 insertions(+), 47 deletions(-) diff --git a/nmigen/build/res.py b/nmigen/build/res.py index c1c0dce..bdd3f0a 100644 --- a/nmigen/build/res.py +++ b/nmigen/build/res.py @@ -147,7 +147,7 @@ class ResourceManager: self._ports.append((resource, pin, port, attrs)) if pin is not None and resource.clock is not None: - self.add_clock_constraint(pin, resource.clock.frequency) + self.add_clock_constraint(pin.i, resource.clock.frequency) return pin if pin is not None else port @@ -212,32 +212,12 @@ class ResourceManager: for bit, pin_name in enumerate(pin_names): yield "{}[{}]".format(port_name, bit), pin_name, attrs - def _map_clock_to_port(self, clock): - if not isinstance(clock, (Signal, Pin)): - raise TypeError("Object {!r} is not a Signal or Pin".format(clock)) - - if isinstance(clock, Pin): - for res, pin, port, attrs in self._ports: - if clock is pin: - if isinstance(res.ios[0], Pins): - clock = port.io - elif isinstance(res.ios[0], DiffPairs): - clock = port.p - else: - assert False - break - else: - raise ValueError("The Pin object {!r}, which is not a previously requested " - "resource, cannot be used to desigate a clock" - .format(clock)) - - return clock - def add_clock_constraint(self, clock, frequency): + if not isinstance(clock, Signal): + raise TypeError("Object {!r} is not a Signal".format(clock)) if not isinstance(frequency, (int, float)): raise TypeError("Frequency must be a number, not {!r}".format(frequency)) - clock = self._map_clock_to_port(clock) if clock in self._clocks: raise ValueError("Cannot add clock constraint on {!r}, which is already constrained " "to {} Hz" @@ -245,9 +225,5 @@ class ResourceManager: else: self._clocks[clock] = float(frequency) - def get_clock_constraint(self, clock): - clock = self._map_clock_to_port(clock) - return self._clocks[clock] - def iter_clock_constraints(self): return iter(self._clocks.items()) diff --git a/nmigen/test/test_build_res.py b/nmigen/test/test_build_res.py index c212700..fc111c8 100644 --- a/nmigen/test/test_build_res.py +++ b/nmigen/test/test_build_res.py @@ -194,24 +194,17 @@ class ResourceManagerTestCase(FHDLTestCase): 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) + (clk100.i, 100e6), + (clk50.i, 50e6) ]) def test_add_clock(self): i2c = self.cm.request("i2c") - self.cm.add_clock_constraint(i2c.scl, 100e3) - scl_port, sda_port = self.cm.iter_ports() + self.cm.add_clock_constraint(i2c.scl.o, 100e3) self.assertEqual(list(self.cm.iter_clock_constraints()), [ - (scl_port, 100e3) + (i2c.scl.o, 100e3) ]) - def test_get_clock(self): - clk100 = self.cm.request("clk100", 0) - self.assertEqual(self.cm.get_clock_constraint(clk100), 100e6) - with self.assertRaises(KeyError): - self.cm.get_clock_constraint(Signal()) - def test_wrong_resources(self): with self.assertRaises(TypeError, msg="Object 'wrong' is not a Resource"): self.cm.add_resources(['wrong']) @@ -239,7 +232,7 @@ class ResourceManagerTestCase(FHDLTestCase): def test_wrong_clock_signal(self): with self.assertRaises(TypeError, - msg="Object None is not a Signal or Pin"): + msg="Object None is not a Signal"): self.cm.add_clock_constraint(None, 10e6) def test_wrong_clock_frequency(self): @@ -247,12 +240,6 @@ class ResourceManagerTestCase(FHDLTestCase): msg="Frequency must be a number, not None"): self.cm.add_clock_constraint(Signal(), None) - def test_wrong_clock_pin(self): - with self.assertRaises(ValueError, - msg="The Pin object (rec i), which is not a previously requested " - "resource, cannot be used to desigate a clock"): - self.cm.add_clock_constraint(Pin(1, dir="i"), 1e6) - def test_wrong_request_duplicate(self): with self.assertRaises(ResourceError, msg="Resource user_led#0 has already been requested"): @@ -304,6 +291,6 @@ class ResourceManagerTestCase(FHDLTestCase): 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 " + msg="Cannot add clock constraint on (sig clk100_0__i), which is already " "constrained to 100000000.0 Hz"): - self.cm.add_clock_constraint(clk100, 1e6) + self.cm.add_clock_constraint(clk100.i, 1e6) -- 2.30.2