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

[sam] GPIO setOutput overrides peripheral control #822

Merged

Conversation

mcbridejc
Copy link
Contributor

Calling setOutput now overrides peripheral control of the pin, which is
consistent with the STM32 GPIO implementation. This commit also
ensures output value is set before the output drive is enabled to prevent
glitch.

Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

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

Great!

Copy link
Member

@chris-durand chris-durand left a comment

Choose a reason for hiding this comment

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

Thanks, could you also do it for setInput()? On STM32 this will also reset the alternate function mode to plain input.

@chris-durand
Copy link
Member

I will also make the same change for the other SAM GPIO implementation and add it here.

@mcbridejc mcbridejc force-pushed the feature/gpio-output-periph-override branch from a4da024 to cfbd963 Compare February 8, 2022 00:14
@mcbridejc mcbridejc closed this Feb 8, 2022
@mcbridejc mcbridejc reopened this Feb 8, 2022
@mcbridejc
Copy link
Contributor Author

Thanks, could you also do it for setInput()? On STM32 this will also reset the alternate function mode to plain input.

That seems pretty sensible. Done.

@chris-durand
Copy link
Member

I found another bug in the other GPIO implementation (D2x, D/E5x). Calling configure(InputMode) will reset peripheral control as well but this function should only configure the pull-up/down mode. Will fix it.

@salkinium salkinium added this to the 2022q1 milestone Feb 8, 2022
@chris-durand
Copy link
Member

I have pushed the fixes for the SAM D21, D/E5x, GPIO implementation. I will test it tomorrow on a E54 dev board.

@chris-durand
Copy link
Member

I will test it tomorrow on a E54 dev board.

It works. I cherry-picked the commits onto the SAME5x PR. With the changes from this PR the button on the dev board works, without them it doesn't. That's proof enough for me.

I will squash the fixup commits and rebase.

mcbridejc and others added 2 commits February 9, 2022 02:51
Calling setOutput now overrides peripheral control of the pin. This
behavior is consistent with the STM32 GPIO implementation, this commit also
ensures output value is set before the output drive is enabled to prevent
glitch.

Co-authored-by: Christopher Durand <christopher.durand@rwth-aachen.de>
@chris-durand chris-durand force-pushed the feature/gpio-output-periph-override branch from 72170ff to c868f59 Compare February 9, 2022 01:51
@chris-durand chris-durand added the ci:hal Triggers the exhaustive HAL compile CI jobs label Feb 9, 2022
@salkinium salkinium merged commit c868f59 into modm-io:develop Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:hal Triggers the exhaustive HAL compile CI jobs enhancement 🌈 fix 💎
Development

Successfully merging this pull request may close these issues.

None yet

3 participants