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

mmreg fix pollable interrupt status #17

Merged
merged 3 commits into from
Jul 24, 2024

Conversation

marnovandermaas
Copy link
Contributor

This essentially removes the debug signals and wires from the mmreg module. It doesn't make much sense to expose these to software and with hardware simulation you can derive these signals from existing ones.

It also removes tbre_go and tbre_intr_stat from the read register interface. These signals are only set for one cycle and it doesn't make sense to expose that over a bus interface.

@kliuMsft
Copy link
Contributor

I don't see intr_stat being removed in the code change. We probably should leave intr_stat in the readable space since it provides a way for software to poll the completion status without enabling interrupt.

Also, please leave the dbg fifo in for now. The feature was required for FPGA debugging (by Wes I think). Basically we need a fast printf (compared to UART) to dump information when debugging firmware. Inserting UART printfs might change the software timing too much and make it harder to reproduce issues.

Both tbre_go and tbre_intr_stat are only asserted for one cycle so it
does not make sense to have these be exposed as a register that can be
read from the bus.
Previously the interrupt status could be polled when interrupts were
disabled but there was no way to clear the interrupt. This commits
allows reading out the interrupt status again and added clearing logic.
This makes it more clear from code inspection that register address 2
exists because it is in the case statement.
@marnovandermaas marnovandermaas changed the title mmreg remove debug mmreg fix pollable interrupt status Jul 24, 2024
@marnovandermaas
Copy link
Contributor Author

Thanks @kliuMsft ! I've put the debug logic back in. I've put the interrupt status back into the readable space and also added it to the writable space so that it can be cleared in case interrupts are not enabled. Let me know what you think.

@kliuMsft kliuMsft merged commit dc86eeb into microsoft:main Jul 24, 2024
1 of 2 checks passed
@kliuMsft
Copy link
Contributor

Looks good, thanks Marno. Merging.

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