From 5ab57ebb98523a4d093aa53ba64f3487aa9fa130 Mon Sep 17 00:00:00 2001 From: Greg Taylor Date: Wed, 17 Jul 2024 13:23:26 -0400 Subject: [PATCH] Add slang and Verilator linting (#51) * Add slang linting, fix minor lint errors * passes verilator lint checks --- .gitignore | 1 + fpga/Makefile | 8 +- fpga/modules/channels/src/channels.sv | 74 ++++++++++--------- .../modules/channels/src/control_operators.sv | 7 +- fpga/modules/host_if/src/host_if.sv | 4 +- fpga/modules/operator/src/calc_phase_inc.sv | 2 +- .../operator/src/envelope_generator.sv | 1 + fpga/modules/top_level/pkg/opl3_pkg.sv | 6 +- 8 files changed, 57 insertions(+), 46 deletions(-) diff --git a/.gitignore b/.gitignore index 0735b50..6f40bb8 100644 --- a/.gitignore +++ b/.gitignore @@ -25,3 +25,4 @@ fpga/NA/ps7_summary.html fpga/vsim.wlf fpga/modules/operator/vsim.wlf fpga/modules/misc/vsim.wlf +.sv_cache/ \ No newline at end of file diff --git a/fpga/Makefile b/fpga/Makefile index 6d1c8af..457c4dc 100755 --- a/fpga/Makefile +++ b/fpga/Makefile @@ -44,8 +44,6 @@ # Copyright (C) 2010-2013 by carbon14 and opl3 # #****************************************************************************** -uname_O := $(shell sh -c 'uname -o 2>/dev/null || echo not') - BOARD = zybo RTL_SRC = \ @@ -105,7 +103,7 @@ INC_DIR0 = \ compile: test -e work || vlib work - vlog -incr ${PKG_SRC} $(SIM_SRC) $(RTL_SRC) +define+SIM +incdir+$(INC_DIR0) + vlog -incr $(PKG_SRC) $(SIM_SRC) $(RTL_SRC) +define+SIM +incdir+$(INC_DIR0) sim: compile vsim -c opl3_tb -do "run -a" @@ -129,6 +127,10 @@ probes: build/opl3.ltx program: build/opl3.bit vivado -mode batch -source scripts/vivado_program.tcl -log build/program_log.txt -nojournal +lint: $(PKG_SRC) $(RTL_SRC) + slang $(PKG_SRC) $(RTL_SRC) + verilator --lint-only -Wno-WIDTHEXPAND -Wno-WIDTHTRUNC --top-module opl3 $(PKG_SRC) $(RTL_SRC) + clean: rm -rf *.tmp *.log log transcript work *.wlf vsim.fcdb rm -rf *~ core csrc simv* vc_hdrs.h ucli.key urg* *.log core.* synlog.tcl diff --git a/fpga/modules/channels/src/channels.sv b/fpga/modules/channels/src/channels.sv index 5370b0f..c318f44 100755 --- a/fpga/modules/channels/src/channels.sv +++ b/fpga/modules/channels/src/channels.sv @@ -74,6 +74,43 @@ module channels logic cnt; // operator connection logic [REG_FB_WIDTH-1:0] fb_dummy; + enum { + IDLE, + LOAD_2_OP_SECOND_0, + LOAD_2_OP_SECOND_1, + LOAD_2_OP_FIRST_AND_ACCUMULATE, + LOAD_4_OP_THIRD_0, + LOAD_4_OP_THIRD_1, + LOAD_4_OP_SECOND, + LOAD_4_OP_FIRST_AND_ACCUMULATE, + DONE + } state = IDLE, next_state; + + struct packed { + logic cnt_second; + logic signed [OP_OUT_WIDTH-1:0] operator_out_third; + logic signed [OP_OUT_WIDTH-1:0] operator_out_second; + logic bank_num; + logic [$clog2(NUM_OPERATORS_PER_BANK)-1:0] op_num; + logic [$clog2(NUM_CHANNELS_PER_BANK)-1:0] channel_num; + logic signed [CHANNEL_ACCUMULATOR_WIDTH-1:0] channel_l_acc_pre_clamp; + logic signed [CHANNEL_ACCUMULATOR_WIDTH-1:0] channel_r_acc_pre_clamp; + } self = 0, next_self; + + // verilator lint_off UNOPTFLAT + struct packed { + logic [$clog2(NUM_OPERATORS_PER_BANK)-1:0] op_mem_op_num; + logic op_mem_rd; + logic [$clog2(NUM_CHANNELS_PER_BANK)-1:0] ch_abcd_cnt_mem_channel_num; + logic signed [CHANNEL_OUT_WIDTH-1:0] channel_out; + logic latch_channels; + logic add_a; + logic add_b; + logic add_c; + logic add_d; + } signals; + // verilator lint_on UNOPTFLAT + always_ff @(posedge clk) begin if (opl3_reg_wr.valid) begin if (opl3_reg_wr.bank_num == 1 && opl3_reg_wr.address == 4) @@ -133,41 +170,6 @@ module channels .dob(operator_mem_out) ); - enum { - IDLE, - LOAD_2_OP_SECOND_0, - LOAD_2_OP_SECOND_1, - LOAD_2_OP_FIRST_AND_ACCUMULATE, - LOAD_4_OP_THIRD_0, - LOAD_4_OP_THIRD_1, - LOAD_4_OP_SECOND, - LOAD_4_OP_FIRST_AND_ACCUMULATE, - DONE - } state = IDLE, next_state; - - struct packed { - logic cnt_second; - logic signed [OP_OUT_WIDTH-1:0] operator_out_third; - logic signed [OP_OUT_WIDTH-1:0] operator_out_second; - logic bank_num; - logic [$clog2(NUM_OPERATORS_PER_BANK)-1:0] op_num; - logic [$clog2(NUM_CHANNELS_PER_BANK)-1:0] channel_num; - logic signed [CHANNEL_ACCUMULATOR_WIDTH-1:0] channel_l_acc_pre_clamp; - logic signed [CHANNEL_ACCUMULATOR_WIDTH-1:0] channel_r_acc_pre_clamp; - } self = 0, next_self; - - struct packed { - logic [$clog2(NUM_OPERATORS_PER_BANK)-1:0] op_mem_op_num; - logic op_mem_rd; - logic [$clog2(NUM_CHANNELS_PER_BANK)-1:0] ch_abcd_cnt_mem_channel_num; - logic signed [CHANNEL_OUT_WIDTH-1:0] channel_out; - logic latch_channels; - logic add_a; - logic add_b; - logic add_c; - logic add_d; - } signals; - always_ff @(posedge clk) if (sample_clk_en) begin state <= IDLE; @@ -389,4 +391,4 @@ module channels ); endmodule -`default_nettype wire \ No newline at end of file +`default_nettype wire diff --git a/fpga/modules/channels/src/control_operators.sv b/fpga/modules/channels/src/control_operators.sv index 90f2321..f2617dc 100755 --- a/fpga/modules/channels/src/control_operators.sv +++ b/fpga/modules/channels/src/control_operators.sv @@ -58,8 +58,9 @@ module control_operators ); localparam PIPELINE_DELAY = 6; localparam MODULATION_DELAY = 1; // output of operator 0 must be ready by cycle 2 of operator 3 so it can modulate it + localparam DELAY_COUNTER_WIDTH = MODULATION_DELAY > 1 ? $clog2(MODULATION_DELAY) : 1; localparam NUM_OPERATOR_UPDATE_STATES = NUM_BANKS*NUM_OPERATORS_PER_BANK + 1; // 36 operators + idle state - logic [$clog2(MODULATION_DELAY)-1:0] delay_counter = 0; + logic [DELAY_COUNTER_WIDTH-1:0] delay_counter = 0; logic [$clog2(NUM_OPERATOR_UPDATE_STATES)-1:0] state = 0; logic [$clog2(NUM_OPERATOR_UPDATE_STATES)-1:0] next_state; @@ -70,7 +71,7 @@ module control_operators logic [OP_NUM_WIDTH-1:0] op_num_p1 = 0; logic use_feedback_p1 = 0; - logic signed [OP_OUT_WIDTH-1:0] modulation_p1 = 0; + logic signed [OP_OUT_WIDTH-1:0] modulation_p1; logic signed [OP_OUT_WIDTH-1:0] out_p6; logic signed [OP_OUT_WIDTH-1:0] modulation_out_p1; @@ -532,4 +533,4 @@ module control_operators always_ff @(posedge clk) ops_done_pulse <= operator_out.valid && operator_out.bank_num == 1 && operator_out.op_num == 17; endmodule -`default_nettype wire \ No newline at end of file +`default_nettype wire diff --git a/fpga/modules/host_if/src/host_if.sv b/fpga/modules/host_if/src/host_if.sv index f2bc0a5..3f322ab 100755 --- a/fpga/modules/host_if/src/host_if.sv +++ b/fpga/modules/host_if/src/host_if.sv @@ -53,7 +53,7 @@ module host_if input wire wr_n, input wire [1:0] address, input wire [REG_FILE_DATA_WIDTH-1:0] din, - output logic [REG_FILE_DATA_WIDTH-1:0] dout = 0, + output logic [REG_FILE_DATA_WIDTH-1:0] dout, output opl3_reg_wr_t opl3_reg_wr = 0, input wire [REG_FILE_DATA_WIDTH-1:0] status, output logic force_timer_overflow @@ -141,4 +141,4 @@ module host_if endgenerate endmodule -`default_nettype wire \ No newline at end of file +`default_nettype wire diff --git a/fpga/modules/operator/src/calc_phase_inc.sv b/fpga/modules/operator/src/calc_phase_inc.sv index 2f83a4b..19d55e4 100755 --- a/fpga/modules/operator/src/calc_phase_inc.sv +++ b/fpga/modules/operator/src/calc_phase_inc.sv @@ -54,7 +54,7 @@ module calc_phase_inc input wire [REG_BLOCK_WIDTH-1:0] block, input wire vib, input wire dvb, - output logic [PHASE_ACC_WIDTH-1:0] phase_inc_p2 = 0 + output logic [PHASE_ACC_WIDTH-1:0] phase_inc_p2 ); localparam PIPELINE_DELAY = 2; diff --git a/fpga/modules/operator/src/envelope_generator.sv b/fpga/modules/operator/src/envelope_generator.sv index 1ef04a0..24f74a3 100755 --- a/fpga/modules/operator/src/envelope_generator.sv +++ b/fpga/modules/operator/src/envelope_generator.sv @@ -207,6 +207,7 @@ module envelope_generator always_comb begin eg_reset_p0 = 0; + requested_rate_p0 = 0; if (key_on_p0 && state_p0 == RELEASE) begin eg_reset_p0 = 1; diff --git a/fpga/modules/top_level/pkg/opl3_pkg.sv b/fpga/modules/top_level/pkg/opl3_pkg.sv index e0a6f93..5e6c1a6 100755 --- a/fpga/modules/top_level/pkg/opl3_pkg.sv +++ b/fpga/modules/top_level/pkg/opl3_pkg.sv @@ -40,6 +40,9 @@ # Copyright (C) 2010-2013 by carbon14 and opl3 # #******************************************************************************/ +`timescale 1ns / 1ps +`default_nettype none + package opl3_pkg; /* * Original OPL3 used a 14.31818MHz master clock, divided by 288 giving a @@ -56,7 +59,7 @@ package opl3_pkg; localparam INSTANTIATE_SAMPLE_SYNC_TO_DAC_CLK = 0; localparam DESIRED_SAMPLE_FREQ = 49.7159e3; - localparam int CLK_DIV_COUNT = $ceil(CLK_FREQ/DESIRED_SAMPLE_FREQ); // unsupported by Quartus 17, set manually + localparam CLK_DIV_COUNT = int'($ceil(CLK_FREQ/DESIRED_SAMPLE_FREQ)); // unsupported by Quartus 17, set manually localparam ACTUAL_SAMPLE_FREQ = CLK_FREQ/CLK_DIV_COUNT; localparam NUM_REG_PER_BANK = 'hF6; @@ -118,3 +121,4 @@ package opl3_pkg; } operator_out_t; endpackage +`default_nettype wire