Fix the gpio_stress_all with random reset #25657
Conversation
marcelocarvalhoLowRisc
requested review from
matutem
and removed request for
a team
marcelocarvalhoLowRisc
force-pushed
the
rnd_reset_debug
branch
14 times, most recently
from
ada03cb to
245aab3
Compare
|
Please could you squash all these commits together? |
marcelocarvalhoLowRisc
force-pushed
the
rnd_reset_debug
branch
5 times, most recently
from
a6b6d85 to
3e539ca
Compare
marcelocarvalhoLowRisc
force-pushed
the
rnd_reset_debug
branch
from
3e539ca to
eace0fe
Compare
marcelocarvalhoLowRisc
force-pushed
the
rnd_reset_debug
branch
from
eace0fe to
7d67ef6
Compare
hw/ip/gpio/dv/env/seq_lib/gpio_random_long_reg_writes_reg_reads_vseq.sv
Outdated
Show resolved
Hide resolved
|
Thanks! |
marcelocarvalhoLowRisc
force-pushed
the
rnd_reset_debug
branch
2 times, most recently
from
354ccf3 to
1d90818
Compare
marcelocarvalhoLowRisc
force-pushed
the
rnd_reset_debug
branch
from
1d90818 to
afc55dd
Compare
marcelocarvalhoLowRisc
force-pushed
the
rnd_reset_debug
branch
from
768bfda to
afc55dd
Compare
There was a problem hiding this comment.
Hi @marcelocarvalhoLowRisc! I've just gone through the changed files and I think my questions from December 17th still apply. Would you mind going through and updating things?
marcelocarvalhoLowRisc
force-pushed
the
rnd_reset_debug
branch
from
afc55dd to
069140d
Compare
I think now everything is fixed, sometimes I missed some comments that was not in the main page or in the commit section. Sorry about that |
| `uvm_info(msg_id, $sformatf("gpio_i = %0h", gpio_i), UVM_LOW) | ||
|
|
There was a problem hiding this comment.
I think we can drop this hunk (which changes a verbosity level and adds a newline).
There was a problem hiding this comment.
fixed.
| //Skip if a reset is ongoing... | ||
| if (!cfg.clk_rst_vif.rst_n) break; |
There was a problem hiding this comment.
I think this comment still applies (now on line 70)
| // Skip if a reset is ongoing... | ||
| if (!cfg.clk_rst_vif.rst_n) break; |
There was a problem hiding this comment.
I don't think this break statement will have any effect.
The next one won't either, but I guess at least it avoids us generating a pointless UVM transaction. This one should probably be dropped.
There was a problem hiding this comment.
I kept the second one, and removed the first one. =)
| cfg.clk_rst_vif.wait_clks_or_rst(delay); | ||
| `uvm_info(msg_id, $sformatf("waiting for %0d clock cycles", delay), UVM_HIGH) |
There was a problem hiding this comment.
I know that this isn't caused by your change, but this is rather silly! It prints the message after it has waited! I'd probably swap the two lines over.
There was a problem hiding this comment.
ahahah, fixed =)
Updates to the sequences to replace the wait_clk to wait_clks_or_rst. Signed-off-by: Marcelo Carvalho Faleiro de Almeida <[email protected]>
marcelocarvalhoLowRisc
force-pushed
the
rnd_reset_debug
branch
from
069140d to
3f2c959
Compare
New fix for the stress_all_gpio testcase with random reset. Basically included new if(reset) checks into the sequences to skip the flow when a reset comes, and replaced the wait_clks task to wait_clks_or_rst. This makes the gpio test pass with a reasonable number of seeds. It's a temporally fix until we have the reset agent on place.