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

Change timer wraparound behavior to be more useful #109

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

alinja
Copy link
Contributor

@alinja alinja commented Nov 12, 2023

Risc-v timer is specified to be 64 bits long, meaning wraparound times of hundreds or thousands of years, making wraparound practically a non-issue. Servant has 32-bit timer, and it is not useful then timer wraps around. To avoid actually using 64 bits, this makes the timer useful even when the timer wraps around.

@olofk
Copy link
Owner

olofk commented Nov 16, 2023

Thank you for your contribution. I trust that this is an improvement, but I'm terrible at Verilog math, so would you mind quickly explaining what the actual difference is? Been staring at the code and can't for my life understand why this is better :)

@alinja
Copy link
Contributor Author

alinja commented Nov 16, 2023

Current unsigned comparison has a wraparound period of about 30s with 125MHz clock. When the mtime is near its maximum and a timer irq is requested after even a small time in future, mtimecmp will wrap around and an interrupt is generated immediately.

With signed comparison like this, the interrupt is generated at the right time, because the difference is positive even when crossing the wraparound boundary. Only downside is that interrupts can only be requested 15s in advance.

@olofk
Copy link
Owner

olofk commented Nov 17, 2023

Aha! I think I understand. All good. Thanks for the explanation. Picked and pushed!

@olofk olofk merged commit bc984f6 into olofk:main Nov 17, 2023
2 of 3 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