Merge pull request #10 from andreaskuster/code_review_bluewww

Code review bluewww

.github/workflows/main.yml

@@ -20,4 +20,4 @@ jobs:
        - uses: actions/checkout@v2
        - run: |
            source setup_env.sh
-           pytest tb/
+           pytest tests/

Bender.yml

@@ -25,7 +25,7 @@ sources:
   # Sources
   ##########
   # Top level
-  - src/dut.sv
+  - tb/dut.sv
   - src/axi_io_pmp.sv
   # AXI connector
   - src/connector/axi_master_connector.sv

Makefile

@@ -18,10 +18,10 @@ SHELL := /bin/bash
 all: bender_install bender_dl bender_gen_src sim wave questa_coverage_report
 
 sim:
-	pytest tb/ 
+	pytest tests/ 
 
 sim_mt:
-	pytest tb/ -n $(shell nproc)
+	pytest tests/ -n $(shell nproc)
 
 wave:
 	gtkwave sim_build/axi_io_pmp.vcd

extras/Makefile

@@ -45,7 +45,7 @@ sources :=  common_cells/src/lzc.sv \
 			pmp/pmp_entry.sv \
 			pmp/pmp.sv \
 			axi_io_pmp.sv \
-			dut.sv
+			../tb/dut.sv
 list_sources := $(foreach dir, ${sources}, ../src/$(dir))
 
 

setup_env.sh

@@ -27,6 +27,7 @@ xargs sudo apt-get -y install < ubuntu_requirements.txt
 git submodule update --init --recursive
 python3 -m venv venv/
 source venv/bin/activate
+python3 -m pip install wheel
 python3 -m pip install -r requirements.txt
 
 # setup questasim

src/axi_io_pmp.sv

@@ -11,19 +11,18 @@
 // Author:      Andreas Kuster, <[email protected]>
 // Description: Traditional style AXI IO-PMP module
 
