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] Remove ECC related data_rdata_i -> instr_X_o feedthroughs #2206

Merged
merged 1 commit into from
Aug 23, 2024

Conversation

GregAC
Copy link
Collaborator

@GregAC GregAC commented Aug 23, 2024

Prior to this commit an ECC failure on the incoming data memory response factored directly into the outputs for the instruction memory interfaces. This existed due to a desire to take an NMI on an ECC failure as soon as possible but causes timing issues so it has been altered.

Now rather than directly raise the NMI the same cycle the assertion of 'irq_nm_int' is delayed by a cycle which breaks the feedthrough path.

Prior to this commit an ECC failure on the incoming data memory response
factored directly into the outputs for the instruction memory
interfaces. This existed due to a desire to take an NMI on an ECC
failure as soon as possible but causes timing issues so it has been
altered.

Now rather than directly raise the NMI the same cycle the assertion of
'irq_nm_int' is delayed by a cycle which breaks the feedthrough path.
@GregAC GregAC requested a review from vogelpi August 23, 2024 20:00
Copy link
Contributor

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

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

Thanks @GregAC , this looks good to me!

@GregAC GregAC added this pull request to the merge queue Aug 23, 2024
Merged via the queue into lowRISC:master with commit 38c0709 Aug 23, 2024
11 checks passed
@GregAC GregAC deleted the ecc_nmi_timing_fix branch August 27, 2024 08:37
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