Gpio straps func cov #26009
Gpio straps func cov #26009
Conversation
marcelocarvalhoLowRisc
requested review from
hcallahan-lowrisc
and removed request for
a team
marcelocarvalhoLowRisc
force-pushed
the
gpio_straps_func_cov
branch
from
c2c7ae2 to
c5c99e7
Compare
hw/dv/tools/vcs/cover.cfg
Outdated
| +tree tb | ||
| +module gpio_straps_if |
There was a problem hiding this comment.
We talked about this in person, but the right way to do this is to add a block-specific configuration file. See rom_ctrl and rv_dm for two examples that do this for VCS.
marcelocarvalhoLowRisc
force-pushed
the
gpio_straps_func_cov
branch
11 times, most recently
from
9d95e6a to
87b277d
Compare
marcelocarvalhoLowRisc
force-pushed
the
gpio_straps_func_cov
branch
2 times, most recently
from
ad99314 to
85ab23a
Compare
Yes, it's fixed. I droped most of the items, I only kept the strap interface, where it is located the coverage properties. |
- Included new file gpio_cover.cfg to add the gpio_straps_if cover
assertions in the database
- Added the configuration file above into the gpio_sim_cfg.hjson
- Included the commit from PR: 25868 about straps verification,
from this point, only needs to review these files:
- new file: hw/ip/gpio/dv/cov/gpio_cover.cfg
- modified: hw/ip/gpio/dv/gpio_sim_cfg.hjson (Inclusion about
the gpio_cover.cfg)
- modified: hw/ip/gpio/dv/env/gpio_env_cov.sv
- new file: hw/ip/gpio/dv/interfaces/gpio_straps_if.sv (New
cover properties included)
Signed-off-by: Marcelo Carvalho Faleiro de Almeida <[email protected]>
marcelocarvalhoLowRisc
force-pushed
the
gpio_straps_func_cov
branch
from
85ab23a to
c7d0124
Compare
| { | ||
| name: gpio_rand_straps | ||
| uvm_test_seq: gpio_rand_straps_vseq | ||
| } |
There was a problem hiding this comment.
I don't think this should be here?
| logic strap_en; // Signal to enable straps | ||
| gpio_straps_t sampled_straps; // Sampled gpio_i snapshot data from GPIO (DUT) | ||
|
|
||
| modport straps_port_in(input clk, |
There was a problem hiding this comment.
I find this sort of thing really hard to understand (not just from you!). Could you add a comment above it that explains who we're expecting to interact through this modport? ("This is the dut's view of the sampled straps" or "This is the way the outside world passes in straps" or ...)
| output strap_en, | ||
| input sampled_straps); | ||
|
|
||
| // Sequence to capture the timing relationship between strap_en and strap_data |
There was a problem hiding this comment.
I'm not sure I understand this. Normally a sequence models a 1-bit value changing over time (as used by a property or assertion). I don't think that sampled_straps.data is this.
Functional coverage properties included in the gpio_straps_if.sv and some other minor fixes.