-
Notifications
You must be signed in to change notification settings - Fork 60
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
Support for the STM32L0x0 subfamily #146
base: master
Are you sure you want to change the base?
Conversation
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.
Thank you @jglauche! I've taken a look, and nothing looks out of the ordinary. Left two comments about things I'm not sure about (maybe someone else will know more).
I'm going to convert this PR to a draft, as it's still blocked on various things (as you note). Please mark it as ready to review once it's ready.
@@ -55,6 +55,7 @@ disable-linker-script = [] | |||
|
|||
# STM32L0 subfamilies | |||
# (Warning: Some peripherals, e.g. GPIO, don't follow this subfamily grouping.) | |||
stm32l0x0 = ["stm32l0/stm32l0x0"] |
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.
Is that enough, or do we need to add more features flags (thinking about the ones @dbrgn auto-generated from CubeMX).
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.
probably yes. I only have a l0x0RBT to test against, which is a class 5. I keep finding more things to fix; basically it's similar to the features of L020 class 5 with a lot of stuff cut out.
@@ -324,14 +331,14 @@ macro_rules! impl_channel { | |||
handle: &mut Handle, | |||
address: u32, | |||
) { | |||
handle.dma.$chfield.par.write(|w| w.pa().bits(address)); | |||
unsafe { handle.dma.$chfield.par.write(|w| w.pa().bits(address)); } |
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.
This confuses me. Wouldn't the bits
method be unsafe for all targets? Why did it work in the first place?
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 below]
src/gpio.rs
Outdated
@@ -469,7 +469,7 @@ macro_rules! gpio { | |||
} | |||
|
|||
#[allow(dead_code)] | |||
pub(crate) fn set_alt_mode(&self, mode: AltMode) { | |||
pub fn set_alt_mode(&self, mode: AltMode) { |
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.
is there any reason not to make this public? I looked at other hal crates, they made this a private function, but referenced all the pin modes as function
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 don't see a reason, but I'm not very familiar with that code. I also don't know why the #[allow(dead_code)]
is here and on AltMode
itself. Seems all very weird.
Some updates on this: it looks like my PR to the stm32-rs library for the L0x0 SVD will be merged soon-ish. On this HAL crate... there's a lot of work TBD I tried merging the gpio.rs with an another crate to find out that I should have based it off another one (stm32f3xx-hal).
I'm fighting bit with serial.rs too:
|
I haven't followed this closely, but a general note: It would make our (the maintainers) life MUCH easier, if you could submit as much as possible in as many self-contained pull requests as possible. Doesn't make sense to start now, as it is still blocked on your upstream PR, but I'm just saying, if this turns into one huge pull requests that changes everything, it will be hard to find the time to review it. |
I assumed as much, but I feel like I opened a can of worms here. The more I actually test the more things I find that don't work for a reason.. |
To be clear, improvements and fixes are definitely welcome. I'm just saying, I hope when they're ready they'll arrive in neat packages 😄 |
Quick update:
|
Any news? |
I haven't had the chance to keep my branch updated as my client that I was developing the l0x0 support to switched to l4 MCU |
I implemented L0x0 subfamily and got the examples compile (and verified that the blink example is working!)
#141
Please someone have a look at the unsafe brackets I had to add for my nightly rust in src/dma.rs .
For local testing, I had to
replace
stm32l0
dependency in Cargo.toml tostm32l0 = { path = "../stm32-rs/stm32l0" }
after building the stm32-rs project. I didn't add this to the PR.
Requires my fork of https://github.com/jglauche/stm32-rs
Waits on PR to merge: stm32-rs/stm32-rs#505