-
Notifications
You must be signed in to change notification settings - Fork 240
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
demo_blink_led_asm bugfix #639
Conversation
Hey there! I never thought about this overflow issue (and it seems like I did not run the problem long enough...) - so thanks for discovering this! 👍 I like your simple fix, but this requires that the MTIME machine timer is present in the system. Furthermore, MTIME is expected to show "wall clock" time so altering the MTIME counter during operation might not be a good thing (but that is not relevant for this minimal demo anyway). However, what do you think about going back to the By the way, thumbs up for the illustrating video! 😉 |
I agree with you, MTIME should not be altered, so i went back to the |
Looks good to me! 👍 |
Yes i have tested it, and the issue is now resolved, i also finded a typo in the datasheet i can fix it here or i need to open another PR? |
Great, thank you!
If that is just a typo then just add it to this PR. 😉 |
you'r welcome, thank you so much for making this awesome opensource project! |
Thank you very much 😉 Ready to merge? |
Yes. |
This PR fix a bug that make demo_blink_led_asm not work as intended.
Example of the wrong behavior
This behavior is caused by the delay subroutine which sometimes doesn't work as it should.
The instruction
add t1, t1, t0
can cause an overflow when mcycle is sufficiently large.These overflows then leads to the delay_loop being skipped multiple times.
solution
I have solved this bug by using the MTIME module instead of mcycle, ensuring that every time the delay subroutine is called, the MTIME.TIME_LOW register is resetted back to zero, thereby avoiding overflows.