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/sam0_common: merge GPIO driver #7559

Merged
merged 1 commit into from
Oct 1, 2017
Merged

Conversation

dylad
Copy link
Member

@dylad dylad commented Sep 2, 2017

I pursuit my effort to merge samd21 & saml21 drivers together.
Both files were almost the same.
I just move EXTI lines definitions to cpu/sam0_common/include/periph_cpu_common.h instead of cpu/samx21/periph/gpio.c because this is where they should belong IMHO. (Maybe I'm wrong ?)
What do you think ?
Tested on saml21-xpro with tests/periph_gpio (2 EXTI + 2 GPIO out). Everything was fine but I'll need help for testing on a samd21 based board.
Cheers.

@aabadie
Copy link
Contributor

aabadie commented Sep 4, 2017

Tested on Arduino Zero, I could make on-board LED blink using SAUL and default application. Please rebase on latest master so that the shell works !

@aabadie aabadie added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Sep 4, 2017
@aabadie aabadie added this to the Release 2017.10 milestone Sep 4, 2017
@dylad
Copy link
Member Author

dylad commented Sep 4, 2017

@aabadie I rebased this PR as you wanted.
Could you also confirme nothing is broken with EXTI please ?

@aabadie
Copy link
Contributor

aabadie commented Sep 4, 2017

Could you also confirme nothing is broken with EXTI please ?

I'll try to test this today.

@aabadie
Copy link
Contributor

aabadie commented Sep 4, 2017

@dylad, I tested some networking functions (just ping nodes) with samr21-xpro (with EXTI inside) and it's broken with this PR. I have no idea why.

@dylad
Copy link
Member Author

dylad commented Sep 4, 2017

@aabadie Good to know. I'll investigate tonight & submit a fix (if I find what's wrong)

@aabadie
Copy link
Contributor

aabadie commented Sep 4, 2017

@dylad, looking at your changes you seem to have mixed a bit the order of the different initialization steps for the samd21. That's probably the reason of my failing test.

@dylad
Copy link
Member Author

dylad commented Sep 4, 2017

Yes I wanted to avoid as much #ifdef as I can.
Seems It was not the best idea I had...

@dylad
Copy link
Member Author

dylad commented Sep 4, 2017

@aabadie I've arranged the calls order. EXTI should work now.
I'll retry on my saml21-xpro tomorrow and share results :)

@photonthunder
Copy link

@dylad tried this on my samd21 board and it appears to work. One note is that for EXTI the code calls for generic clock 2 which is either the ultra low power internal or external 32 kHz if you use RTC/RTT. It should work either way but there probably should be a comment or something indicating the choice. If I can get some traction, PR #7315 does attempt to organize these generic clock calls.

@dylad
Copy link
Member Author

dylad commented Sep 5, 2017

@photonthunder Thanks for testing, I'll add a comment regarding the generic clock. I'm sure we can optimize a bit more this PR but I'll wait to have the required hardware to do so. I'll also work on an extension to #7315 for SAML21 whenever I find some time :)

Note: I re-tested this PR on saml21-xpro -> works like a charm

@miri64 miri64 requested a review from aabadie September 19, 2017 12:25
@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 Oct 1, 2017
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.

Tested on samr21-xpro, works now.
ACK, please squash

@aabadie aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 1, 2017
@dylad
Copy link
Member Author

dylad commented Oct 1, 2017

@aabadie Squashed.

@aabadie aabadie removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Oct 1, 2017
@aabadie aabadie merged commit 385623f into RIOT-OS:master Oct 1, 2017
@dylad dylad deleted the sam0_gpio branch October 1, 2017 13:06
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: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants