Skip to content

Commit e6b4002

Browse files
committed
memory: remove support for address space extension.
The extend= parameter of MemoryMap.add_resource()/.add_window() was added in 2d18ac7. However, it prevents interfaces whose shape depend on a memory map from being instantiated until the latter is frozen. Also, bus interfaces may have a strict requirement on their address width, and it should not be possible to implicitly increase it by adding a subordinate resource or window. As a side-effect of this change, csr.EventMonitor requires a populated EventMap in order to be instantiated.
1 parent a872c1b commit e6b4002

File tree

9 files changed

+72
-190
lines changed

9 files changed

+72
-190
lines changed

amaranth_soc/csr/bus.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ def align_to(self, alignment):
577577
"""
578578
return self._map.align_to(alignment)
579579

580-
def add(self, element, *, name, addr=None, alignment=None, extend=False):
580+
def add(self, element, *, name, addr=None, alignment=None):
581581
"""Add a register.
582582
583583
See :meth:`MemoryMap.add_resource` for details.
@@ -587,7 +587,7 @@ def add(self, element, *, name, addr=None, alignment=None, extend=False):
587587

588588
size = (element.width + self._map.data_width - 1) // self._map.data_width
589589
return self._map.add_resource(element, size=size, addr=addr, alignment=alignment,
590-
extend=extend, name=name)
590+
name=name)
591591

592592
def elaborate(self, platform):
593593
m = Module()
@@ -721,7 +721,7 @@ def align_to(self, alignment):
721721
"""
722722
return self._map.align_to(alignment)
723723

724-
def add(self, sub_bus, *, addr=None, extend=False):
724+
def add(self, sub_bus, *, addr=None):
725725
"""Add a window to a subordinate bus.
726726
727727
See :meth:`MemoryMap.add_resource` for details.
@@ -732,7 +732,7 @@ def add(self, sub_bus, *, addr=None, extend=False):
732732
raise ValueError(f"Subordinate bus has data width {sub_bus.data_width}, which is not "
733733
f"the same as decoder data width {self._map.data_width}")
734734
self._subs[sub_bus.memory_map] = sub_bus
735-
return self._map.add_window(sub_bus.memory_map, addr=addr, extend=extend)
735+
return self._map.add_window(sub_bus.memory_map, addr=addr)
736736

737737
def elaborate(self, platform):
738738
m = Module()

amaranth_soc/csr/event.py

Lines changed: 24 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
# amaranth: UnusedElaboratable=no
2+
from math import ceil
23

34
from amaranth import *
5+
from amaranth.utils import log2_int
46

