| From 50b4c6afc918c715a9884048e687d15801426cc1 Mon Sep 17 00:00:00 2001 |
| From: Michael Schaffner <msf@google.com> |
| Date: Mon, 13 Jul 2020 19:25:04 -0700 |
| Subject: [PATCH 1/2] Fix various lint warnings |
| |
| Fix style and width lint warnings emitted by Verible Lint and Verilator |
| Lint. |
| |
| The debug rom has been regenerated from the assembly files with `make |
| GCC=riscv32-unknown-elf-gcc OBJCOPY=riscv32-unknown-elf-objcopy |
| OBJDUMP=riscv32-unknown-elf-objdump`. |
| |
| Signed-off-by: Michael Schaffner <msf@google.com> |
| Signed-off-by: Philipp Wagner <phw@lowrisc.org> |
| |
| diff --git a/debug_rom/debug_rom.sv b/debug_rom/debug_rom.sv |
| index a0e0208..0299db6 100644 |
| --- a/debug_rom/debug_rom.sv |
| +++ b/debug_rom/debug_rom.sv |
| @@ -23,7 +23,8 @@ module debug_rom ( |
| |
| localparam int unsigned RomSize = 19; |
| |
| - const logic [RomSize-1:0][63:0] mem = { |
| + logic [RomSize-1:0][63:0] mem; |
| + assign mem = { |
| 64'h00000000_7b200073, |
| 64'h7b202473_7b302573, |
| 64'h10852423_f1402473, |
| diff --git a/debug_rom/debug_rom_one_scratch.sv b/debug_rom/debug_rom_one_scratch.sv |
| index 78d47be..af7e67c 100644 |
| --- a/debug_rom/debug_rom_one_scratch.sv |
| +++ b/debug_rom/debug_rom_one_scratch.sv |
| @@ -23,7 +23,8 @@ module debug_rom_one_scratch ( |
| |
| localparam int unsigned RomSize = 13; |
| |
| - const logic [RomSize-1:0][63:0] mem = { |
| + logic [RomSize-1:0][63:0] mem; |
| + assign mem = { |
| 64'h00000000_7b200073, |
| 64'h7b202473_10802423, |
| 64'hf1402473_ab1ff06f, |
| diff --git a/debug_rom/gen_rom.py b/debug_rom/gen_rom.py |
| index b3ce019..b8cb60b 100755 |
| --- a/debug_rom/gen_rom.py |
| +++ b/debug_rom/gen_rom.py |
| @@ -50,7 +50,8 @@ module $filename ( |
| |
| localparam int unsigned RomSize = $size; |
| |
| - const logic [RomSize-1:0][63:0] mem = { |
| + logic [RomSize-1:0][63:0] mem; |
| + assign mem = { |
| $content |
| }; |
| |
| @@ -131,4 +132,3 @@ with open(filename + ".sv", "w") as f: |
| f.write(license) |
| s = Template(module) |
| f.write(s.substitute(filename=filename, size=int(len(rom)/8), content=rom_str)) |
| - |
| diff --git a/src/dm_csrs.sv b/src/dm_csrs.sv |
| index 73722a3..4b0f254 100644 |
| --- a/src/dm_csrs.sv |
| +++ b/src/dm_csrs.sv |
| @@ -91,8 +91,8 @@ module dm_csrs #( |
| logic resp_queue_pop; |
| logic [31:0] resp_queue_data; |
| |
| - localparam dm::dm_csr_e DataEnd = dm::dm_csr_e'(dm::Data0 + {4'b0, dm::DataCount} - 8'h01); |
| - localparam dm::dm_csr_e ProgBufEnd = dm::dm_csr_e'(dm::ProgBuf0 + {4'b0, dm::ProgBufSize} - 8'h01); |
| + localparam dm::dm_csr_e DataEnd = dm::dm_csr_e'(dm::Data0 + {4'h0, dm::DataCount} - 8'h1); |
| + localparam dm::dm_csr_e ProgBufEnd = dm::dm_csr_e'(dm::ProgBuf0 + {4'h0, dm::ProgBufSize} - 8'h1); |
| |
| logic [31:0] haltsum0, haltsum1, haltsum2, haltsum3; |
| logic [((NrHarts-1)/2**5 + 1) * 32 - 1 : 0] halted; |
| @@ -211,10 +211,18 @@ module dm_csrs #( |
| end |
| |
| // helper variables |
| + dm::dm_csr_e dm_csr_addr; |
| dm::sbcs_t sbcs; |
| dm::dmcontrol_t dmcontrol; |
| dm::abstractcs_t a_abstractcs; |
| - logic [4:0] autoexecdata_idx; |
| + logic [3:0] autoexecdata_idx; // 0 == Data0 ... 11 == Data11 |
| + |
| + // Get the data index, i.e. 0 for dm::Data0 up to 11 for dm::Data11 |
| + assign dm_csr_addr = dm::dm_csr_e'({1'b0, dmi_req_i.addr}); |
| + // Xilinx Vivado 2020.1 does not allow subtraction of two enums; do the subtraction with logic |
| + // types instead. |
| + assign autoexecdata_idx = 4'({dm_csr_addr} - {dm::Data0}); |
| + |
| always_comb begin : csr_read_write |
| // -------------------- |
| // Static Values (R/O) |
| @@ -271,7 +279,7 @@ module dm_csrs #( |
| sbaddr_d = 64'(sbaddress_i); |
| sbdata_d = sbdata_q; |
| |
| - resp_queue_data = 32'b0; |
| + resp_queue_data = 32'h0; |
| cmd_valid_d = 1'b0; |
| sbaddress_write_valid_o = 1'b0; |
| sbdata_read_valid_o = 1'b0; |
| @@ -283,24 +291,17 @@ module dm_csrs #( |
| dmcontrol = '0; |
| a_abstractcs = '0; |
| |
| - autoexecdata_idx = dmi_req_i.addr[4:0] - 5'(dm::Data0); |
| - |
| - // localparam int unsigned DataCountAlign = $clog2(dm::DataCount); |
| // reads |
| if (dmi_req_ready_o && dmi_req_valid_i && dtm_op == dm::DTM_READ) begin |
| - unique case ({1'b0, dmi_req_i.addr}) inside |
| + unique case (dm_csr_addr) inside |
| [(dm::Data0):DataEnd]: begin |
| - // logic [$clog2(dm::DataCount)-1:0] resp_queue_idx; |
| - // resp_queue_idx = dmi_req_i.addr[4:0] - int'(dm::Data0); |
| resp_queue_data = data_q[$clog2(dm::DataCount)'(autoexecdata_idx)]; |
| if (!cmdbusy_i) begin |
| // check whether we need to re-execute the command (just give a cmd_valid) |
| - if (autoexecdata_idx < $bits(abstractauto_q.autoexecdata)) begin |
| - cmd_valid_d = abstractauto_q.autoexecdata[autoexecdata_idx]; |
| - end |
| - //An abstract command was executing while one of the data registers was read |
| + cmd_valid_d = abstractauto_q.autoexecdata[autoexecdata_idx]; |
| + // An abstract command was executing while one of the data registers was read |
| end else if (cmderr_q == dm::CmdErrNone) begin |
| - cmderr_d = dm::CmdErrBusy; |
| + cmderr_d = dm::CmdErrBusy; |
| end |
| end |
| dm::DMControl: resp_queue_data = dmcontrol_q; |
| @@ -316,8 +317,8 @@ module dm_csrs #( |
| // check whether we need to re-execute the command (just give a cmd_valid) |
| // range of autoexecprogbuf is 31:16 |
| cmd_valid_d = abstractauto_q.autoexecprogbuf[{1'b1, dmi_req_i.addr[3:0]}]; |
| - |
| - //An abstract command was executing while one of the progbuf registers was read |
| + |
| + // An abstract command was executing while one of the progbuf registers was read |
| end else if (cmderr_q == dm::CmdErrNone) begin |
| cmderr_d = dm::CmdErrBusy; |
| end |
| @@ -358,16 +359,14 @@ module dm_csrs #( |
| |
| // write |
| if (dmi_req_ready_o && dmi_req_valid_i && dtm_op == dm::DTM_WRITE) begin |
| - unique case (dm::dm_csr_e'({1'b0, dmi_req_i.addr})) inside |
| + unique case (dm_csr_addr) inside |
| [(dm::Data0):DataEnd]: begin |
| if (dm::DataCount > 0) begin |
| // attempts to write them while busy is set does not change their value |
| if (!cmdbusy_i) begin |
| data_d[dmi_req_i.addr[$clog2(dm::DataCount)-1:0]] = dmi_req_i.data; |
| // check whether we need to re-execute the command (just give a cmd_valid) |
| - if (autoexecdata_idx < $bits(abstractauto_q.autoexecdata)) begin |
| - cmd_valid_d = abstractauto_q.autoexecdata[autoexecdata_idx]; |
| - end |
| + cmd_valid_d = abstractauto_q.autoexecdata[autoexecdata_idx]; |
| //An abstract command was executing while one of the data registers was written |
| end else if (cmderr_q == dm::CmdErrNone) begin |
| cmderr_d = dm::CmdErrBusy; |
| @@ -412,7 +411,7 @@ module dm_csrs #( |
| dm::AbstractAuto: begin |
| // this field can only be written legally when there is no command executing |
| if (!cmdbusy_i) begin |
| - abstractauto_d = 32'b0; |
| + abstractauto_d = 32'h0; |
| abstractauto_d.autoexecdata = 12'(dmi_req_i.data[dm::DataCount-1:0]); |
| abstractauto_d.autoexecprogbuf = 16'(dmi_req_i.data[dm::ProgBufSize-1+16:16]); |
| end else if (cmderr_q == dm::CmdErrNone) begin |
| @@ -526,7 +525,7 @@ module dm_csrs #( |
| dmcontrol_d.resumereq = 1'b0; |
| end |
| // static values for dcsr |
| - sbcs_d.sbversion = 3'b1; |
| + sbcs_d.sbversion = 3'd1; |
| sbcs_d.sbbusy = sbbusy_i; |
| sbcs_d.sbasize = $bits(sbcs_d.sbasize)'(BusWidth); |
| sbcs_d.sbaccess128 = 1'b0; |
| @@ -543,7 +542,7 @@ module dm_csrs #( |
| // default assignment |
| haltreq_o = '0; |
| resumereq_o = '0; |
| - if (selected_hart < (HartSelLen+1)'(NrHarts)) begin |
| + if (selected_hart <= HartSelLen'(NrHarts-1)) begin |
| haltreq_o[selected_hart] = dmcontrol_q.haltreq; |
| resumereq_o[selected_hart] = dmcontrol_q.resumereq; |
| end |
| diff --git a/src/dm_mem.sv b/src/dm_mem.sv |
| index 302998b..178259f 100755 |
| --- a/src/dm_mem.sv |
| +++ b/src/dm_mem.sv |
| @@ -336,7 +336,8 @@ module dm_mem #( |
| abstract_cmd[0][31:0] = dm::illegal(); |
| // load debug module base address into a0, this is shared among all commands |
| abstract_cmd[0][63:32] = HasSndScratch ? dm::auipc(5'd10, '0) : dm::nop(); |
| - abstract_cmd[1][31:0] = HasSndScratch ? dm::srli(5'd10, 5'd10, 6'd12) : dm::nop(); // clr lowest 12b -> DM base offset |
| + // clr lowest 12b -> DM base offset |
| + abstract_cmd[1][31:0] = HasSndScratch ? dm::srli(5'd10, 5'd10, 6'd12) : dm::nop(); |
| abstract_cmd[1][63:32] = HasSndScratch ? dm::slli(5'd10, 5'd10, 6'd12) : dm::nop(); |
| abstract_cmd[2][31:0] = dm::nop(); |
| abstract_cmd[2][63:32] = dm::nop(); |
| @@ -395,7 +396,9 @@ module dm_mem #( |
| end |
| end else if (32'(ac_ar.aarsize) < MaxAar && ac_ar.transfer && !ac_ar.write) begin |
| // store a0 in dscratch1 |
| - abstract_cmd[0][31:0] = HasSndScratch ? dm::csrw(dm::CSR_DSCRATCH1, LoadBaseAddr) : dm::nop(); |
| + abstract_cmd[0][31:0] = HasSndScratch ? |
| + dm::csrw(dm::CSR_DSCRATCH1, LoadBaseAddr) : |
| + dm::nop(); |
| // this range is reserved |
| if (ac_ar.regno[15:14] != '0) begin |
| abstract_cmd[0][31:0] = dm::ebreak(); // we leave asap |
| diff --git a/src/dm_obi_top.sv b/src/dm_obi_top.sv |
| index 2f7ee7a..b1906ec 100644 |
| --- a/src/dm_obi_top.sv |
| +++ b/src/dm_obi_top.sv |
| @@ -61,42 +61,48 @@ |
| //////////////////////////////////////////////////////////////////////////////// |
| |
| module dm_obi_top #( |
| - parameter int unsigned IdWidth = 1, // Width of aid/rid |
| + parameter int unsigned IdWidth = 1, // Width of aid/rid |
| parameter int unsigned NrHarts = 1, |
| parameter int unsigned BusWidth = 32, |
| - parameter int unsigned DmBaseAddress = 'h1000, // default to non-zero page |
| + parameter int unsigned DmBaseAddress = 'h1000, // default to non-zero page |
| // Bitmask to select physically available harts for systems |
| // that don't use hart numbers in a contiguous fashion. |
| parameter logic [NrHarts-1:0] SelectableHarts = {NrHarts{1'b1}} |
| ) ( |
| - input logic clk_i, // clock |
| - input logic rst_ni, // asynchronous reset active low, connect PoR here, not the system reset |
| + input logic clk_i, // clock |
| + // asynchronous reset active low, connect PoR here, not the system reset |
| + input logic rst_ni, |
| input logic testmode_i, |
| - output logic ndmreset_o, // non-debug module reset |
| - output logic dmactive_o, // debug module is active |
| - output logic [NrHarts-1:0] debug_req_o, // async debug request |
| - input logic [NrHarts-1:0] unavailable_i, // communicate whether the hart is unavailable (e.g.: power down) |
| - input dm::hartinfo_t [NrHarts-1:0] hartinfo_i, |
| + output logic ndmreset_o, // non-debug module reset |
| + output logic dmactive_o, // debug module is active |
| + output logic [NrHarts-1:0] debug_req_o, // async debug request |
| + // communicate whether the hart is unavailable (e.g.: power down) |
| + input logic [NrHarts-1:0] unavailable_i, |
| + input dm::hartinfo_t [NrHarts-1:0] hartinfo_i, |
| |
| input logic slave_req_i, |
| - output logic slave_gnt_o, // OBI grant for slave_req_i (not present on dm_top) |
| + // OBI grant for slave_req_i (not present on dm_top) |
| + output logic slave_gnt_o, |
| input logic slave_we_i, |
| input logic [BusWidth-1:0] slave_addr_i, |
| input logic [BusWidth/8-1:0] slave_be_i, |
| input logic [BusWidth-1:0] slave_wdata_i, |
| - input logic [IdWidth-1:0] slave_aid_i, // Address phase transaction identifier (not present on dm_top) |
| - output logic slave_rvalid_o, // OBI rvalid signal (end of response phase for reads/writes) (not present on dm_top) |
| + // Address phase transaction identifier (not present on dm_top) |
| + input logic [IdWidth-1:0] slave_aid_i, |
| + // OBI rvalid signal (end of response phase for reads/writes) (not present on dm_top) |
| + output logic slave_rvalid_o, |
| output logic [BusWidth-1:0] slave_rdata_o, |
| - output logic [IdWidth-1:0] slave_rid_o, // Response phase transaction identifier (not present on dm_top) |
| + // Response phase transaction identifier (not present on dm_top) |
| + output logic [IdWidth-1:0] slave_rid_o, |
| |
| output logic master_req_o, |
| - output logic [BusWidth-1:0] master_addr_o, // Renamed according to OBI spec |
| + output logic [BusWidth-1:0] master_addr_o, // Renamed according to OBI spec |
| output logic master_we_o, |
| output logic [BusWidth-1:0] master_wdata_o, |
| output logic [BusWidth/8-1:0] master_be_o, |
| input logic master_gnt_i, |
| - input logic master_rvalid_i, // Renamed according to OBI spec |
| - input logic [BusWidth-1:0] master_rdata_i, // Renamed according to OBI spec |
| + input logic master_rvalid_i, // Renamed according to OBI spec |
| + input logic [BusWidth-1:0] master_rdata_i, // Renamed according to OBI spec |
| |
| // Connection to DTM - compatible to RocketChip Debug Module |
| input logic dmi_rst_ni, |
| @@ -137,13 +143,13 @@ module dm_obi_top #( |
| .slave_rdata_o ( slave_rdata_o ), |
| |
| .master_req_o ( master_req_o ), |
| - .master_add_o ( master_addr_o ), // Renamed according to OBI spec |
| + .master_add_o ( master_addr_o ), // Renamed according to OBI spec |
| .master_we_o ( master_we_o ), |
| .master_wdata_o ( master_wdata_o ), |
| .master_be_o ( master_be_o ), |
| .master_gnt_i ( master_gnt_i ), |
| - .master_r_valid_i ( master_rvalid_i ), // Renamed according to OBI spec |
| - .master_r_rdata_i ( master_rdata_i ), // Renamed according to OBI spec |
| + .master_r_valid_i ( master_rvalid_i ), // Renamed according to OBI spec |
| + .master_r_rdata_i ( master_rdata_i ), // Renamed according to OBI spec |
| |
| .dmi_rst_ni ( dmi_rst_ni ), |
| .dmi_req_valid_i ( dmi_req_valid_i ), |
| @@ -165,16 +171,16 @@ module dm_obi_top #( |
| slave_rvalid_q <= 1'b0; |
| slave_rid_q <= 'b0; |
| end else begin |
| - if (slave_req_i && slave_gnt_o) begin // 1 cycle pulse on rvalid for every granted request |
| + if (slave_req_i && slave_gnt_o) begin // 1 cycle pulse on rvalid for every granted request |
| slave_rvalid_q <= 1'b1; |
| - slave_rid_q <= slave_aid_i; // Mirror aid to rid |
| + slave_rid_q <= slave_aid_i; // Mirror aid to rid |
| end else begin |
| - slave_rvalid_q <= 1'b0; // rid is don't care if rvalid = 0 |
| + slave_rvalid_q <= 1'b0; // rid is don't care if rvalid = 0 |
| end |
| end |
| end |
| |
| - assign slave_gnt_o = 1'b1; // Always receptive to request (slave_req_i) |
| + assign slave_gnt_o = 1'b1; // Always receptive to request (slave_req_i) |
| assign slave_rvalid_o = slave_rvalid_q; |
| assign slave_rid_o = slave_rid_q; |
| |
| diff --git a/src/dm_pkg.sv b/src/dm_pkg.sv |
| index dbbc031..971f128 100644 |
| --- a/src/dm_pkg.sv |
| +++ b/src/dm_pkg.sv |
| @@ -215,7 +215,7 @@ package dm; |
| logic sbaccess8; |
| } sbcs_t; |
| |
| - localparam logic[1:0] DTM_SUCCESS = 2'h0; |
| + localparam logic [1:0] DTM_SUCCESS = 2'h0; |
| |
| typedef struct packed { |
| logic [6:0] addr; |
| diff --git a/src/dm_sba.sv b/src/dm_sba.sv |
| index 8f98cdc..98c586c 100644 |
| --- a/src/dm_sba.sv |
| +++ b/src/dm_sba.sv |
| @@ -77,7 +77,7 @@ module dm_sba #( |
| be_mask[int'({be_idx[$high(be_idx):1], 1'b0}) +: 2] = '1; |
| end |
| 3'b010: begin |
| - if (BusWidth == 32'd64) be_mask[int'({be_idx[$high(be_idx)], 2'b0}) +: 4] = '1; |
| + if (BusWidth == 32'd64) be_mask[int'({be_idx[$high(be_idx)], 2'h0}) +: 4] = '1; |
| else be_mask = '1; |
| end |
| 3'b011: be_mask = '1; |
| @@ -125,7 +125,7 @@ module dm_sba #( |
| if (sbdata_valid_o) begin |
| state_d = dm::Idle; |
| // auto-increment address |
| - if (sbautoincrement_i) sbaddress_o = sbaddress_i + (32'b1 << sbaccess_i); |
| + if (sbautoincrement_i) sbaddress_o = sbaddress_i + (32'h1 << sbaccess_i); |
| end |
| end |
| |
| @@ -133,7 +133,7 @@ module dm_sba #( |
| if (sbdata_valid_o) begin |
| state_d = dm::Idle; |
| // auto-increment address |
| - if (sbautoincrement_i) sbaddress_o = sbaddress_i + (32'b1 << sbaccess_i); |
| + if (sbautoincrement_i) sbaddress_o = sbaddress_i + (32'h1 << sbaccess_i); |
| end |
| end |
| |
| diff --git a/tb/boot_rom.sv b/tb/boot_rom.sv |
| index 3610af4..7c214d9 100644 |
| --- a/tb/boot_rom.sv |
| +++ b/tb/boot_rom.sv |
| @@ -20,7 +20,8 @@ module boot_rom ( |
| localparam int RomSize = 2; |
| localparam logic [31:0] entry_addr = 32'h1c00_0080; |
| |
| - const logic [RomSize-1:0][31:0] mem = { |
| + logic [RomSize-1:0][31:0] mem; |
| + assign mem = { |
| dm_tb_pkg::jalr(5'h0, 5'h1, entry_addr[11:0]), |
| dm_tb_pkg::lui(5'h1, entry_addr[31:12]) |
| }; |
