F4 GPIO #186
F4 GPIO #186
Conversation
burrbull
force-pushed
the
f4-gpio
branch
3 times, most recently
from
e122258 to
cd18012
Compare
burrbull
force-pushed
the
f4-gpio
branch
4 times, most recently
from
5b40c21 to
83e5b28
Compare
|
cc @usbalbin |
| _tx: TX, | ||
| _rx: RX, | ||
| pins: (impl Into<Self::Tx>, impl Into<Self::Rx>), |
There was a problem hiding this comment.
Why this change? Why a pair instead of two arguments?
| pub struct Tx<USART: Instance, Dma, Otype = PushPull> { | ||
| pin: USART::Tx<Otype>, | ||
| pub struct Tx<USART: Instance, Dma = NoDMA, Otype = PushPull> { | ||
| pin: Option<USART::Tx<Otype>>, |
There was a problem hiding this comment.
This effectively makes this no longer a ZST? What is the advantage of the Option here as opposed to something like struct NoPin? Correct me if I am wrong but one disadvantage with Option is that you still have to specify the type even when passing None. That in my opinion defeats any ergonomics advantages vs just passing NoPin. Also there is not really any reduction in the amount of generic type parameters.
However I am still open to be convinced otherwise :)
There was a problem hiding this comment.
This effectively makes this no longer a ZST?
This PR makes all alternate pins non-ZST as they are enums. If it is important to you to keep these structs a ZST than this PR is not for you.
Correct me if I am wrong but one disadvantage with Option is that you still have to specify the type even when passing None
Yes, this is issue. That is why I've added SPI1::NoMiso and friends in SpiExt trait. And alternative is to add None variant in each enum.
The reason of this PR is:
- You can keep pins inside structs and release with much less of generics (using associated types instead).
- Through
Intoyou can passgpioa.pa0(Default on another Mode except another Alternate) instead ofgpioa.pa0.into_alternate()(still allowed). In first case conversion will be done in.into()during peripheral initialization. In second case it will be done before peripheral initialization.
Saying about firmware size I don't see significant change in release mode. Although you should test it yourself if you interested in this PR.
| use super::*; | ||
|
|
||
| pin! { | ||
| <Out> default:PushPull for [ |
There was a problem hiding this comment.
Just to see if I am following here.
Here we create an enum Out which contains all the possible pins that can be used as output for this comparator. Then in the comparator we take a pin that implements Into<Out> which I assume all pins that have a corresponding enum variant do? Thanks to the enum we no longer need a type parameter for the pin type of peripherals that store the pin, which the comparator happens to not do. So in this case this should not incur any additional cost.
Just out curiosity how often do people actually need to be able to disable a peripheral and get back all pins etc? I have never felt the need. But that may very well be me missing some use case. If this functionality is indeed not needed then we do not need to store the pin and the peripheral should stay ZST(assuming nothing else needs to be stored), right?
There was a problem hiding this comment.
Just out curiosity how often do people actually need to be able to disable a peripheral and get back all pins etc?
Not often. But sometimes people ask this functionality. I think it depends on peripheral.
should stay ZST
When you say "ZST" what features you mean except size of firmware?
There was a problem hiding this comment.
I guess speed and size. Sorry, I have nothing more concrete in mind other than the feeling that there would be less for the optimizer to mess up(fail to optimize well) if there were simply nothing there to begin with. However I suppose the cost of a perhipheral having a size would probably only show when passing it around by value which I imagine is quite rare?
Sorry, my concern is likely completely moot. However on the of chance that it is not...
Not often. But sometimes people ask this functionality. I think it depends on peripheral.
Interesting
There was a problem hiding this comment.
Usually it optimizes approximately the same. Sometimes even better. But tests are needed anyway.
No description provided.