-
Notifications
You must be signed in to change notification settings - Fork 221
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
Peripheral interconnect redo, vol 2 (split()
)
#2418
Conversation
24efde3
to
3ad9b69
Compare
8b14709
to
71d9427
Compare
|
||
/// Returns a peripheral [input][interconnect::InputSignal] connected to | ||
/// this pin. | ||
impl<P> Input<'_, P> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment on spi_slave
test.
hil-test/tests/spi_slave.rs
Outdated
let cs = cs_gpio.peripheral_input(); | ||
let sclk = sclk_gpio.peripheral_input(); | ||
let mosi = mosi_gpio.peripheral_input(); | ||
// let miso = unsafe { miso_gpio.clone_unchecked() }.into_peripheral_output(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm considering my options here. This unsafe clone can be replaced by a Flex::split_peripheral_output() -> (interconnect::OutputSignal, Input)
(and same method on Output and OutputOpenDrain), but then we can't have split
and into_peripheral_output
on Input
. I'm not sure which one is better, working with Flex is somewhat low level - but so is splitting off an output signal and keeping a GPIO input.
This comment was marked as resolved.
This comment was marked as resolved.
52f4fb0
to
71d9427
Compare
4d59d75
to
16b376a
Compare
split()
)
Perhaps a naive question, what's the point of the owned |
What is "owned split"? |
Ah this PR is a preparation for #2419. After #2419 there will be a difference between passing a pin to a peripheral (may bypass the GPIO matrix) and passing a signal to a peripheral (forces use of the GPIO matrix). We can't provide a non-splitting way to obtain signals, because GPIO matrix usage won't be unambiguous. |
Ah okay I think I answered my own question actually. This PR stops someone using a GPIO pin's signal for something and then doing something with the pin itself which might end up overwriting some signal-related operations? |
The only intended change is that peripherals won't try to force their specific alternate function on a GPIO, but instead always go through the GPIO matrix. The side-effect is that "split-then-create-a-gpio-driver" use cases will need to be flipped to "create-driver-then-split-off-signal". There shouldn't be any reduction in flexibility I hope. |
16b376a
to
d6cf19a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, one note about the migration guide entry.
8754f5d
to
0d605cb
Compare
Not really related but I'm wondering if we should briefly explain things like signals and alternate functions in the GPIO module docs |
Definitely, although I'd leave it to upcoming documentation work (or just the next PR) |
0d605cb
to
c6ce9f3
Compare
c6ce9f3
to
6c3f051
Compare
I've tweaked the docs in #2419 a bit but as we don't exactly advertise alternate functions I think it's fine to keep their docs where they are. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
This PR replaces the previous getters with a
split
function. This will allow eventually resolving #2273 by introducing different variants for when the GPIO is split, vs when the GPIO is passed wholly to a peripheral. This also allows us to remove a few hacks around input/output initialization - if a pin was split,enable_output
shouldn't disable input, for example.Pin drivers retain their getters and instead convert into signals that (will) force GPIO matrix use.