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.

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

cpu/stm32_common: unify gpio driver #6714

Merged
merged 2 commits into from
Mar 13, 2017

Conversation

vincent-d
Copy link
Member

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
Contributor

aabadie commented Mar 7, 2017

Nice PR !

@vincent-d
Copy link
Member Author

vincent-d commented Mar 7, 2017

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

#endif /* ndef DOXYGEN */

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

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
Member Author

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
Member

lebrush commented Mar 7, 2017

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
Contributor

aabadie commented Mar 13, 2017

Tested on stm32f3discovery and nucleo-f303 : OK

@aabadie
Copy link
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
@@ -287,8 +287,7 @@ typedef struct
__IO uint32_t PUPDR; /*!< GPIO port pull-up/pull-down register, Address offset: 0x0C */
__IO uint32_t IDR; /*!< GPIO port input data register, Address offset: 0x10 */
__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
Contributor

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.

Copy link
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
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
Member Author

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
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
Member

lebrush commented Mar 13, 2017

And go!

@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
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
Development

Successfully merging this pull request may close these issues.

None yet

6 participants