Add support for lazy Decoder/Multiplexer interface creation. (#7)

Before this commit, the bus interface of a Decoder or a Multiplexer
would be created during __init__. A bus interface would always be tied
to an underlying memory map.

After this commit, the following changes are introduced:

* The bus interface of a Decoder (or a Multiplexer) is lazily created
  upon request. This allows the record fields of the bus interface to
  be up-to-date with any address space extension that may have occured.

* The memory_map attribute of a bus interface is no longer assigned
  during __init__. It is instead assigned externally at a later time.
  Decoupling the memory map from the bus interface allows a Decoder (or
  a Multiplexer) to first create the memory map, extend it, and then
  use it to create the bus interface, once requested.

* Decoder.add(extend=True) (or Multiplexer.add) can be used to add
  a window (or resource) that would otherwise not fit inside its
  underlying memory map.

Author: Jean-François Nguyen

nmigen_soc/csr/bus.py

@@ -104,8 +104,6 @@ class Interface(Record):
         Address width. At most ``(2 ** addr_width) * data_width`` register bits will be available.
     data_width : int
         Data width. Registers are accessed in ``data_width`` sized chunks.
-    alignment : int
-        Register and window alignment. See :class:`MemoryMap`.
     name : str
         Name of the underlying record.
 
@@ -132,7 +130,7 @@ class Interface(Record):
         nothing.
     """
 
-    def __init__(self, *, addr_width, data_width, alignment=0, name=None):
+    def __init__(self, *, addr_width, data_width, name=None):
         if not isinstance(addr_width, int) or addr_width <= 0:
             raise ValueError("Address width must be a positive integer, not {!r}"
                              .format(addr_width))
@@ -141,8 +139,7 @@ def __init__(self, *, addr_width, data_width, alignment=0, name=None):
                              .format(data_width))
         self.addr_width = addr_width
         self.data_width = data_width
-        self.memory_map = MemoryMap(addr_width=addr_width, data_width=data_width,
-                                    alignment=alignment)
+        self._map       = None
 
         super().__init__([
             ("addr",    addr_width),
@@ -152,6 +149,29 @@ def __init__(self, *, addr_width, data_width, alignment=0, name=None):
             ("w_stb",   1),
         ], name=name, src_loc_at=1)
 
+    @property
+    def memory_map(self):
+        if self._map is None:
+            raise NotImplementedError("Bus interface {!r} does not have a memory map"
+                                      .format(self))
+        return self._map
+
+    @memory_map.setter
+    def memory_map(self, memory_map):
+        if not isinstance(memory_map, MemoryMap):
+            raise TypeError("Memory map must be an instance of MemoryMap, not {!r}"
+                            .format(memory_map))
+        if memory_map.addr_width != self.addr_width:
+            raise ValueError("Memory map has address width {}, which is not the same as "
+                             "bus interface address width {}"
+                             .format(memory_map.addr_width, self.addr_width))
+        if memory_map.data_width != self.data_width:
+            raise ValueError("Memory map has data width {}, which is not the same as "
+                             "bus interface data width {}"
+                             .format(memory_map.data_width, self.data_width))
+        memory_map.freeze()
+        self._map = memory_map
+
 
 class Multiplexer(Elaboratable):
     """CSR register multiplexer.
@@ -201,9 +221,18 @@ class Multiplexer(Elaboratable):
         CSR bus providing access to registers.
     """
     def __init__(self, *, addr_width, data_width, alignment=0):
-        self.bus  = Interface(addr_width=addr_width, data_width=data_width, alignment=alignment,
-                              name="csr")
-        self._map = self.bus.memory_map
+        self._map = MemoryMap(addr_width=addr_width, data_width=data_width, alignment=alignment)
+        self._bus = None
+
+    @property
+    def bus(self):
+        if self._bus is None:
+            self._map.freeze()
+            self._bus = Interface(addr_width=self._map.addr_width,
+                                  data_width=self._map.data_width,
+                                  name="csr")
+            self._bus.memory_map = self._map
+        return self._bus
 
     def align_to(self, alignment):
         """Align the implicit address of the next register.
@@ -212,7 +241,7 @@ def align_to(self, alignment):
         """
         return self._map.align_to(alignment)
 
-    def add(self, element, *, addr=None, alignment=None):
+    def add(self, element, *, addr=None, alignment=None, extend=False):
         """Add a register.
 
         See :meth:`MemoryMap.add_resource` for details.
@@ -221,8 +250,9 @@ def add(self, element, *, addr=None, alignment=None):
             raise TypeError("Element must be an instance of csr.Element, not {!r}"
                             .format(element))
 
-        size = (element.width + self.bus.data_width - 1) // self.bus.data_width
-        return self._map.add_resource(element, size=size, addr=addr, alignment=alignment)
+        size = (element.width + self._map.data_width - 1) // self._map.data_width
+        return self._map.add_resource(element, size=size, addr=addr, alignment=alignment,
+                                      extend=extend)
 
     def elaborate(self, platform):
         m = Module()
@@ -306,32 +336,41 @@ class Decoder(Elaboratable):
         CSR bus providing access to subordinate buses.
     """
     def __init__(self, *, addr_width, data_width, alignment=0):
-        self.bus   = Interface(addr_width=addr_width, data_width=data_width, alignment=alignment,
-                               name="csr")
-        self._map  = self.bus.memory_map
+        self._map  = MemoryMap(addr_width=addr_width, data_width=data_width, alignment=alignment)
+        self._bus  = None
         self._subs = dict()
 
+    @property
+    def bus(self):
+        if self._bus is None:
+            self._map.freeze()
+            self._bus = Interface(addr_width=self._map.addr_width,
+                                  data_width=self._map.data_width,
+                                  name="csr")
+            self._bus.memory_map = self._map
+        return self._bus
+
     def align_to(self, alignment):
         """Align the implicit address of the next window.
 
         See :meth:`MemoryMap.align_to` for details.
         """
         return self._map.align_to(alignment)
 
-    def add(self, sub_bus, *, addr=None):
+    def add(self, sub_bus, *, addr=None, extend=False):
         """Add a window to a subordinate bus.
 
         See :meth:`MemoryMap.add_resource` for details.
         """
         if not isinstance(sub_bus, Interface):
             raise TypeError("Subordinate bus must be an instance of csr.Interface, not {!r}"
                             .format(sub_bus))
-        if sub_bus.data_width != self.bus.data_width:
+        if sub_bus.data_width != self._map.data_width:
             raise ValueError("Subordinate bus has data width {}, which is not the same as "
                              "decoder data width {}"
-                             .format(sub_bus.data_width, self.bus.data_width))
+                             .format(sub_bus.data_width, self._map.data_width))
         self._subs[sub_bus.memory_map] = sub_bus
-        return self._map.add_window(sub_bus.memory_map, addr=addr)
+        return self._map.add_window(sub_bus.memory_map, addr=addr, extend=extend)
 
     def elaborate(self, platform):
         m = Module()

nmigen_soc/csr/wishbone.py

@@ -3,6 +3,7 @@
 
 from . import Interface as CSRInterface
 from ..wishbone import Interface as WishboneInterface
+from ..memory import MemoryMap
 
 
 __all__ = ["WishboneCSRBridge"]
@@ -49,6 +50,9 @@ def __init__(self, csr_bus, *, data_width=None):
             granularity=csr_bus.data_width,
             name="wb")
 
+        self.wb_bus.memory_map = MemoryMap(addr_width=csr_bus.addr_width,
+                                           data_width=csr_bus.data_width)
+
         # Since granularity of the Wishbone interface matches the data width of the CSR bus,
         # no width conversion is performed, even if the Wishbone data width is greater.
         self.wb_bus.memory_map.add_window(self.csr_bus.memory_map)

nmigen_soc/test/test_csr_bus.py

@@ -6,6 +6,7 @@
 from nmigen.back.pysim import *
 
 from ..csr.bus import *
+from ..memory import MemoryMap
 
 
 class ElementTestCase(unittest.TestCase):
@@ -83,6 +84,41 @@ def test_wrong_data_width(self):
                 r"Data width must be a positive integer, not -1"):
             Interface(addr_width=16, data_width=-1)
 
+    def test_get_map_wrong(self):
+        iface = Interface(addr_width=16, data_width=8)
+        with self.assertRaisesRegex(NotImplementedError,
+                r"Bus interface \(rec iface addr r_data r_stb w_data w_stb\) does not "
+                r"have a memory map"):
+            iface.memory_map
+
+    def test_get_map_frozen(self):
+        iface = Interface(addr_width=16, data_width=8)
+        iface.memory_map = MemoryMap(addr_width=16, data_width=8)
+        with self.assertRaisesRegex(ValueError,
+                r"Memory map has been frozen. Address width cannot be extended "
+                r"further"):
+            iface.memory_map.addr_width = 24
+
+    def test_set_map_wrong(self):
+        iface = Interface(addr_width=16, data_width=8)
+        with self.assertRaisesRegex(TypeError,
+                r"Memory map must be an instance of MemoryMap, not 'foo'"):
+            iface.memory_map = "foo"
+
+    def test_set_map_wrong_addr_width(self):
+        iface = Interface(addr_width=16, data_width=8)
+        with self.assertRaisesRegex(ValueError,
+                r"Memory map has address width 8, which is not the same as "
+                r"bus interface address width 16"):
+            iface.memory_map = MemoryMap(addr_width=8, data_width=8)
+
+    def test_set_map_wrong_data_width(self):
+        iface = Interface(addr_width=16, data_width=8)
+        with self.assertRaisesRegex(ValueError,
+                r"Memory map has data width 16, which is not the same as "
+                r"bus interface data width 8"):
+            iface.memory_map = MemoryMap(addr_width=16, data_width=16)
+
 
 class MultiplexerTestCase(unittest.TestCase):
     def setUp(self):
@@ -110,6 +146,11 @@ def test_add_two(self):
         self.assertEqual(self.dut.add(Element(8, "rw")),
                          (2, 3))
 
+    def test_add_extend(self):
+        self.assertEqual(self.dut.add(Element(8, "rw"), addr=0x10000, extend=True),
+                         (0x10000, 0x10001))
+        self.assertEqual(self.dut.bus.addr_width, 17)
+
     def test_add_wrong(self):
         with self.assertRaisesRegex(TypeError,
                 r"Element must be an instance of csr\.Element, not 'foo'"):
@@ -122,6 +163,12 @@ def test_align_to(self):
         self.assertEqual(self.dut.add(Element(8, "rw")),
                          (4, 5))
 
+    def test_add_wrong_out_of_bounds(self):
+        with self.assertRaisesRegex(ValueError,
+                r"Address range 0x10000\.\.0x10001 out of bounds for memory map spanning "
+                r"range 0x0\.\.0x10000 \(16 address bits\)"):
+            self.dut.add(Element(8, "rw"), addr=0x10000)
+
     def test_sim(self):
         bus = self.dut.bus
 
@@ -263,11 +310,21 @@ def setUp(self):
         self.dut = Decoder(addr_width=16, data_width=8)
 
     def test_align_to(self):
-        self.assertEqual(self.dut.add(Interface(addr_width=10, data_width=8)),
-                         (0, 0x400, 1))
+        sub_1 = Interface(addr_width=10, data_width=8)
+        sub_1.memory_map = MemoryMap(addr_width=10, data_width=8)
+        self.assertEqual(self.dut.add(sub_1), (0, 0x400, 1))
+
         self.assertEqual(self.dut.align_to(12), 0x1000)
-        self.assertEqual(self.dut.add(Interface(addr_width=10, data_width=8)),
-                         (0x1000, 0x1400, 1))
+
+        sub_2 = Interface(addr_width=10, data_width=8)
+        sub_2.memory_map = MemoryMap(addr_width=10, data_width=8)
+        self.assertEqual(self.dut.add(sub_2), (0x1000, 0x1400, 1))
+
+    def test_add_extend(self):
+        iface = Interface(addr_width=17, data_width=8)
+        iface.memory_map = MemoryMap(addr_width=17, data_width=8)
+        self.assertEqual(self.dut.add(iface, extend=True), (0, 0x20000, 1))
+        self.assertEqual(self.dut.bus.addr_width, 18)
 
     def test_add_wrong_sub_bus(self):
         with self.assertRaisesRegex(TypeError,
@@ -283,6 +340,14 @@ def test_add_wrong_data_width(self):
                 r"decoder data width 8"):
             self.dut.add(mux.bus)
 
+    def test_add_wrong_out_of_bounds(self):
+        iface = Interface(addr_width=17, data_width=8)
+        iface.memory_map = MemoryMap(addr_width=17, data_width=8)
+        with self.assertRaisesRegex(ValueError,
+                r"Address range 0x0\.\.0x20000 out of bounds for memory map spanning "
+                r"range 0x0\.\.0x10000 \(16 address bits\)"):
+            self.dut.add(iface)
+
     def test_sim(self):
         mux_1  = Multiplexer(addr_width=10, data_width=8)
         self.dut.add(mux_1.bus)