57
from . import Element, Multiplexer
68
from .. import event
@@ -16,50 +18,40 @@ class EventMonitor(Elaboratable):
1618
1719
CSR registers
1820
-------------
19-
enable : ``self.src.event_map.size``, read/write
21+
enable : ``event_map.size``, read/write
2022
Enabled events. See :meth:`..event.EventMap.sources` for layout.
21-
pending : ``self.src.event_map.size``, read/clear
23+
pending : ``event_map.size``, read/clear
2224
Pending events. See :meth:`..event.EventMap.sources` for layout.
2325
2426
Parameters
2527
----------
28+
event_map : :class:`..event.EventMap`
29+
A collection of event sources.
30+
trigger : :class:`..event.Source.Trigger`
31+
Trigger mode. See :class:`..event.Source`.
2632
data_width : int
2733
CSR bus data width. See :class:`..csr.Interface`.
2834
alignment : int, power-of-2 exponent
2935
CSR address alignment. See :class:`..memory.MemoryMap`.
30-
trigger : :class:`..event.Source.Trigger`
31-
Trigger mode. See :class:`..event.Source`.
3236
name : str
3337
Window name. Optional. See :class:`..memory.MemoryMap`.
3438
"""
35-
def __init__(self, *, data_width, alignment=0, trigger="level", name=None):
36-
choices = ("level", "rise", "fall")
37-
if not isinstance(trigger, event.Source.Trigger) and trigger not in choices:
38-
raise ValueError("Invalid trigger mode {!r}; must be one of {}"
39-
.format(trigger, ", ".join(choices)))
40-
41-
self._trigger = event.Source.Trigger(trigger)
42-
self._map = event.EventMap()
43-
self._monitor = None
44-
self._enable = None
45-
self._pending = None
46-
self._mux = Multiplexer(addr_width=1, data_width=data_width, alignment=alignment,
47-
name=name)
48-
self._frozen = False
49-
50-
def freeze(self):
51-
"""Freeze the event monitor.
52-
53-
Once the event monitor is frozen, subordinate sources cannot be added anymore.
54-
"""
55-
if self._frozen:
56-
return
57-
self._monitor = event.Monitor(self._map, trigger=self._trigger)
58-
self._enable = Element(self._map.size, "rw", path=("enable",))
59-
self._pending = Element(self._map.size, "rw", path=("pending",))
60-
self._mux.add(self._enable, name="enable", extend=True)
61-
self._mux.add(self._pending, name="pending", extend=True)
62-
self._frozen = True
39+
def __init__(self, event_map, *, trigger="level", data_width, alignment=0, name=None):
40+
if not isinstance(data_width, int) or data_width <= 0:
41+
raise ValueError(f"Data width must be a positive integer, not {data_width!r}")
42+
if not isinstance(alignment, int) or alignment < 0:
43+
raise ValueError(f"Alignment must be a non-negative integer, not {alignment!r}")
44+
45+
self._monitor = event.Monitor(event_map, trigger=trigger)
46+
self._enable = Element(event_map.size, "rw", path=("enable",))
47+
self._pending = Element(event_map.size, "rw", path=("pending",))
48+
49+
elem_size = ceil(event_map.size / data_width)
50+
addr_width = 1 + max(log2_int(elem_size, need_pow2=False), alignment)
51+
self._mux = Multiplexer(addr_width=addr_width, data_width=data_width,
52+
alignment=alignment, name=name)
53+
self._mux.add(self._enable, name="enable")
54+
self._mux.add(self._pending, name="pending")
6355

6456
@property
6557
def src(self):
@@ -70,7 +62,6 @@ def src(self):
7062
An :class:`..event.Source`. Its input line is asserted by the monitor when a subordinate
7163
event is enabled and pending.
7264
"""
73-
self.freeze()
7465
return self._monitor.src
7566

