Use correct PACs for stm32f318 and stm32f378 #116
Conversation
| @@ -82,7 +82,7 @@ pub use embedded_hal as hal; | |||
| pub use nb; | |||
| pub use nb::block; | |||
|
|
|||
| #[cfg(feature = "stm32f301")] | |||
| #[cfg(any(feature = "stm32f301", feature = "stm32f318"))] | |||
There was a problem hiding this comment.
Instead of using the HAL feature gate, we could use the pac feature gate directly, I guess. It shouldn't make a difference but the logic, which device has which pac feature, would solely live in the Cargo.toml.
There was a problem hiding this comment.
Good idea! However, it seems like you cannot use features of dependencies in #[cfg] attributes. At least I couldn't get it to pick up something like #[cfg(feature = "stm32f3/stm32f303")]. Did I miss something?
I guess one way to make is work is to introduce more internal features like pac-stm32f303. But this fits better in a separate feature cleanup PR.
| AF10: (into_af10, af10, ["stm32f303xb", "stm32f303xc", "stm32f303xd", "stm32f303xe", "stm32f358", "stm32f398",],), | ||
| AF11: (into_af11, af11, ["stm32f378",],), |
There was a problem hiding this comment.
I'm not sure about this change. I guess you where removing all stm32f378 instances, when stm32f373 instances are missing and stm32f318 instances, when stm32f301 instances are missing, am I correct?
But this case is a bit strange. I became suspicious, that only the stm32f378 instance was there in the first place.
This logic was added in #21 and looking at it, I'm not sure how this line came to be.
I checked it against the datasheet available on st.com and for pin PB8 exists an alternate function AF11 / TIM19_CH3 (as we can see on page 43). But in this case, stm32f373 should also be in the list.
I'm not sure now, how to go about this, because this list probably contains more of these errors (if these are errors and I am reading it right).
IMO we can leave it as is for this PR and open another PR, which rechecks all these functions against the datasheet again :)
There was a problem hiding this comment.
I'm not sure about this change. I guess you where removing all
stm32f378instances, whenstm32f373instances are missing andstm32f318instances, whenstm32f301instances are missing, am I correct?
That's correct, but I'm also adding new instances of stm32f378 and stm32f318 which should make up for the ones removed. For example, the instance you're commenting on, in GPIOB, is added again in line 1097: There is a second GPIOB definition, which was for stm32f373 only before. Now stm32f378 is added there too.
So in general, no AF definition should be lost, they are just (slightly) more compact now.
But yeah, the macro situation here is pretty hopeless. We need to refactor this somehow.
According to the "Supported Devices" table in the stm32f3 crate's README (https://crates.io/crates/stm32f3) we were using the wrong PAC module (`stm32f3x8`) for both smt32f318 and stm32f378. Because of that we also supported some peripherals for these devices that are not actually available for them. This commit adds the correct PAC modules for the two devices in question and fixes up the HAL code to use the correct peripherals.
According to the "Supported Devices" table in the stm32f3 crate's README (https://crates.io/crates/stm32f3) we were using the wrong PAC module (
stm32f3x8) for both smt32f318 and stm32f378. Because of that we also supported some peripherals for these devices that are not actually available for them.This commit adds the correct PAC modules for the two devices in question and fixes up the HAL code to use the correct peripherals.