From 8d00e15835fced6c9d5707fc06d13c5b309e6ccc Mon Sep 17 00:00:00 2001 From: David Lenfesty Date: Sun, 5 Mar 2023 11:42:47 -0700 Subject: [PATCH] gateware: fix dumb mistakes Bus widths should now all be correct. (addresses and sizes in amaranth take into account the granularity selection, so granularity acts like two extra bits on the bus address line) Also don't ignore a warning about undriven resets. Turns out that can gate *everything* off in your design. --- gateware/eth.py | 11 +++++++---- gateware/led.py | 4 ++-- gateware/main.py | 26 +++++++++++++++----------- gateware/memory.py | 10 ++++++---- 4 files changed, 30 insertions(+), 21 deletions(-) diff --git a/gateware/eth.py b/gateware/eth.py index ffe4855..fe2c223 100644 --- a/gateware/eth.py +++ b/gateware/eth.py @@ -12,10 +12,10 @@ class LiteEth(Elaboratable, Interface): def __init__(self, eth_interface): self.eth_interface = eth_interface - # Addr width is 13 bits to accomodate 0x1FFF, which is well p - Interface.__init__(self, addr_width=13, data_width=32, granularity=8, features=["cti", "bte", "err"]) + # Addr width is 13 bits to accomodate 0x1FFF, which is well past what we care about + Interface.__init__(self, addr_width=15, data_width=32, granularity=8, features=["cti", "bte", "err"]) # TODO I need to understand the semantics here better - memory_map = MemoryMap(addr_width=15, data_width=8) + memory_map = MemoryMap(addr_width=17, data_width=8) #memory_map.add_resource(self, name="LiteETH", size=0x2000) self.memory_map = memory_map @@ -25,7 +25,8 @@ class LiteEth(Elaboratable, Interface): # TODO this really shouldn't technically happen here, because we can elaborate one module multiple times, # but since I use it once it isn't actually a problem. def elaborate(self, platform): - platform.add_file("liteeth_core.v", open("liteeth/gateware/liteeth_core.v", 'r').read()) + if platform is not None: + platform.add_file("liteeth_core.v", open("liteeth/gateware/liteeth_core.v", 'r').read()) m = Module() @@ -34,6 +35,7 @@ class LiteEth(Elaboratable, Interface): core = Instance( "liteeth_core", i_sys_clock=ClockSignal(), + i_sys_reset=Const(0), # RGMII signals o_rgmii_eth_clocks_tx=self.eth_interface.tx_clk, @@ -54,6 +56,7 @@ class LiteEth(Elaboratable, Interface): o_wishbone_dat_r=self.dat_r, i_wishbone_sel=self.sel, i_wishbone_cyc=self.cyc, + i_wishbone_stb=self.stb, o_wishbone_ack=self.ack, i_wishbone_we=self.we, i_wishbone_cti=self.cti, diff --git a/gateware/led.py b/gateware/led.py index 8f2fc7e..3dc3542 100644 --- a/gateware/led.py +++ b/gateware/led.py @@ -5,8 +5,8 @@ from amaranth_soc.memory import * class LEDPeripheral(Elaboratable, Interface): def __init__(self, led_signal): - Interface.__init__(self, addr_width=1, data_width=32) - memory_map = MemoryMap(addr_width=1, data_width=32) + Interface.__init__(self, addr_width=1, data_width=32, granularity=8) + memory_map = MemoryMap(addr_width=3, data_width=8) #memory_map.add_resource("my_led", name="led_peripheral", size=1) self.memory_map = memory_map diff --git a/gateware/main.py b/gateware/main.py index ca85793..0fe0381 100644 --- a/gateware/main.py +++ b/gateware/main.py @@ -41,9 +41,9 @@ def load_firmware_for_mem() -> List[int]: class Core(Elaboratable): def __init__(self, clk25, led_signal, eth_interface): self.count = Signal(64) - self.cpu = Minerva(reset_address=0x01000000) - self.arbiter = Arbiter(addr_width=32, data_width=32) - self.decoder = Decoder(addr_width=32, data_width=32, features=["err"]) + self.cpu = Minerva(reset_address=0x01000000, with_debug=False) + self.arbiter = Arbiter(addr_width=30, data_width=32, granularity=8) + self.decoder = Decoder(addr_width=30, data_width=32, granularity=8, features=["err"]) self.clk25 = clk25 self.led_signal = led_signal self.eth_interface = eth_interface @@ -55,7 +55,8 @@ class Core(Elaboratable): m.submodules.decoder = self.decoder # Create main and sampling clock, using PLL and 25MHz input clock - platform.add_file("pll.v", open("pll.v", "r").read()) + if platform is not None: + platform.add_file("pll.v", open("pll.v", "r").read()) sys_clk = Signal() sample_clk = Signal() pll = Instance( @@ -79,16 +80,15 @@ class Core(Elaboratable): # Connect ibus and dbus together for simplicity for now minerva_wb_features = ["cti", "bte", "err"] - self.ibus = Interface(addr_width=32, data_width=32, features=minerva_wb_features) + self.ibus = Interface(addr_width=30, data_width=32, granularity=8, features=minerva_wb_features) # Note this is a set of statements! without assigning to the comb domain, this will do nothing m.d.comb += self.cpu.ibus.connect(self.ibus) self.arbiter.add(self.ibus) - self.dbus = Interface(addr_width=32, data_width=32, features=minerva_wb_features) + self.dbus = Interface(addr_width=30, data_width=32, granularity=8, features=minerva_wb_features) m.d.comb += self.cpu.dbus.connect(self.dbus) # Don't use .eq, use .connect, which will appropriately assign signals # using .eq() gave me Multiple Driven errors self.arbiter.add(self.dbus) - # TODO do something with interrupts # These are interrupts headed into the CPU self.interrupts = Signal(32) @@ -111,7 +111,7 @@ class Core(Elaboratable): m.submodules.rom = self.rom # Problem: not sure to handle how we do byte vs word addressing properly # So doing this shift is a bit of a hacky way to impl anything - start, _stop, _step = self.decoder.add(self.rom, addr=(0x01000000 >> 2)) + start, _stop, _step = self.decoder.add(self.rom, addr=0x01000000) print(f"ROM added at 0x{start:08x}") self.ram = RAM() @@ -129,7 +129,7 @@ class Core(Elaboratable): # Ethernet self.eth = LiteEth(self.eth_interface) m.submodules.eth = self.eth - start, _stop, _step = self.decoder.add(self.eth, addr=0x00500000) + start, _stop, _step = self.decoder.add(self.eth, addr=0x02000000) print(f"LiteETH added at 0x{start:08x}") # Connect arbiter to decoder @@ -167,12 +167,12 @@ def run_sim(): sim = Simulator(dut) def proc(): - for i in range(10000): + for i in range(1000): yield Tick() sim.add_clock(1e-6) sim.add_sync_process(proc) - with sim.write_vcd('test.vcd', gtkw_file='test.gtkw'): + with sim.write_vcd('sim.vcd', gtkw_file='sim.gtkw'): sim.reset() sim.run() @@ -184,6 +184,7 @@ if __name__ == "__main__": # TODO maybe allow an optional arg to specify an individual test to run? args.add_argument("--test", action="store_true", help="Run RTL test suite and report results.") args.add_argument("--save-vcd", action="store_true", help="Save VCD waveforms from test(s).") + args.add_argument("--sim", action="store_true", help="Run overall simulation") args = args.parse_args() if args.build: @@ -203,3 +204,6 @@ if __name__ == "__main__": for mod in test_modules: unittest.main(module=mod, argv=[sys.argv[0]]) + if args.sim: + run_sim() + diff --git a/gateware/memory.py b/gateware/memory.py index fb98d28..915b7fc 100644 --- a/gateware/memory.py +++ b/gateware/memory.py @@ -2,6 +2,8 @@ from amaranth import * from amaranth_soc.wishbone import * from amaranth_soc.memory import * +# TODO impl select + # We sub-class wishbone.Interface here because it needs to be a bus object to be added as a window to Wishbone stuff class ROM(Elaboratable, Interface): @@ -11,11 +13,11 @@ class ROM(Elaboratable, Interface): self.r = self.data.read_port() # Need to init Interface - Interface.__init__(self, addr_width=10, data_width=32) + Interface.__init__(self, addr_width=10, data_width=32, granularity=8) # This is effectively a "window", and it has a certain set of resources # 12 = log2(4096) - memory_map = MemoryMap(addr_width=10, data_width=32) + memory_map = MemoryMap(addr_width=12, data_width=8) # TODO need to unify how I deal with size # In this case, one resource, which is out memory memory_map.add_resource(self.data, name="rom_data", size=(4096 >> 2)) @@ -57,11 +59,11 @@ class RAM(Elaboratable, Interface): self.w = self.data.write_port() # Need to init Interface - Interface.__init__(self, addr_width=10, data_width=32) + Interface.__init__(self, addr_width=10, data_width=32, granularity=8) # This is effectively a "window", and it has a certain set of resources # 12 = log2(4096) - memory_map = MemoryMap(addr_width=10, data_width=32) + memory_map = MemoryMap(addr_width=12, data_width=8) # TODO need to unify how I deal with size # In this case, one resource, which is out memory memory_map.add_resource(self.data, name="ram_data", size=(4096 >> 2))