From: whitequark <whitequark@whitequark.org>
Date: Wed, 3 Jul 2019 15:07:44 +0000 (+0000)
Subject: build.res: detect physical conflicts earlier.
X-Git-Tag: locally_working~116
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=b471e33d7f79a2e427c341e1e3fffb8c7a368213;p=nmigen.git

build.res: detect physical conflicts earlier.

This is useful for two reasons:
  1. nMigen can provide better error messages than the platform and
     do it earlier in the build pipeline.
  2. Many platforms handle diffpairs by only constraining the P pin;
     the N pin is completely ignored. If this is undetected,
     downstream users (human or software) can rely on this
     information assuming it is correct and introduce more errors.
     (Of course, this will not catch every mistake, but the most
     common is a copy-paste issue, and that will handle it.)

Fixes #124.
---

diff --git a/nmigen/build/res.py b/nmigen/build/res.py
index a17adf0..0c9e851 100644
--- a/nmigen/build/res.py
+++ b/nmigen/build/res.py
@@ -18,6 +18,7 @@ class ResourceManager:
     def __init__(self, resources, connectors):
         self.resources  = OrderedDict()
         self._requested = OrderedDict()
+        self._phys_reqd = OrderedDict()
 
         self.connectors = OrderedDict()
         self._conn_pins = OrderedDict()
@@ -115,14 +116,25 @@ class ResourceManager:
             elif isinstance(resource.ios[0], (Pins, DiffPairs)):
                 phys = resource.ios[0]
                 if isinstance(phys, Pins):
+                    phys_names = phys.names
                     port = Record([("io", len(phys))], name=name)
                 if isinstance(phys, DiffPairs):
+                    phys_names = phys.p.names + phys.n.names
                     port = Record([("p", len(phys)),
                                    ("n", len(phys))], name=name)
                 if dir == "-":
                     pin = None
                 else:
                     pin = Pin(len(phys), dir, xdr, name=name)
+
+                for phys_name in phys_names:
+                    if phys_name in self._phys_reqd:
+                        raise ResourceError("Resource component {} uses physical pin {}, but it "
+                                            "is already used by resource component {} that was "
+                                            "requested earlier"
+                                            .format(name, phys_name, self._phys_reqd[phys_name]))
+                    self._phys_reqd[phys_name] = name
+
                 self._ports.append((resource, pin, port, attrs))
 
                 if pin is not None and resource.clock is not None:
diff --git a/nmigen/test/test_build_res.py b/nmigen/test/test_build_res.py
index d6df1ca..42e762f 100644
--- a/nmigen/test/test_build_res.py
+++ b/nmigen/test/test_build_res.py
@@ -238,6 +238,16 @@ class ResourceManagerTestCase(FHDLTestCase):
             self.cm.request("user_led", 0)
             self.cm.request("user_led", 0)
 
+    def test_wrong_request_duplicate_physical(self):
+        self.cm.add_resources([
+            Resource("clk20", 0, Pins("H1", dir="i")),
+        ])
+        self.cm.request("clk100", 0)
+        with self.assertRaises(ResourceError,
+                msg="Resource component clk20_0 uses physical pin H1, but it is already "
+                    "used by resource component clk100_0 that was requested earlier"):
+            self.cm.request("clk20", 0)
+
     def test_wrong_request_with_dir(self):
         with self.assertRaises(TypeError,
                 msg="Direction must be one of \"i\", \"o\", \"oe\", \"io\", or \"-\", "