Skip to content
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.

Sign up for GitHub

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

Jump to bottom

updates for working with next svd2rust release #540

Merged
merged 3 commits into from
Jul 22, 2021
Merged

updates for working with next svd2rust release #540

merged 3 commits into from
Jul 22, 2021

Conversation

burrbull
Copy link
Member

@burrbull burrbull commented Apr 17, 2021 •
edited
Loading

@github-actions
Copy link

Memory map comparison

@github-actions
Copy link

Memory map comparison

@burrbull

This comment has been minimized.

Sign in to view
@burrbull burrbull mentioned this pull request Apr 18, 2021
Open
@github-actions
Copy link

Memory map comparison

@adamgreig adamgreig mentioned this pull request Apr 24, 2021
Closed
@github-actions
Copy link

github-actions bot commented May 8, 2021

Memory map comparison

@burrbull
Copy link
Member Author

burrbull commented May 9, 2021 •
edited
Loading

Rebased on #546

Require rust-embedded/svdtools#57 !!!

@github-actions
Copy link

Memory map comparison

@burrbull
Copy link
Member Author

@adamgreig rebased

@github-actions
Copy link

Memory map comparison

@burrbull burrbull marked this pull request as ready for review July 21, 2021 05:11
@github-actions
Copy link

Memory map comparison

@github-actions
Copy link

Memory map comparison

@adamgreig
Copy link
Member

Thanks for working through this! I need to check some of the G0 ones a bit more carefully, but otherwise the main question is about all the F7 and L4 tim_ccm_v2.yaml files that are commented out: what needs to be done with them?

adamgreig
adamgreig reviewed Jul 21, 2021
View reviewed changes
peripherals/flash/flash_latency16.yaml
WS12: [12, "12 wait states"]
WS13: [13, "13 wait states"]
WS14: [14, "14 wait states"]
WS15: [15, "15 wait states"]
Copy link
Member

Choose a reason for hiding this comment

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

Do we even need this latency16 file? Looks like all users of flash_v1.yaml use the flash_latency8.yaml version.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like I planned to do something, but forgot.

Copy link
Member Author

Choose a reason for hiding this comment

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

According to RM only 405/415 407/417 series in STM32F4 don't support higher states.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@adamgreig
Copy link
Member

I've finished reviewing and I guess it makes sense to merge this before tackling #546 since it's based off it. My only outstanding question really is about the commented-out CCMR files, what do you think?

  • Make new CCMR YAMLs for the different field widths and apply them as required to each device?
  • Merge this now and do that in another PR?
  • Something else?

@burrbull
Copy link
Member Author

I've finished reviewing and I guess it makes sense to merge this before tackling #546 since it's based off it. My only outstanding question really is about the commented-out CCMR files, what do you think?

* Make new CCMR YAMLs for the different field widths and apply them as required to each device?

* Merge this now and do that in another PR?

* Something else?

The comments of CCMR files is fast fix to make compile, as currently those files are broken.
A real fix of this you can see in #546. So I propose merge this. And then review #546.

@github-actions
Copy link

Memory map comparison

adamgreig
adamgreig approved these changes Jul 22, 2021
View reviewed changes
Copy link
Member

@adamgreig adamgreig left a comment

Choose a reason for hiding this comment

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

Thank you very much for working this out!

Understood about CCR being fixed in #546.

I've just noticed on a second review that the F7x2's rebase of TIM5 to use TIM2 means its option register is no longer correct, but let's fix that in another PR. Everything else LGTM, thanks for fixing the flash latency bits.

bors merge

@bors
Copy link
Contributor

bors bot commented Jul 22, 2021

Build succeeded:

@bors bors bot merged commit bf70c7f into stm32-rs:master Jul 22, 2021
This was referenced Jul 22, 2021
Merged
Closed
@burrbull burrbull deleted the update branch July 22, 2021 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamgreig adamgreig adamgreig approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@burrbull @adamgreig