-
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 --mark_range flag #576
base: master
Are you sure you want to change the base?
Conversation
This flag allows implementing a marker trait for registers in specific memory addresses. These can then be used to implement methods on registers. One such example is providing atomic access to only some of the registers directly in the PAC. Currently this is done in HAL, but this loses the ability to use writer proxies. Moreover this allows keeping atomic and non-atomic usage in HAL more similar.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @adamgreig (or someone else) soon. Please see the contribution instructions for more information. |
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.
really neat!
src/main.rs
Outdated
"Mark registers in given range with a marker trait. | ||
The first argument after the flag sets the name for the marker trait. | ||
The second and third argument are the start and end addresses of registers | ||
which this trait is applied for.", |
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.
I think this deserves an example of how it could look.
I think this is a really nice way for specific PACs to implement chip-specific functionality like you've done, nice work! Is there any reason in your |
Not really, other than to show that you can specify the same marker multiple times, and to list all peripherals that allow atomic access. I still need to go through that list once more; it seems that some USB peripheral registers allow atomic access, while now none implement it. It could still be simplified. |
Could you add an entry to the changelog for this, please? Also, I wonder if it's a bit unnatural to have command line arguments like |
Even with new comments it is not obvious how it works. Please add more complete example to CI tests. |
Sure, I'll add an entry with next batch of changes.
I originally did that because I didn't want to write a parser for separating the marker name from the address, especially because I didn't know would this even be something that is wanted for svd2rust. Now that it seems that there is interest, I will write a proper parser. And now that I think of it, it shouldn't be anything more complicated than just a couple of splits. I was thinking that maybe the following would be natural:
I can add a CI test, but I'm not exactly sure how. Should I make it a new option that is passed to rust2svd in |
Not necessary. Any way you feel comfortable. You can just add separate job. We'll do refactoring some other day. |
That suggested format looks good to me, thanks! |
What does it do if there's several instances of the peripheral at different addrs, and only one is covered by the range in |
@henkkuli Could you rebase this on current master branch? |
Maybe something like: |
This flag allows implementing a marker trait for registers in specific memory addresses. These can then be used to implement methods on registers. One such example is providing atomic access to only some of the registers directly in the PAC. Currently this is done in HAL, but this loses the ability to use writer proxies. It is also finicky to use raw register addresses. Moreover this allows keeping atomic and non-atomic usage in HAL more similar.
For an example on how to use this, see my fork of rp2040-pac. The most interesting changes regarding the use reside in
generic_extension.rs
update.sh
Even though I have an example only for the RP2040, I believe that other architectures and devices could also benefit from this. For example on SMT32 MCUs this could be used to implement a nicer API for bit-banding.
Addresses #535