Skip to content

common/gpio: Allow disconnecting IO from peripheral #147

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

Merged

Conversation

har7an
Copy link
Contributor

@har7an har7an commented Aug 10, 2022

This is a work-in-progress that enables disconnecting IOs from peripherals they were previously associated with.

It is currently implemented only for output signals because I'm not quite sure yet if I got it right at all. I'm looking forward to your feedback!

Why?

I need this personally for a project I'm currently working on, in order to replicate some existing C code that performs half-duplex UART with a shared IO line for RX and TX.

Another, probably more obvious and cool application comes with the SpiDevice trait from embedded hal 1.0.0-alpha.8. Here, disconnecting peripherals allows us to share the SPI bus, and have the CS line controlled by hardware by connecting it to CS before we start the transactions, and disconnecting it afterwards.

In #100, @jrmoulton already implements the bit where the CS line is selectively connected before a transaction. This is the "missing" bit to have the CS line controlled by hardware on a shared bus.

@har7an
Copy link
Contributor Author

har7an commented Aug 10, 2022

I don't understand the Github UI. Where do I mark PRs as "draft"?

@bjoernQ bjoernQ marked this pull request as draft August 10, 2022 09:35
@har7an
Copy link
Contributor Author

har7an commented Aug 10, 2022

@bjoernQ Implemented disconnecting for the InputPin, too, and added a bit of docs to explain what it does. Would you mind having a look to tell me if I got the concept right?

@har7an har7an marked this pull request as ready for review August 10, 2022 12:31
@har7an har7an changed the title WIP: common/gpio: Allow disconnecting IO from peripheral common/gpio: Allow disconnecting IO from peripheral Aug 10, 2022
@jrmoulton
Copy link
Contributor

Ahhh I was just trying to connect the pin back to the general GPIO signal(128) but this explains why I was seeing small effects in the voltage even after a pin was connected to some other output signal.

@bjoernQ
Copy link
Contributor

bjoernQ commented Aug 11, 2022

@jessebraham mind taking a look?

besides the things I commented this should be technically fine - I thought if the new functions should be unsafe since you can move the pin into some random state not really covered by the type-state pattern but that is true for all those functions and those functions are not meant to be used in user code I guess

@bjoernQ bjoernQ requested a review from jessebraham August 11, 2022 06:18
@jessebraham
Copy link
Member

I share the same concerns as Bjoern, however it's not like our current API is completely sound anyways so I'm not sure that's a fair criticism. I think it's probably fine to merge this, as my gut tells me we'll end up doing a major rework of the GPIO stuff at some point in the future anyways. If it becomes a problem it can always be removed after all.

passed in from the gpio configuration.
@har7an
Copy link
Contributor Author

har7an commented Aug 16, 2022

Done!

Copy link
Member

@jessebraham jessebraham left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants