Skip to content

Implement InterruptNumber for bare_metal::Nr #266

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
merged 2 commits into from
Sep 21, 2020
Merged

Implement InterruptNumber for bare_metal::Nr #266

merged 2 commits into from
Sep 21, 2020

Conversation

adamgreig
Copy link
Member

@adamgreig adamgreig commented Sep 20, 2020

This PR aims to help backwards compatibility by implementing the new InterruptNumber trait (coming in cortex-m 0.7) for the old bare_metal::Nr trait. With this included in cortex-m 0.7, existing PACs generated from the current svd2rust (0.17) will work with cortex-m 0.7, and new PACs generated from a to-be-released svd2rust which uses `InterruptNumber directly will also work.

We can then remove this implementation in cortex-m 0.8 and upgrade cortex-m to depend on bare-metal 1.0 (or not depend on it at all) at that time.

With this PR in place, the upgrade path looks like:

  • We release cortex-m 0.7, which users can upgrade to without needing a new PAC
  • We release svd2rust 0.18, which will generate new PACs
  • PACs update, now requiring cortex-m 0.7
  • Users can update their PAC so long as they've already upgraded to cortex-m 0.7
  • For cortex-m 0.8, we drop this impl and move off bare-metal 0.2, and a new PAC is required to use 0.8 onwards

@rust-highfive
Copy link

r? @therealprof

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cortex-m labels Sep 20, 2020
@therealprof
Copy link
Contributor

I was kinda hoping we would march towards 1.0 rather sooner than later. 😅

Other than that, looks fine to me. Other opions? @jonas-schievink @thalesfragoso

thalesfragoso
thalesfragoso previously approved these changes Sep 21, 2020
Copy link
Member

@thalesfragoso thalesfragoso left a comment

Choose a reason for hiding this comment

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

I agree with this change, thanks Adam.

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

I think this is worth mentioning in the CHANGELOG.

CHANGELOG.md Outdated
@@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- New `InterruptNumber` trait is now required on interrupt arguments to the
various NVIC functions, replacing the previous use of `Nr` from bare-metal.
For backwards compatibility, `InterruptNumber` is implemented for types
which are `Nr + Copy`, but this will be removed in cortex-m v0.8.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really want to tie this to a specific version already?

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

LGTM, too. Thanks!

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 21, 2020

Build succeeded:

@bors bors bot merged commit 43b9383 into master Sep 21, 2020
@bors bors bot deleted the impl-nr-for-nr branch September 21, 2020 23:58
adamgreig pushed a commit that referenced this pull request Jan 12, 2022
287: Fix common uses of INSERT AFTER with .bss and .text r=adamgreig a=mattico

Fixes #267 
Fixes #266

This fixes two related issues.

1. Named sections are often inserted after `.bss` or `.text` in order to have them handled as if they were part of that section. Defining the start/end symbols outside of the section allows this to work.
2. Uninitialized C statics will end up as common symbols which end up in the COMMON input section. If this section is orphaned, it will likely end up placed after `.bss`. C code often expects these statics to be zero initialized. The first change would cause these symbols to be placed before `__ebss` so they will get zeroed by the reset handler. Explicitly placing the common symbols into `.bss` ensures this happens. Users who want uninitialized symbols should use the `.uninit` section.

See rust-embedded/cortex-m-rt#287 (comment)

Co-authored-by: Matt Ickstadt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cortex-m
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants