Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make optional properties return None while uninitialized. #26

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions nmigen_soc/csr/bus.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,6 @@ class Interface(Record):

Attributes
----------
memory_map : MemoryMap
Map of the bus.
addr : Signal(addr_width)
Address for reads and writes.
r_data : Signal(data_width)
Expand Down Expand Up @@ -151,9 +149,21 @@ def __init__(self, *, addr_width, data_width, name=None):

@property
def memory_map(self):
if self._map is None:
raise NotImplementedError("Bus interface {!r} does not have a memory map"
.format(self))
"""Map of the bus.

See :class:`..memory.MemoryMap` for details.

Return value
------------
A :class:`~..memory.MemoryMap` describing the target address space, or ``None`` if the
interface does not have a memory map.

Exceptions
----------
Raises :exn:`ValueError` if the setter is called with a memory map whose address width is
not equal to the interface address width, or whose data width is not equal to the interface
data width.
"""
return self._map

@memory_map.setter
Expand Down Expand Up @@ -370,6 +380,9 @@ def add(self, sub_bus, *, addr=None, extend=False):
raise ValueError("Subordinate bus has data width {}, which is not the same as "
"decoder data width {}"
.format(sub_bus.data_width, self._map.data_width))

if sub_bus.memory_map is None:
raise ValueError("Subordinate bus does not have a memory map")
self._subs[sub_bus.memory_map] = sub_bus
return self._map.add_window(sub_bus.memory_map, addr=addr, extend=extend)

Expand Down
3 changes: 3 additions & 0 deletions nmigen_soc/csr/wishbone.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ def __init__(self, csr_bus, *, data_width=None):
self.wb_bus.memory_map = MemoryMap(addr_width=csr_bus.addr_width,
data_width=csr_bus.data_width)

if self.csr_bus.memory_map is None:
raise ValueError("CSR bus does not have a memory map")

# 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)
Expand Down
10 changes: 2 additions & 8 deletions nmigen_soc/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,9 @@ def event_map(self):

Return value
------------
A :class:`EventMap` describing subordinate event sources.

Exceptions
----------
Raises :exn:`NotImplementedError` if the source does not have an event map.
An :class:`EventMap` describing subordinate event sources, or ``None`` if the source does
not have an event map.
"""
if self._map is None:
raise NotImplementedError("Event source {!r} does not have an event map"
.format(self))
return self._map

@event_map.setter
Expand Down
11 changes: 2 additions & 9 deletions nmigen_soc/periph.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,16 +176,9 @@ def irq(self):

Return value
------------
An :class:`event.Source` used by the peripheral to request interrupts. If provided, its
event map describes local events.

Exceptions
----------
Raises :exn:`NotImplementedError` if the peripheral info does not have an IRQ line.
An :class:`event.Source` used by the peripheral to request interrupts, or ``None`` if the
peripheral does not have an IRQ line. If provided, its event map describes local events.
"""
if self._irq is None:
raise NotImplementedError("Peripheral info does not have an IRQ line"
.format(self))
return self._irq

@property
Expand Down
13 changes: 8 additions & 5 deletions nmigen_soc/test/test_csr_bus.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,9 @@ 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):
def test_get_map_none(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
self.assertEqual(iface.memory_map, None)

def test_get_map_frozen(self):
iface = Interface(addr_width=16, data_width=8)
Expand Down Expand Up @@ -351,6 +348,12 @@ def test_add_wrong_out_of_bounds(self):
r"range 0x0\.\.0x10000 \(16 address bits\)"):
self.dut.add(iface)

def test_add_wrong_no_map(self):
iface = Interface(addr_width=10, data_width=8)
with self.assertRaisesRegex(ValueError,
r"Subordinate bus does not have a memory map"):
self.dut.add(iface)

def test_sim(self):
mux_1 = Multiplexer(addr_width=10, data_width=8)
self.dut.add(mux_1.bus)
Expand Down
11 changes: 10 additions & 1 deletion nmigen_soc/test/test_csr_wishbone.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from nmigen.back.pysim import *

from .. import csr
from ..memory import MemoryMap
from ..csr.wishbone import *


Expand Down Expand Up @@ -36,9 +37,17 @@ def test_wrong_csr_bus(self):
WishboneCSRBridge("foo")

def test_wrong_csr_bus_data_width(self):
csr_bus = csr.Interface(addr_width=10, data_width=7)
csr_bus.memory_map = MemoryMap(addr_width=10, data_width=7)
with self.assertRaisesRegex(ValueError,
r"CSR bus data width must be one of 8, 16, 32, 64, not 7"):
WishboneCSRBridge(csr_bus=csr.Interface(addr_width=10, data_width=7))
WishboneCSRBridge(csr_bus)