-`timescale 1ns / 1ps
 
 module axi_io_pmp #(
     // width of data bus in bits
-    parameter              DATA_WIDTH     = 64,
+    parameter int unsigned DATA_WIDTH     = 64,
     // width of address bus in bits
-    parameter              ADDR_WIDTH     = 64,
+    parameter int unsigned ADDR_WIDTH     = 64,
     // width of strobe (width of data bus in words)
-    parameter              STRB_WIDTH     = (DATA_WIDTH / 8),
+    parameter int unsigned STRB_WIDTH     = (DATA_WIDTH / 8),
     // width of id signal
-    parameter              ID_WIDTH       = 8,
+    parameter int unsigned ID_WIDTH       = 8,
     // propagate awuser signal
-    parameter              USER_WIDTH     = 0,
+    parameter int unsigned USER_WIDTH     = 0,
     // AXI channel structs
     parameter type         axi_aw_chan_t  = logic,
     parameter type         axi_w_chan_t   = logic,
@@ -59,7 +58,7 @@ module axi_io_pmp #(
     // configuration port
     input  reg_req_t cfg_req_i,
     output reg_rsp_t cfg_rsp_o,
-    // enable test and dev modes of the different modules
+    // enable test/dev modes of the different modules
     input  logic     devmode_i
 );
 
@@ -116,10 +115,10 @@ module axi_io_pmp #(
   io_pmp_reg_top #(
       .reg_req_t(reg_req_t),
       .reg_rsp_t(reg_rsp_t)
-  ) io_pmp_reg_top0 (
-      .clk_i    (clk_i),
-      .rst_ni   (rst_ni),
-      .devmode_i(devmode_i),     // if 1, explicit error return for unmapped register access
+  ) i_io_pmp_reg_top (
+      .clk_i,
+      .rst_ni,
+      .devmode_i,  // if 1, explicit error return for unmapped register access
       // register interface
       .reg_req_i(cfg_req_mod),
       .reg_rsp_o(cfg_rsp_mod),
@@ -199,7 +198,7 @@ module axi_io_pmp #(
       .PMP_LEN       (PMP_LEN),
       .NR_ENTRIES    (NR_ENTRIES),
       .PMPGranularity(PMPGranularity)
-  ) pmp0 (
+  ) i_read_pmp (
       // input
       .addr_i       (pmp_addr_r),              // [PLEN-1:0], TODO: check if we slice the right bits
       .access_type_i(riscv::ACCESS_READ),      // handle read accesses
@@ -282,7 +281,7 @@ module axi_io_pmp #(
       .PMP_LEN       (PMP_LEN),
       .NR_ENTRIES    (NR_ENTRIES),
       .PMPGranularity(PMPGranularity)
-  ) pmp1 (
+  ) i_write_pmp (
       // input
       .addr_i       (pmp_addr_w),              // [PLEN-1:0], TODO: check if we slice the right bits
       .access_type_i(riscv::ACCESS_WRITE),     // handle write accesses
@@ -318,9 +317,9 @@ module axi_io_pmp #(
       .SpillB     (1'b0),
       .SpillAr    (1'b0),
       .SpillR     (1'b0)
-  ) axi_demux0 (
-      .clk_i          (clk_i),
-      .rst_ni         (rst_ni),
+  ) i_axi_demux (
+      .clk_i,
+      .rst_ni,
       .test_i         (testmode_i),
       .slv_aw_select_i(allow_w),
       .slv_ar_select_i(allow_r),
@@ -343,18 +342,25 @@ module axi_io_pmp #(
       .RespData(64'hCA11AB1EBADCAB1E),  // hexvalue for data return value
       .ATOPs(1'b1),
       .MaxTrans(1)
-  ) i_err_slv (
-      .clk_i     (clk_i),
-      .rst_ni    (rst_ni),
-      .test_i    (1'b0),
+  ) i_axi_err_slv (
+      .clk_i,
+      .rst_ni,
+      .test_i    (testmode_i),
       .slv_req_i (error_req),
       .slv_resp_o(error_rsp)
   );
 
-  // check IO-PMP granularity restrictions
-  initial begin
-    assert (PMPGranularity >= 10)
-    else $fatal(1, "AXI IO-PMP only supports granularity >= 4K");
-  end
+
+  //
+  // Assumptions and assertions
+  //
+`ifndef VERILATOR
+`ifndef SYNTHESIS
+  // pragma translate_off
+  if (PMPGranularity < 10)  // check IO-PMP granularity restrictions
+    $fatal(1, "AXI IO-PMP only supports granularity >= 4K");
+  // pragma translate_on
+`endif
+`endif
 
 endmodule

src/connector/axi_master_connector.sv

@@ -11,30 +11,28 @@
 // Author:      Andreas Kuster, <[email protected]>
 // Description: AXI master to (req_t, resp_t) pair connector (pulp-platform interface)
 
-`timescale 1ns / 1ps
-
 module axi_master_connector #(
     // width of data bus in bits
-    parameter      DATA_WIDTH   = 32,
+    parameter int unsigned DATA_WIDTH   = 32,
     // width of address bus in bits
-    parameter      ADDR_WIDTH   = 32,
+    parameter int unsigned ADDR_WIDTH   = 32,
     // width of strobe (width of data bus in words)
-    parameter      STRB_WIDTH   = (DATA_WIDTH / 8),
+    parameter int unsigned STRB_WIDTH   = (DATA_WIDTH / 8),
     // width of id signal
-    parameter      ID_WIDTH     = 8,
+    parameter int unsigned ID_WIDTH     = 8,
     // width of awuser signal
-    parameter      AWUSER_WIDTH = 1,
+    parameter int unsigned AWUSER_WIDTH = 1,
     // width of wuser signal
-    parameter      WUSER_WIDTH  = 1,
+    parameter int unsigned WUSER_WIDTH  = 1,
     // width of buser signal
