Closed reitermarkus closed 2 years ago
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @therealprof (or someone else) soon.
Please see the contribution instructions for more information.
I'm a bit concerned about what this means for platforms that don't have critical-section
support yet. I know msp430 is working on it but having some codegen size issues, and I don't know anything about mips_mcu
. It might be worth waiting a little longer for those architectures to get support before releasing svd2rust that uses it?
Having it as an optional dependency/feature seems useful though and does mean those platforms don't need immediate critical-section support, even end-users could add simple interrupt-based implementations to use take()
if needed.
It might be worth waiting a little longer
I don't think it's worth waiting. Other architectures will have to implement these only once they upgrade svd2rust
anyways. I have opened issues for the affected architectures to give them a heads up though.
Oh, thanks, I didn't see the edit to the first comment. SGTM to me then.
cortex_m
https://github.com/rust-embedded/cortex-m/pull/447msp430
https://github.com/rust-embedded/msp430/issues/13riscv
https://github.com/rust-embedded/riscv/pull/110xtensa_lx
https://github.com/esp-rs/xtensa-lx/issues/20, https://github.com/esp-rs/esp-hal/pull/151mips_mcu
https://github.com/kiffie/pic32-rs/issues/5