def test_wrong_csr_bus_no_map(self):
csr_bus = csr.Interface(addr_width=10, data_width=8)
with self.assertRaisesRegex(ValueError,
r"CSR bus does not have a memory map"):
WishboneCSRBridge(csr_bus)

def test_narrow(self):
mux = csr.Multiplexer(addr_width=10, data_width=8)
Expand Down
6 changes: 2 additions & 4 deletions nmigen_soc/test/test_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,9 @@ def test_trigger_wrong(self):
r"Invalid trigger mode 'foo'; must be one of level, rise, fall"):
src = Source(trigger="foo")

def test_get_map_wrong(self):
def test_get_map_none(self):
src = Source()
with self.assertRaisesRegex(NotImplementedError,
r"Event source \(rec src i trg\) does not have an event map"):
src.event_map
self.assertEqual(src.event_map, None)

def test_get_map_frozen(self):
src = Source()
Expand Down
8 changes: 2 additions & 6 deletions nmigen_soc/test/test_periph.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,16 +122,12 @@ def test_irq(self):
def test_irq_none(self):
memory_map = MemoryMap(addr_width=1, data_width=8)
info = PeripheralInfo(memory_map=memory_map, irq=None)
with self.assertRaisesRegex(NotImplementedError,
r"Peripheral info does not have an IRQ line"):
info.irq
self.assertEqual(info.irq, None)

def test_irq_default(self):
memory_map = MemoryMap(addr_width=1, data_width=8)
info = PeripheralInfo(memory_map=memory_map)
with self.assertRaisesRegex(NotImplementedError,
r"Peripheral info does not have an IRQ line"):
info.irq
self.assertEqual(info.irq, None)

def test_irq_wrong(self):
memory_map = MemoryMap(addr_width=1, data_width=8)
Expand Down
13 changes: 8 additions & 5 deletions nmigen_soc/test/test_wishbone_bus.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,9 @@ def test_wrong_features(self):
r"Optional signal\(s\) 'foo' are not supported"):
Interface(addr_width=0, data_width=8, features={"foo"})

def test_get_map_wrong(self):
def test_get_map_none(self):
iface = Interface(addr_width=0, data_width=8)
with self.assertRaisesRegex(NotImplementedError,
r"Bus interface \(rec iface adr dat_w dat_r sel cyc stb we ack\) does "
r"not have a memory map"):
iface.memory_map
self.assertEqual(iface.memory_map, None)

def test_get_map_frozen(self):
iface = Interface(addr_width=1, data_width=8)
Expand Down Expand Up @@ -180,6 +177,12 @@ def test_add_wrong_out_of_bounds(self):
r"range 0x0\.\.0x100000000 \(32 address bits\)"):
self.dut.add(sub, addr=1)

def test_add_wrong_no_map(self):
sub = Interface(addr_width=31, data_width=32, granularity=16)
with self.assertRaisesRegex(ValueError,
r"Subordinate bus does not have a memory map"):
self.dut.add(sub)


class DecoderSimulationTestCase(unittest.TestCase):
def test_simple(self):
Expand Down
28 changes: 21 additions & 7 deletions nmigen_soc/wishbone/bus.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ class Interface(Record):
of the Wishbone signals. The ``RST_I`` and ``CLK_I`` signals are provided as a part of
the clock domain that drives the interface.

Note that the data width of the underlying memory map of the interface is equal to port
granularity, not port size. If port granularity is less than port size, then the address width
of the underlying memory map is extended to reflect that.

Parameters
----------
addr_width : int
Expand Down Expand Up @@ -145,9 +141,25 @@ def __init__(self, *, addr_width, data_width, granularity=None, features=frozens

@property
def memory_map(self):
if self._map is None:
raise NotImplementedError("Bus interface {!r} does not have a memory map"
.format(self))
"""Map of the bus.

See :class:`..memory.MemoryMap` for details.

Note that the data width of the memory map must be equal to port granularity, not port
size. If port granularity is less than port size, then the address width of the memory map
must be extended to ``self.addr_width + log2(self.data_width / self.granularity)``.

Return value
------------
A :class:`~..memory.MemoryMap` describing the target address space, or ``None`` if the
interface does not have a memory map.

Exceptions
----------
Raises :exn:`ValueError` if the setter is called with a memory map whose data width is not
equal to port granularity, or whose address width is not equal to the extended port
address width.
"""
return self._map

@memory_map.setter
Expand Down Expand Up @@ -263,6 +275,8 @@ def add(self, sub_bus, *, addr=None, sparse=False, extend=False):
"does not have a corresponding input"
.format(opt_output))

if sub_bus.memory_map is None:
raise ValueError("Subordinate bus does not have a memory map")
self._subs[sub_bus.memory_map] = sub_bus
return self._map.add_window(sub_bus.memory_map, addr=addr, sparse=sparse, extend=extend)

Expand Down