Add --mark_range flag

[henkkuli]

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

[rust-highfive]

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.

[adamgreig]

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 update.sh you have each peripheral's address range spelled out, instead of just putting 0x40000000-0x40070000, 0x50200000-0x50204000, 0x50300000-0x50304000, 0x50400000-0x50404000? It seems like all the peripheral spaces are contiguous?

[henkkuli]

Is there any reason in your update.sh you have each peripheral's address range spelled out, instead of just putting 0x40000000-0x40070000, 0x50200000-0x50204000, 0x50300000-0x50304000, 0x50400000-0x50404000?

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.

[adamgreig]

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 --flag a b c: it looks like only a is part of --flag and b and c are extra positional arguments... maybe they should be comma-separated? Maybe it's not a big deal, I just don't feel like I've seen that sort of command line arguments much before.

[burrbull]

Even with new comments it is not obvious how it works. Please add more complete example to CI tests.

[henkkuli]

Could you add an entry to the changelog for this, please?

Sure, I'll add an entry with next batch of changes.

Also, I wonder if it's a bit unnatural to have command line arguments like --flag a b c

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: --mark_range MarkerName:0x1234-0x5678

Even with new comments it is not obvious how it works. Please add more complete example to CI tests.

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 ci/script.sh:13? And make it part of OPTIONS=all?

[burrbull]

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 ci/script.sh:13? And make it part of OPTIONS=all?

Not necessary. Any way you feel comfortable. You can just add separate job. We'll do refactoring some other day.

[adamgreig]

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: --mark_range MarkerName:0x1234-0x5678

That suggested format looks good to me, thanks!

[Dirbaio]

What does it do if there's several instances of the peripheral at different addrs, and only one is covered by the range in --mark_range?

[burrbull]

@henkkuli Could you rebase this on current master branch?

[burrbull]

I was thinking that maybe the following would be natural: --mark_range MarkerName:0x1234-0x5678

Maybe something like: --registers 0x1234-0x5678,0x11234-0x15678:Marker1,Marker2,Marker3 ?