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

[aon_timer,dv] Fix how we wait for intr_state to stop being busy #26123

Closed

Conversation

rswarbrick
Copy link
Contributor

The existing code doesn't actually work because there is nothing that consumes time and the EDA tool doesn't know when to re-evaluate the expression.

That causes warnings from VCS (at least). Avoid them by waiting more explicitly.

The existing code doesn't actually work because there is nothing that
consumes time and the EDA tool doesn't know when to re-evaluate the
expression.

That causes warnings from VCS (at least). Avoid them by waiting more
explicitly.

Signed-off-by: Rupert Swarbrick <[email protected]>
@rswarbrick rswarbrick added Component:DV DV issue: testbench, test case, etc. IP:aon_timer labels Feb 4, 2025
@rswarbrick rswarbrick requested a review from antmarzam February 4, 2025 16:14
@rswarbrick rswarbrick requested a review from a team as a code owner February 4, 2025 16:14
@rswarbrick rswarbrick requested review from eshapira and removed request for a team February 4, 2025 16:14
@antmarzam
Copy link
Contributor

I presume you refer to the following warning?

  Function calls inside wait/event expression are sensitive only to arguments.
  this.ral.intr_state.is_busy() is called without any arguments which may not 
  unblock wait/event expression

Not sure why VCS complains about this, I've seen it supported in multiple simulators and for the UVM RAL it seems the recommendation is to wait until is_busy=0 . is_busy only wraps around m_is_busy from uvm_reg.

I think your suggestion would work, but would also force to wait until the next clock tick, when m_is_busy can be released before the next tick.

How about we replace: wait (ral.intr_state.is_busy() == 0);
With: wait (ral.intr_state.m_is_busy == 0) ? It seems simpler and doesn't have to wait until the next tick?

In uvm_reg, you'll see m_is_busy was intended to be a local variable but the code has been commented out:
/*local*/ bit m_is_busy; I wonder if the warning is related to why they haven't yet uncommented the local keyword

@rswarbrick
Copy link
Contributor Author

I think this is handled more cleanly in @antmarzam's #26167. Let's do it that way :-)

@rswarbrick rswarbrick closed this Feb 12, 2025
@rswarbrick rswarbrick deleted the aon-timer-intr-state-busy-wait branch February 12, 2025 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component:DV DV issue: testbench, test case, etc. IP:aon_timer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants