-
Notifications
You must be signed in to change notification settings - Fork 263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix broken msg passing in wishbone master for (fix #692) #693
Conversation
Fails on modelsim, but working properly on ghdl.
@@ -22,7 +22,7 @@ def encode(tb_cfg): | |||
|
|||
|
|||
def gen_wb_tests(obj, *args): | |||
for dat_width, num_cycles, strobe_prob, ack_prob, stall_prob in product(*args): | |||
for dat_width, num_cycles, strobe_prob, ack_prob, stall_prob, slave_inst in product(*args): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use slave_comb
instead of slave_inst
, since my understanding is that the difference between the two cases is whether the slave is combinatorial or registered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to have possibility to skip wb slave VC instantiation and drive wishbone slave signals directly. It makes possible to test various corner cases - like here when specific set of acknowledge logic lead to crash on modelsim, but not ghdl. I thinks it's not feasible to add this corner cases reproducers to wishbone slave as it will obfuscate code. This is all about wb master VC bug.
|
||
|
||
TB_WISHBONE_MASTER = LIB.test_bench("tb_wishbone_master") | ||
|
||
for test in TB_WISHBONE_MASTER.get_tests(): | ||
gen_wb_tests(test, [8, 32], [1, 64], [0.3, 1.0], [0.3, 1.0], [0.4, 0.0]) | ||
if test.name == "slave comb ack": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure to understand this. Why is the combinatorial version tested with one set of params but the other one is tested with two?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
slave comb ack test case doesn't need/cannot have random stimulus
stall => stall, | ||
ack => ack | ||
); | ||
slave_gen : if tb_cfg.slave_inst generate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense for comb
to be an option (generic) of the wishbone slave, instead of using a generate in the testbench?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as described in first comment IMHO: no, it would not
No description provided.