Skip to content

[ot] hw/opentitan: ot_aon_timer: Make wakeup timer 64 bits #109

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

Merged

Conversation

AlexJones0
Copy link

For earlgrey-1.0.0, OpenTitan's aon_timer has been updated so that it's wakeup timer is now a 64-bit counter rather than a 32-bit counter (whereas the watchdog timer remains 32-bit). This PR makes the necessary implementation changes to add this functionality, including fixes to the values of shifted registers.

This changes has been tested to make the following tests (which were previously failing) pass: aon_timer_irq_test, and pwrmgr_wdog_test (running with an icount shift of icount=7). All tests were built from master on OpenTitan.

Copy link

@rivos-eblot rivos-eblot left a comment

Choose a reason for hiding this comment

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

LGTM, except the file header (lowRISC, 2025 ... 😄)

Adding @loiclefort who better knowns the timer implementation than I do.

Copy link

@rivos-eblot rivos-eblot left a comment

Choose a reason for hiding this comment

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

I would add some explicit casts as there are several signed / unsigned implicit conversions, some of which already existed in the original implementation, sorry about that.

For earlgrey-1.0.0, OpenTitan's aon_timer has been updated so that it's
wakeup timer is now a 64-bit counter rather than a 32-bit counter
(whereas the watchdog timer remains 32-bit). This requires
the implementation of register mapping changes with each wakeup count /
threshold register now having a HI/LO counterpart.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
@AlexJones0
Copy link
Author

I would add some explicit casts as there are several signed / unsigned implicit conversions, some of which already existed in the original implementation, sorry about that.

I believe I've addressed the implicit signed/unsigned casts now, but please do double check

Copy link

@rivos-eblot rivos-eblot left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks

@jwnrt jwnrt merged commit 2ce9880 into lowRISC:dev/ot-earlgrey-1.0.0-updates Jan 22, 2025
6 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.

4 participants