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

Mem response agent fix #2177

Merged
merged 2 commits into from
Jun 21, 2024
Merged

Mem response agent fix #2177

merged 2 commits into from
Jun 21, 2024

Conversation

GregAC
Copy link
Collaborator

@GregAC GregAC commented Jun 20, 2024

No description provided.

@GregAC GregAC force-pushed the mem_response_agent_fix branch from 92f908e to e231721 Compare June 20, 2024 17:10
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.

Thanks for the tidy-up.

cfg.vif.response_driver_cb.rvalid <= 1'b0;
cfg.vif.response_driver_cb.rdata <= 'x;
cfg.vif.response_driver_cb.rintg <= 'x;
cfg.vif.response_driver_cb.error <= 'x;
rdata_queue.get(tr);
if(cfg.vif.response_driver_cb.reset) continue;
cfg.vif.wait_clks(tr.rvalid_delay);

for (int i = 0;i < tr.rvalid_delay; ++i) begin
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Missing space after ';'. But I'd suggest repeat (tr.rvalid_delay) anyway.

GregAC added 2 commits June 21, 2024 12:03
Previous code working with clocking blocks synced to the raw clock
event. Instead they should sync to the clocking block event. This
ensures the values being read are the latest values rather than a cycle
old.

In particular for ibex_mem_intf_agent this meant it was unable to
produce a single cycle response to any memory transaction. With this fix
these are now observed.
@GregAC GregAC force-pushed the mem_response_agent_fix branch from e231721 to 8a0c3f0 Compare June 21, 2024 11:03
@GregAC GregAC added this pull request to the merge queue Jun 21, 2024
Merged via the queue into lowRISC:master with commit 1449ed5 Jun 21, 2024
11 checks passed
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