Skip to content
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 connectivity line devices (stm32f105 and stm32f107) #205

Merged
merged 1 commit into from
Apr 27, 2020

Conversation

timokroeger
Copy link
Contributor

@timokroeger timokroeger commented Apr 24, 2020

This PR adds support for the stm32f105xx and stm32f107xx devices which are also knows as connectivity line.
They have a peripheral configuration similar to the stm32f103 high-density devices but feature USB OTG FS peripheral and the stm32f107xx devices additionally have an ethernet controller.

Both share a common SVD file with the name stm32f107. I introduced the connectivity cargo feature to always cover both devices. The connectivity feature replaces all previously abandoned stm32f107 and stm32f105 features in configuration attributes.

I could blink a LED on a STM32F105 board with a crystal oscillator. No other tests were run on real hardware.

Please have a look and let me know if I missed something and what can be improved.

@timokroeger timokroeger changed the title Connectivity line Support for connectivity line devices (stm32f105 and stm32f107) Apr 24, 2020
@burrbull
Copy link
Contributor

LGTM

@timokroeger
Copy link
Contributor Author

I found one last issue when building for the existing targets.
For the connectivity line devices the cfgr pllmul().bits() method is unsafe.
This leads to an unnecessary `unsafe` block compiler warning when compiling for stm32f103 where the same method is safe.

Is there a better way to fix this than adding yet another #[cfg()] attribute?

@timokroeger
Copy link
Contributor Author

#[allow(unused_unsafe)] fixed the last remaining warning.
This PR is now ready to merge.

@TheZoq2
Copy link
Member

TheZoq2 commented Apr 26, 2020

Thanks for the PR, looks good to me.

I wonder if the recent version bump to stm32-rs (#206) will have fixed that unused unsafe thing. If not, I'd say we should put a comment explaining why it is there.

@burrbull
Copy link
Contributor

This is because f105/107 have less than 16 levels for pllmul:
https://docs.rs/stm32f1/0.11.0/stm32f1/stm32f107/rcc/cfgr/struct.PLLMUL_W.html

@TheZoq2
Copy link
Member

TheZoq2 commented Apr 26, 2020

Ah alright, I guess a comment is appropriate then

@Rua
Copy link

Rua commented Apr 26, 2020

Great work! I'm very curious how you managed to work out the settings for the PLLs and dividers for the user-requested Hz, since there's so many degrees of freedom and possible combinations on the 107.

@timokroeger
Copy link
Contributor Author

timokroeger commented Apr 26, 2020

Rua you raised a very good point!
I assumed the stm32f103 clock configuration to be portable. I have to investigate if this is correct and need to check if the code can be reused.
Anyway with the recent commits to master this PR needs a rebase before it can land.

@Rua
Copy link

Rua commented Apr 26, 2020

Clock config is very different on the 107, see #109.

@therealprof
Copy link
Member

@Rua Yes it is different and even ST themselves got it wrong for quite a while... zephyrproject-rtos/zephyr#9300

@timokroeger
Copy link
Contributor Author

stm32f105/107 only allow PLLMUL values 4..=9
All other stm32f10x devices support 2..=16
With the patch I just put a restriction on that range. That means the number of possible configurations dropped significantly especially if the internal oscillator is used. The maximum possible sysclk in that case is 9*8MHz/2=36MHz.
Yet it is sufficient for my use case and probably enough for to be useful in general.
ATM I do not have the time and knowledge to implement a more sophisticated logic to choose clock frequencies.

@therealprof
Copy link
Member

The trickier part is getting the frequencies right if you want to use Ethernet and USB. Anyway this needs a rebase to get rid of the merge conflicts. Probably easiest if you run cargo fmt locally before rebasing.

Those are known as “Connectivity line" and have a peripheral configuration similar
to the stm32f103 high-density devices but feature a USB OTG FS peripheral.
stm32f107xx devices additionally have an ethernet controller.

The connectivity line devices support less PLL multiplier values.
A second PLL can be cascaded to achieve greater flexibility.
This patch just does not add support for the second PLL but makes sure
the values for PLL1 stay in the allowed range.
A user should check if the requested clock rate could be configured by looking
at the return value of `freeze()`
@timokroeger
Copy link
Contributor Author

Squashed and rebased

Copy link
Member

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

LGTM for a first shot. Let's land this and take it from there.

@therealprof therealprof merged commit 3fd22e9 into stm32-rs:master Apr 27, 2020
@timokroeger timokroeger deleted the connectivity-line branch May 16, 2020 11:26
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.

None yet

6 participants