7667
@property
@@ -81,19 +72,9 @@ def bus(self):
8172
------------
8273
A :class:`..csr.Interface` providing access to registers.
8374
"""
84-
self.freeze()
8575
return self._mux.bus
8676

87-
def add(self, src):
88-
"""Add a subordinate event source.
89-
90-
See :meth:`..event.EventMap.add` for details.
91-
"""
92-
self._map.add(src)
93-
9477
def elaborate(self, platform):
95-
self.freeze()
96-
9778
m = Module()
9879
m.submodules.monitor = self._monitor
9980
m.submodules.mux = self._mux

amaranth_soc/memory.py

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -172,20 +172,6 @@ def __init__(self, *, addr_width, data_width, alignment=0, name=None):
172172
def addr_width(self):
173173
return self._addr_width
174174

175-
@addr_width.setter
176-
def addr_width(self, addr_width):
177-
if self._frozen:
178-
raise ValueError("Memory map has been frozen. Address width cannot be extended "
179-
"further")
180-
if not isinstance(addr_width, int) or addr_width <= 0:
181-
raise ValueError("Address width must be a positive integer, not {!r}"
182-
.format(addr_width))
183-
if addr_width < self._addr_width:
184-
raise ValueError("Address width {!r} must not be less than its previous value {!r}, "
185-
"because resources that were previously added may not fit anymore"
186-
.format(addr_width, self._addr_width))
187-
self._addr_width = addr_width
188-
189175
@property
190176
def data_width(self):
191177
return self._data_width
@@ -202,7 +188,7 @@ def freeze(self):
202188
"""Freeze the memory map.
203189
204190
Once the memory map is frozen, its visible state becomes immutable. Resources and windows
205-
cannot be added anymore, and its address width cannot be extended further.
191+
cannot be added anymore.
206192
"""
207193
self._frozen = True
208194

@@ -231,7 +217,7 @@ def align_to(self, alignment):
231217
self._next_addr = self._align_up(self._next_addr, max(alignment, self.alignment))
232218
return self._next_addr
233219

234-
def _compute_addr_range(self, addr, size, step=1, *, alignment, extend):
220+
def _compute_addr_range(self, addr, size, step=1, *, alignment):
235221
if addr is not None:
236222
if not isinstance(addr, int) or addr < 0:
237223
raise ValueError("Address must be a non-negative integer, not {!r}"
@@ -249,12 +235,9 @@ def _compute_addr_range(self, addr, size, step=1, *, alignment, extend):
249235
size = self._align_up(max(size, 1), alignment)
250236

251237
if addr > (1 << self.addr_width) or addr + size > (1 << self.addr_width):
252-
if extend:
253-
self.addr_width = bits_for(addr + size)
254-
else:
255-
raise ValueError("Address range {:#x}..{:#x} out of bounds for memory map spanning "
256-
"range {:#x}..{:#x} ({} address bits)"
257-
.format(addr, addr + size, 0, 1 << self.addr_width, self.addr_width))
238+
raise ValueError("Address range {:#x}..{:#x} out of bounds for memory map spanning "
239+
"range {:#x}..{:#x} ({} address bits)"
240+
.format(addr, addr + size, 0, 1 << self.addr_width, self.addr_width))
258241

259242
addr_range = range(addr, addr + size, step)
260243
overlaps = self._ranges.overlaps(addr_range)
@@ -274,7 +257,7 @@ def _compute_addr_range(self, addr, size, step=1, *, alignment, extend):
274257

275258
return addr_range
276259

277-
def add_resource(self, resource, *, name, size, addr=None, alignment=None, extend=False):
260+
def add_resource(self, resource, *, name, size, addr=None, alignment=None):
278261
"""Add a resource.
279262
280263
A resource is any device on the bus that is a destination for bus transactions, e.g.
@@ -296,9 +279,6 @@ def add_resource(self, resource, *, name, size, addr=None, alignment=None, exten
296279
``2 ** max(alignment, self.alignment)``.
297280
alignment : int, power-of-2 exponent
298281
Alignment of the resource. Optional. If ``None``, the memory map alignment is used.
299-
extend: bool
300-
Allow memory map extension. If ``True``, the upper bound of the address space is
301-
raised as needed, in order to fit a resource that would otherwise be out of bounds.
302282
303283
Return value
304284
------------
@@ -336,7 +316,7 @@ def add_resource(self, resource, *, name, size, addr=None, alignment=None, exten
336316
else:
337317
alignment = self.alignment
338318

339-
addr_range = self._compute_addr_range(addr, size, alignment=alignment, extend=extend)
319+
addr_range = self._compute_addr_range(addr, size, alignment=alignment)
340320
self._ranges.insert(addr_range, resource)
341321
self._resources[id(resource)] = resource, name, addr_range
342322
self._namespace[name] = resource
@@ -356,7 +336,7 @@ def resources(self):
356336
for resource, resource_name, resource_range in self._resources.values():
357337
yield resource, resource_name, (resource_range.start, resource_range.stop)
358338

359-
def add_window(self, window, *, addr=None, sparse=None, extend=False):
339+
def add_window(self, window, *, addr=None, sparse=None):
360340
"""Add a window.
361341
362342
A window is a device on a bus that provides access to a different bus, i.e. a bus bridge.
@@ -386,9 +366,6 @@ def add_window(self, window, *, addr=None, sparse=None, extend=False):
386366
sparse : bool
387367
Address translation type. Optional. Ignored if the datapath widths of both memory maps
388368
are equal; must be specified otherwise.
389-
extend : bool
390-
Allow memory map extension. If ``True``, the upper bound of the address space is
391-
raised as needed, in order to fit a window that would otherwise be out of bounds.
392369
393370
Return value
394371
------------
@@ -464,8 +441,7 @@ def add_window(self, window, *, addr=None, sparse=None, extend=False):
464441
# a window can still be aligned using align_to().
465442
alignment = max(self.alignment, window.addr_width // ratio)
466443

467-
addr_range = self._compute_addr_range(addr, size, ratio, alignment=alignment,
468-
extend=extend)
444+
addr_range = self._compute_addr_range(addr, size, ratio, alignment=alignment)
469445
window.freeze()
470446
self._ranges.insert(addr_range, window)
471447
self._windows[id(window)] = window, addr_range

amaranth_soc/wishbone/bus.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ def align_to(self, alignment):
342342
"""
343343
return self._map.align_to(alignment)
344344

345-
def add(self, sub_bus, *, addr=None, sparse=False, extend=False):
345+
def add(self, sub_bus, *, addr=None, sparse=False):
346346
"""Add a window to a subordinate bus.
347347
348348
The decoder can perform either sparse or dense address translation. If dense address
@@ -376,7 +376,7 @@ def add(self, sub_bus, *, addr=None, sparse=False, extend=False):
376376
f"decoder does not have a corresponding input")
377377

378378
self._subs[sub_bus.memory_map] = sub_bus
379-
return self._map.add_window(sub_bus.memory_map, addr=addr, sparse=sparse, extend=extend)
379+
return self._map.add_window(sub_bus.memory_map, addr=addr, sparse=sparse)
380380

381381
def elaborate(self, platform):
382382
m = Module()

tests/test_csr_bus.py

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -152,11 +152,10 @@ def test_get_map_frozen(self):
152152
iface = csr.Interface(addr_width=16, data_width=8)
153153
iface.memory_map = MemoryMap(addr_width=16, data_width=8)
154154
with self.assertRaisesRegex(ValueError,
155-
r"Memory map has been frozen. Address width cannot be extended "
156-
r"further"):
157-
iface.memory_map.addr_width = 24
155+
r"Memory map has been frozen\. Cannot add resource 'foo'"):
156+
iface.memory_map.add_resource("foo", name="foo", size=1)
158157

159-
def test_set_map_wrong(self):
158+
def test_set_wrong_map(self):
160159
iface = csr.Interface(addr_width=16, data_width=8)
161160
with self.assertRaisesRegex(TypeError,
162161
r"Memory map must be an instance of MemoryMap, not 'foo'"):
@@ -203,12 +202,6 @@ def test_add_two(self):
203202
self.assertEqual(self.dut.add(elem_16b, name="elem_16b"), (0, 2))
204203
self.assertEqual(self.dut.add(elem_8b, name="elem_8b"), (2, 3))
205204

206-
def test_add_extend(self):
207-
elem_8b = csr.Element(8, "rw", path=("elem_8b",))
208-
self.assertEqual(self.dut.add(elem_8b, name="elem_8b", addr=0x10000, extend=True),
209-
(0x10000, 0x10001))
210-
self.assertEqual(self.dut.bus.addr_width, 17)
211-
212205
def test_add_wrong(self):
213206
with self.assertRaisesRegex(TypeError,
214207
r"Element must be an instance of csr\.Element, not 'foo'"):
@@ -409,12 +402,6 @@ def test_align_to(self):
409402
sub_2.memory_map = MemoryMap(addr_width=10, data_width=8)
410403
self.assertEqual(self.dut.add(sub_2), (0x1000, 0x1400, 1))
411404

412-
def test_add_extend(self):
413-
iface = csr.Interface(addr_width=17, data_width=8, path=("iface",))
414-
iface.memory_map = MemoryMap(addr_width=17, data_width=8)
415-
self.assertEqual(self.dut.add(iface, extend=True), (0, 0x20000, 1))
416-
self.assertEqual(self.dut.bus.addr_width, 18)
417-
418405
def test_add_wrong_sub_bus(self):
419406
with self.assertRaisesRegex(TypeError,
420407
r"Subordinate bus must be an instance of csr\.Interface, not 1"):

0 commit comments

Comments
 (0)