Skip to content

cpu/stm32_common: unify gpio driver#6714

Merged
lebrush merged 2 commits intoRIOT-OS:masterfrom
OTAkeys:pr/stm32_unify_gpio
Mar 13, 2017
Merged

cpu/stm32_common: unify gpio driver#6714
lebrush merged 2 commits intoRIOT-OS:masterfrom
OTAkeys:pr/stm32_unify_gpio

Conversation

@vincent-d
Copy link
Copy Markdown
Member

@vincent-d vincent-d commented Mar 7, 2017

When taking a look at #6705, I realized the stm32 gpio drivers could be unified.

This PR unify them, except for stm32f1, which has a slightly different gpio peripheral.

I had to update the vendor header for some families.

Boards are also updated.

Tested on nucleo32-f042, nucleo-f091, nucleo-f401, nucleo144-f207 and nucleo144-f413 with examples/default (saul test), tests/periph_gpio and tests/leds

@vincent-d vincent-d added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Mar 7, 2017
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Mar 7, 2017

Nice PR !

@vincent-d
Copy link
Copy Markdown
Member Author

vincent-d commented Mar 7, 2017 •
edited by aabadie
Loading

All stm32 cpu (except f1) are impacted. Some boards (LED mainly) are impacted. This checklist should help testing. I already checked the board I tested, I don't have any of the others.

  • stm32f0
    • nucleo-f091
    • nucleo32-f042
  • stm32f2
    • nucleo144-f207
  • stm32f3
    • stm32f3discovery
    • nucleo-f303
    • nucleo32-f303
  • stm32f4
    • f4vi1
    • msbiot
    • nucleo-f401
    • nucleo144-f413
    • stm32f4discovery
  • stm32l0
    • at least one nucleo l0-based
  • stm32l1
    • limifrog-v1
    • nucleo-l1

lebrush
lebrush reviewed Mar 7, 2017
View reviewed changes
cpu/stm32_common/include/periph_cpu_common.h Outdated
#endif /* ndef DOXYGEN */

/**
* @brief Available ports on the STM32F2 family
Copy link
Copy Markdown
Member

@lebrush lebrush Mar 7, 2017

Choose a reason for hiding this comment

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

Now is not only for the STM32F2 :-)

Maybe is not a good idea to define all ports to all mcus since some do not have them available. The code would compile anyway (i.e. F0/L0 don't have E, G, H, I. F3 doesn't have G, H, I. L1 doesn't have I)

Copy link
Copy Markdown
Member Author

@vincent-d vincent-d Mar 7, 2017

Choose a reason for hiding this comment

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

You're right. It needs to be in the cpu specific periph_cpu.h file, because L1 port are not "sorted" in memory (port H base address is between port E and port F).

Then I moved back to each periph_cpu.h and went through each stm32xxxx.h file to check which port were defined. It should be OK now.

@lebrush
Copy link
Copy Markdown
Member

lebrush commented Mar 7, 2017 •
edited
Loading

Tested on stm32f4discovery, nucleo-l053.

UPDATE: retested with the latest changes and OK.

@vincent-d vincent-d added this to the Release 2017.04 milestone Mar 9, 2017
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Mar 13, 2017

Tested on stm32f3discovery and nucleo-f303 : OK

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Mar 13, 2017

Tested the nucleo-l1 and nucleo32-f303 : ok

@aabadie aabadie added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Mar 13, 2017
@aabadie aabadie mentioned this pull request Mar 13, 2017
Merged
aabadie
aabadie reviewed Mar 13, 2017
View reviewed changes
cpu/stm32f4/include/vendor/stm32f401xe.h
__IO uint32_t ODR; /*!< GPIO port output data register, Address offset: 0x14 */
__IO uint16_t BSRRL; /*!< GPIO port bit set/reset low register, Address offset: 0x18 */
__IO uint16_t BSRRH; /*!< GPIO port bit set/reset high register, Address offset: 0x1A */
__IO uint32_t BSRR; /*!< GPIO port bit set/reset low register, Address offset: 0x18 */
Copy link
Copy Markdown
Contributor

@aabadie aabadie Mar 13, 2017

Choose a reason for hiding this comment

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

very nice ! I was bored having to change it all the times in the CMSIS.

aabadie
aabadie approved these changes Mar 13, 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
This PR is very useful. Thanks a lot for this.

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Mar 13, 2017

please squash.

@vincent-d vincent-d force-pushed the pr/stm32_unify_gpio branch from a9082a6 to c302b76 Compare March 13, 2017 14:10
@vincent-d
Copy link
Copy Markdown
Member Author

vincent-d commented Mar 13, 2017

Squashed

@aabadie aabadie 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 labels Mar 13, 2017
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Mar 13, 2017

Let's wait for CI's opinion. I also think that we can skip the remaining boards, their cpu family has already been tested.

@kaspar030 kaspar030 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 13, 2017
@lebrush
Copy link
Copy Markdown
Member

lebrush commented Mar 13, 2017

And go!

@lebrush lebrush merged commit 2af148d into RIOT-OS:master Mar 13, 2017
@lebrush lebrush mentioned this pull request Mar 13, 2017
Merged
@aabadie aabadie mentioned this pull request Mar 13, 2017
Merged
@aabadie aabadie mentioned this pull request Mar 15, 2017
Merged
@vincent-d vincent-d deleted the pr/stm32_unify_gpio branch March 15, 2017 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aabadie aabadie aabadie approved these changes

+1 more reviewer

@lebrush lebrush lebrush left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@haukepetersen haukepetersen

@aabadie aabadie

@kYc0o kYc0o

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: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Milestone

Release 2017.04

Development

Successfully merging this pull request may close these issues.

6 participants

@vincent-d @aabadie @lebrush @haukepetersen @kaspar030 @kYc0o