Skip to content
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

[rtl, aon_timer] Simpler condition to aid coverage closure #25811

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

antmarzam
Copy link
Contributor

The condition originally looked like:

 assign intr_test_qe           = reg2hw.intr_test.wkup_timer_expired.qe |
                                 reg2hw.intr_test.wdog_timer_bark

Which causes a conditional coverage hole due to both Qe signals being connected to the same signal in aon_timer_reg_top. In order to avoid waving the hole, the RTL has been ammended, and an assertion has been added to ensure things work as expected in case the RTL (register top) changes in the future

The condition originally looked like:
```
 assign intr_test_qe           = reg2hw.intr_test.wkup_timer_expired.qe |
                                 reg2hw.intr_test.wdog_timer_bark
```
Which causes a conditional coverage hole due to both Qe signals being
connected to the same signal in aon_timer_reg_top.
In order to avoid waving the hole, the RTL has been ammended, and an
assertion has been added to ensure things work as expected in case the
RTL (register top) changes in the future

Signed-off-by: Antonio Martinez Zambrana <[email protected]>
@antmarzam antmarzam force-pushed the aon_timer_ccov_closure_rtl_simplification branch from ce66518 to 2f489eb Compare January 8, 2025 15:25
Copy link
Contributor

@rswarbrick rswarbrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks sensible to me. I think I'd probably be inclined to shorten the comment a bit though. Maybe "All the fields of a register have the same value for their qe signals, so we just pick one of them. This assertion checks that we don't miss writes if the register top changes." ?

@vogelpi, would you mind taking a look at this?

@rswarbrick rswarbrick requested a review from vogelpi January 8, 2025 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants