-
Notifications
You must be signed in to change notification settings - Fork 151
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
Add support for 64-bit registers #295
Conversation
r? @ryankurte (rust_highfive has picked a reviewer for you, use r? to override) |
I tried this on riscv-rust/k210-pac#14 and there was no change at all :/ How to make it pick up that registers for a peripheral are 64 bit? |
Seems like |
The difference is that CLINT has both 64-bit and 32-bit registers, DMAC and KPU have only 64-bit registers, so it'd make sense to specify it at the peripheral level. But yes I guess specifying it for every register individually would work. |
This |
@Disasm Is there anything we should/can test here wrt to 64bit support? |
bors try |
@therealprof I hope that CI is enough. Before this PR 64-bit register declarations led to |
Shouldn't we at least have some files enabled with this PR which exercise this new part of the code? |
Ah, OK, I'll add them. |
Done! |
tryBuild succeeded |
Co-Authored-By: Daniel Egger <daniel@eggers-club.de>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
bors r+ |
295: Add support for 64-bit registers r=therealprof a=Disasm Some registers on K210 chip are 64-bit, so it's better to declare them as u64 in SVD for the reasons mentioned here: riscv-rust/k210-pac#1 (comment) At the moment, svd2rust forbids 64-bit register declarations. This PR fixes this. This change can cause silent bugs on platforms without 64-bit memory access operations due to the need for proper access sequence to 64-bit registers with two 32-bit accesses. Closes #289 Co-authored-by: Vadim Kaushan <admin@disasm.info>
Build succeeded |
Some registers on K210 chip are 64-bit, so it's better to declare them as u64 in SVD for the reasons mentioned here: riscv-rust/k210-pac#1 (comment)
At the moment, svd2rust forbids 64-bit register declarations. This PR fixes this.
This change can cause silent bugs on platforms without 64-bit memory access operations due to the need for proper access sequence to 64-bit registers with two 32-bit accesses.
Closes #289