-
Notifications
You must be signed in to change notification settings - Fork 16
Description
Hello,
I tried to run tb_apb_regs.sv in QuestaSim andI encountered simulation errors.
1) The first error is parameter definition of clk_rst_gen in the common_verification does not match with the instantiation in tb_apb_regs.sv file.
To run without error, I need to change clk_rst_gen instantiation like below:
clk_rst_gen #(
.ClkPeriod ( CyclTime ),
.RstClkCycles( 5 )
) i_clk_gen (
.clk_o (clk),
.rst_no(rst_n)
);
2) Second error is about register initialization values.
As I understand, the intention of the apb_regs is to set every read only registers to reg_init values (by connecting output of registers to reg_init directly) and non read only registers to zero.
Here are lines for that inference:
assign reg_q_o[i] = ReadOnly[i] ? reg_init_i[i] : reg_q[i];
`FFLARN(reg_q[i], reg_d[i], reg_update[i], '0, pclk_i, preset_ni) // Initialization of non read only registers to 0
However, this behavior does not comply with the expected values in tb_apb_regs because golden model (reg_compare) is filled with random values for both read only and non-read only registers, which causes simulation errors when reading non read only registers.
Below you can find the problematic lines:
// initialize reset values and golden model
for (int unsigned i = 0; i < NoApbRegs; i++) begin
init_val = reg_data_t'($urandom()); // Initial value is 0 for non read only registers!
reg_init[i] = init_val;
reg_compare[i] = init_val;
end
done <= 1'b0;
3) Write problem with strobe
When the strobe is not full set (e.g., 1100), corresponding bytes of zero indexes are set to corresponding value in reg_init_i in the line below. This is because has_reset_q is set to 0.
// default assignments
reg_d = has_reset_q ? reg_q : reg_init_i;
I believe since reg_d is only meaningful for non read-only registers and it is only used to set value for them. It needs to be set to output of the register as default. Then, if there is a write request, it will overwrite the values with respect to strobe value and corresponding bytes would be stay unchanged when their strobe bit is set to 0.
I suggest couple of changes for fixing the issues that I mentioned in my pull request.