-    parameter      BUSER_WIDTH  = 1,
+    parameter int unsigned BUSER_WIDTH  = 1,
     // width of aruser signal
-    parameter      ARUSER_WIDTH = 1,
+    parameter int unsigned ARUSER_WIDTH = 1,
     // width of ruser signal
-    parameter      RUSER_WIDTH  = 1,
+    parameter int unsigned RUSER_WIDTH  = 1,
     // AXI request/response
-    parameter type axi_req_t    = logic,
-    parameter type axi_rsp_t    = logic
+    parameter type         axi_req_t    = logic,
+    parameter type         axi_rsp_t    = logic
 ) (
     //
     // Write address channel

src/connector/axi_slave_connector.sv

@@ -11,30 +11,28 @@
 // Author:      Andreas Kuster, <[email protected]>
 // Description: AXI slave to (req_t, resp_t) pair connector (pulp-platform interface)
 
-`timescale 1ns / 1ps
-
 module axi_slave_connector #(
     // width of data bus in bits
-    parameter      DATA_WIDTH   = 32,
+    parameter int unsigned DATA_WIDTH   = 32,
     // width of address bus in bits
-    parameter      ADDR_WIDTH   = 32,
+    parameter int unsigned ADDR_WIDTH   = 32,
     // width of strobe (width of data bus in words)
-    parameter      STRB_WIDTH   = (DATA_WIDTH / 8),
+    parameter int unsigned STRB_WIDTH   = (DATA_WIDTH / 8),
     // width of id signal
-    parameter      ID_WIDTH     = 8,
+    parameter int unsigned ID_WIDTH     = 8,
     // width of awuser signal
-    parameter      AWUSER_WIDTH = 1,
+    parameter int unsigned AWUSER_WIDTH = 1,
     // width of wuser signal
-    parameter      WUSER_WIDTH  = 1,
+    parameter int unsigned WUSER_WIDTH  = 1,
     // width of buser signal
-    parameter      BUSER_WIDTH  = 1,
+    parameter int unsigned BUSER_WIDTH  = 1,
     // width of aruser signal
-    parameter      ARUSER_WIDTH = 1,
+    parameter int unsigned ARUSER_WIDTH = 1,
     // width of ruser signal
-    parameter      RUSER_WIDTH  = 1,
+    parameter int unsigned RUSER_WIDTH  = 1,
     // AXI request/response
-    parameter type axi_req_t    = logic,
-    parameter type axi_rsp_t    = logic
+    parameter type         axi_req_t    = logic,
+    parameter type         axi_rsp_t    = logic
 ) (
     //
     // Write address channel

src/connector/reg_cut.sv

@@ -1,18 +1,31 @@
+// Copyright 2022 ETH Zurich and University of Bologna.
+// Copyright and related rights are licensed under the Solderpad Hardware
+// License, Version 0.51 (the "License"); you may not use this file except in
+// compliance with the License.  You may obtain a copy of the License at
+// http://solderpad.org/licenses/SHL-0.51. Unless required by applicable law
+// or agreed to in writing, software, hardware and materials distributed under
+// this License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
+// CONDITIONS OF ANY KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations under the License.
+//
+// Author:      Andreas Kuster, <[email protected]>
+// Description: Register interface cut (breaks combinatorial paths)
+
 module reg_cut #(
     // make this register transparent
     parameter bit  Bypass    = 1'b0,
     // register interface request/response
     parameter type reg_req_t = logic,
     parameter type reg_rsp_t = logic
 ) (
-    input logic clk_i,
-    input logic rst_ni,
+    input  logic     clk_i,
+    input  logic     rst_ni,
     // input
-    input reg_req_t req_in,
+    input  reg_req_t req_in,
     output reg_rsp_t rsp_in,
     // output
     output reg_req_t req_out,
-    input reg_rsp_t rsp_out
+    input  reg_rsp_t rsp_out
 );
 
   if (Bypass) begin : gen_bypass