ROM memory in SystemVerilog and cocotb

Question

The Verilog module describes a ROM memory. An initialization file is needed for the INIT_FILE parameter.

Below are Makefile, gtkwave.tcl to launch gtkwave, launch_tb.py (please use it to start simulation) and rom_tb.py as well as rom.sv.

Refer also to the cocotb home page.

rom.sv

module rom #
(
  parameter integer DATA_WIDTH = 8,
  parameter integer ADDR_WIDTH = 8,
  
  parameter string  INIT_FILE  = ""
)
(
  input  logic                      clk_i,
  input  logic [ADDR_WIDTH - 1 : 0] addr_i,

  output logic [ADDR_WIDTH - 1 : 0] data_o
);

  localparam integer MEM_DEPTH = 2 ** ADDR_WIDTH;

  logic [ADDR_WIDTH - 1 : 0] data;
  logic [ADDR_WIDTH - 1 : 0] rom_mem [0 : MEM_DEPTH - 1];

  initial 
    begin
      if (INIT_FILE != "")
        begin
          $display("loading rom");
          $readmemh(INIT_FILE, rom_mem);
        end
      else
        begin
          $error("init file is needed");
        end
    end

  always_ff @ (posedge clk_i)
    begin
      data <= rom_mem[addr_i];
    end

  always_comb
    begin
      data_o = data;
    end

  initial begin
    $dumpfile("dump.vcd");
    $dumpvars(1, rom);
  end

endmodule

Makefile

SIM?=icarus
TOPLEVEL_LANG?=verilog
WAVES?=0

COCOTB_HDL_TIMEUNIT=1ns
COCOTB_HDL_TIMEPRECISION=1ns

SRC_DIR=../../rtl
DUT=rom
TOPLEVEL=$(DUT)
MODULE=$(DUT)_tb
VERILOG_SOURCES+=$(SRC_DIR)/$(DUT).sv

DATA_WIDTH?=8
ADDR_WIDTH?=3
INIT_FILE?=$(shell pwd)/rom_init.txt

COMPILE_ARGS+=-P$(TOPLEVEL).DATA_WIDTH=$(DATA_WIDTH)
COMPILE_ARGS+=-P$(TOPLEVEL).ADDR_WIDTH=$(ADDR_WIDTH)
COMPILE_ARGS+=-P$(TOPLEVEL).INIT_FILE=$(INIT_FILE)

ifeq ($(WAVES), 1)
    VERILOG_SOURCES+=iverilog_dump.v
    COMPILE_ARGS+=-s iverilog_dump
else
    GFLAGS=-S gtkwave.tcl
endif

include $(shell cocotb-config --makefiles)/Makefile.sim

all:
    echo $(COMPILE_ARGS)
    gtkwave $(GFLAGS) *.vcd 2>/dev/null || $(GTKWAVE_OSX) $(GFLAGS) *.vcd 2>/dev/null

iverilog_dump.v:
    echo 'module iverilog_dump();' > $@
    echo 'initial begin' >> $@
    echo '    $$dumpfile("$(TOPLEVEL).fst");' >> $@
    echo '    $$dumpvars(0, $(TOPLEVEL));' >> $@
    echo 'end' >> $@
    echo 'endmodule' >> $@

clean::
    rm -rf *.vcd

rom_tb.py

#!/usr/bin/python

import random

import cocotb
from cocotb.clock import Clock
from cocotb.triggers import RisingEdge


class Rom_tb(object):
    def __init__(self, dut):
        self.dut = dut
        cocotb.start_soon(Clock(self.dut.clk_i, 10, units="ns").start())

    async def read_addr(self, addr):
        self.dut.addr_i.value = addr
        await RisingEdge(self.dut.clk_i)
        await RisingEdge(self.dut.clk_i)
        return self.dut.data_o.value

    async def read_rand_addr(self, max_range):
        rand_addr = random.randint(0, max_range)
        cocotb.log.info(f"Random address: {rand_addr}  Data: {await self.read_addr(rand_addr)}")

    #todo: compare with file content 
    async def reading_whole_mem(self, max_range):
        for addr in range(0, max_range):
            cocotb.log.info(f"Address: {addr}  Data: {await self.read_addr(addr)}")
            

@cocotb.test()
async def main(dut):
    highest_addr = 8
    max_rand_val = 7

    cocotb.log.info("Start of simulation")

    tb = Rom_tb(dut)
    await tb.reading_whole_mem(highest_addr)
    await tb.read_rand_addr(max_rand_val)
    await tb.read_rand_addr(max_rand_val)

    cocotb.log.info("End of simulation")
        
if __name__ == "__main__":
    sys.exit(main())

gtkwave.tcl

set nfacs [ gtkwave::getNumFacs ]
set all_facs [list]
for {set i 0} {$i < $nfacs } {incr i} {
    set facname [ gtkwave::getFacName $i ]
    lappend all_facs "$facname"
}
set num_added [ gtkwave::addSignalsFromList $all_facs ]
puts "num signals added: $num_added"

# zoom full
gtkwave::/Time/Zoom/Zoom_Full

# Print
set dumpname [ gtkwave::getDumpFileName ]
gtkwave::/File/Print_To_File PDF {Letter (8.5" x 11")} Minimal $dumpname.pdf

launch_tb.py

#!/usr/bin/python
import subprocess

print("cleaning...")
subprocess.run(["make", "clean"])
print("launching the tb")
subprocess.run(["make"])

Answer 1

This review is limited to the SystemVerilog code (rom.sv).

You declared the DATA_WIDTH parameter, but your code does not use it. I assume you really want to use it for the declarations of the data and memory variables. You probably didn't notice since you assigned the WIDTH parameters to the same value (8).

Otherwise, the code is easy to understand, uses descriptive variable names and makes good use of parameters for constant values.

As far as the code layout goes, I prefer to make better use of vertical space by having the begin and end keywords "cuddled" on lines with other meaningful code, as you did with one of your initial blocks. I also prefer 4 singles spaces for indentation over 2.

module rom #
(
    parameter integer DATA_WIDTH = 8,
    parameter integer ADDR_WIDTH = 8,
  
    parameter string  INIT_FILE  = ""
)
(
    input  logic                      clk_i,
    input  logic [ADDR_WIDTH - 1 : 0] addr_i,

    output logic [DATA_WIDTH - 1 : 0] data_o
);

    localparam integer MEM_DEPTH = 2 ** ADDR_WIDTH;

    logic [DATA_WIDTH - 1 : 0] data;
    logic [DATA_WIDTH - 1 : 0] rom_mem [0 : MEM_DEPTH - 1];

    initial begin
        if (INIT_FILE != "") begin
            $display("loading rom");
            $readmemh(INIT_FILE, rom_mem);
        end else begin
            $error("init file is needed");
        end
    end

    always_ff @ (posedge clk_i) begin
        data <= rom_mem[addr_i];
    end

    always_comb begin
        data_o = data;
    end

    initial begin
        $dumpfile("dump.vcd");
        $dumpvars(1, rom);
    end
endmodule