Skip to content

cpu/stm32f[0|1|3] unify stmclk with stm32_common#7500

Merged
aabadie merged 8 commits intoRIOT-OS:masterfrom
OTAkeys:pr/stmclk_f0_f1
Sep 1, 2017
Merged

cpu/stm32f[0|1|3] unify stmclk with stm32_common#7500
aabadie merged 8 commits intoRIOT-OS:masterfrom
OTAkeys:pr/stmclk_f0_f1

Conversation

@vincent-d
Copy link
Copy Markdown
Member

@vincent-d vincent-d commented Aug 23, 2017

Based on #7477

Another step in unifying clock config for stm32.
F0 and F1 are quite similar for clock config.

I can test only on nucleo-f091 and nucleo32-f042

I don't plan to work on stm32lx cpu

@vincent-d vincent-d added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable labels Aug 23, 2017
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Aug 23, 2017

Ran a few tests. examples/default, tests/xtimer_msg, tests/driver_sx127x:

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Aug 23, 2017

doesn't build with nucleo32-f031

@vincent-d
Copy link
Copy Markdown
Member Author

vincent-d commented Aug 23, 2017

@aabadie I updated only nucleo-f091 and nucleo-f103 so far, others need to be ported too

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Aug 23, 2017

I updated only nucleo-f091 and nucleo-f103 so far, others need to be ported too

I saw that. Just nucleo32-f042, nucleo32-f031 and stm32f0disco remain. I think the first 2 don't have HSE (they are configured on HSI)

@vincent-d
Copy link
Copy Markdown
Member Author

vincent-d commented Aug 23, 2017

Indeed, nucleo32 do not have HSE IIRC

@vincent-d
Copy link
Copy Markdown
Member Author

vincent-d commented Aug 23, 2017

updated nucleo32-f031 / f042

@vincent-d vincent-d added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 23, 2017
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Aug 23, 2017

great! unfortunately I won't be able to test before next week...

@vincent-d vincent-d mentioned this pull request Aug 25, 2017
Merged
@vincent-d vincent-d force-pushed the pr/stmclk_f0_f1 branch 2 times, most recently from 7360b76 to a122d61 Compare August 25, 2017 16:47
@vincent-d
Copy link
Copy Markdown
Member Author

vincent-d commented Aug 27, 2017

It turned out f3 is very similar to f0 and f1, so merging it as well.

@vincent-d vincent-d changed the title cpu/stm32f[0|1] unify stmclk with stm32_common cpu/stm32f[0|1|3] unify stmclk with stm32_common Aug 27, 2017
@vincent-d vincent-d force-pushed the pr/stmclk_f0_f1 branch 3 times, most recently from 7f12df3 to 21ca774 Compare August 29, 2017 21:27
@vincent-d
Copy link
Copy Markdown
Member Author

vincent-d commented Aug 29, 2017

Rebased on master

haukepetersen
haukepetersen reviewed Aug 30, 2017
View reviewed changes
cpu/stm32f1/stmclk.c Outdated
@@ -27,7 +27,7 @@
#include "cpu.h"
Copy link
Copy Markdown
Contributor

@haukepetersen haukepetersen Aug 30, 2017

Choose a reason for hiding this comment

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

can't we remove this file completely now?

Copy link
Copy Markdown
Member Author

@vincent-d vincent-d Aug 30, 2017

Choose a reason for hiding this comment

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

I forgot it, but we must remove it, for sure!

@haukepetersen haukepetersen mentioned this pull request Aug 31, 2017
Closed
9 tasks
aabadie
aabadie requested changes Sep 1, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Notes for stm32f3 are missing in the documentation of the clock configuration generator.

See the comments below.

I'll test on my f3 boards asap. Hope it can be merged today.

cpu/stm32_common/doc.txt Outdated
* @ref cpu_stm32l4, @ref cpu_stm32f7.
*
* STM32F[2|4|7] Clock configuration
* STM32F[0|1|2|4|7] Clock configuration
Copy link
Copy Markdown
Contributor

@aabadie aabadie Sep 1, 2017

Choose a reason for hiding this comment

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

f3 update missing here

cpu/stm32_common/doc.txt Outdated
* =================================
*
* stm32f2, stm32f4 and stm32f7 cpus share clock configuration code and macro.
* stm32f0, stm32f1, stm32f2, stm32f4 and stm32f7 cpus share clock configuration
Copy link
Copy Markdown
Contributor

@aabadie aabadie Sep 1, 2017

Choose a reason for hiding this comment

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

stm32f3 missing

cpu/stm32_common/doc.txt Outdated
* (CLOCK_APB2_DIV is not needed for stm32f0)
* The following macro must be defined for stm32f[2|4|7]:
* - CLOCK_PLL_M, CLOCK_PLL_N, CLOCK_PLL_P, CLOCK_PLL_Q: Main PLL factors
* The following macro must be defined for stm32f[0|1]:
Copy link
Copy Markdown
Contributor

@aabadie aabadie Sep 1, 2017

Choose a reason for hiding this comment

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

f3 missing

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Sep 1, 2017

Test results:

  • nucleo32-f042: works
  • nucleo-f072: works
  • nucleo-f103: works
  • nucleo32-f303: works
  • nucleo-f302: works
  • nucleo-f303: works
  • nucleo-f334: works
  • nucleo144-f303: works

@vincent-d
Copy link
Copy Markdown
Member Author

vincent-d commented Sep 1, 2017

Fixed doc, I also anticipated a bit #7537 in the doc

@vincent-d
Copy link
Copy Markdown
Member Author

vincent-d commented Sep 1, 2017

Thanks for the test effort @aabadie !

aabadie
aabadie approved these changes Sep 1, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

ACK (I'm fine with the doc anticipation)

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Sep 1, 2017

Please squash

@vincent-d
Copy link
Copy Markdown
Member Author

vincent-d commented Sep 1, 2017

Squashed

@vincent-d vincent-d added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 1, 2017
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Sep 1, 2017

Murdock is green, go!

@aabadie aabadie merged commit fc63f99 into RIOT-OS:master Sep 1, 2017
@vincent-d vincent-d deleted the pr/stmclk_f0_f1 branch September 1, 2017 08:44
@vincent-d vincent-d mentioned this pull request Jan 23, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@haukepetersen haukepetersen haukepetersen left review comments

@aabadie aabadie aabadie approved these changes

@kYc0o kYc0o Awaiting requested review from kYc0o

Assignees

@haukepetersen haukepetersen

@aabadie aabadie

Labels

CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vincent-d @aabadie @haukepetersen