ARMmbed / mbed-hal-st-stm32f4

mbed HAL for ST STM32F4-series microcontrollers
Other
7 stars 21 forks source link

LP: Fix overflow in timer2 prescaler calculation. #21

Closed niklarm closed 9 years ago

niklarm commented 9 years ago

This fixes the prescaler overflow and therefore the accuracy of the LP timer.

In all target configurations in the chip modules the MINAR_PLATFORM_TIME_BASE needs to be set to 2000 NOT 1000.

Fixes #20. Depends on https://github.com/ARMmbed/mbed-hal-st-stm32f429zi/pull/15.

@bogdanm @bremoran @0xc0170

bogdanm commented 9 years ago

LGTM. I assume that now the LED blinks at the correct rate?

0xc0170 commented 9 years ago

+1

betzw commented 9 years ago

The generic problem I am seeing here that within the STM32F4 families the clock tree might be configured quite differnently and therefore there is no single solution which fits all cases. At least the situation on STM32F429 is different from the one on STM32F401.

0xc0170 commented 9 years ago

@betzw how different? Are you saying this won't work for F401?

@niklas-arm Shouldn't we check the APB1 prescaler valueif we need to /2 or not?

betzw commented 9 years ago

Yes, I believe this won't work for F401, which drives TIM2 @ 84Mhz

bogdanm commented 9 years ago

It looks to me like the only thing that'll fix this kind of problem properly is a form of device clock tree. We're probably going to have to implement that anyway at some point. Failing that, there are preprocessor tricks that can help us get through until we have a proper device clock tree. For now, I'd suggest to leave this as it is.

betzw commented 9 years ago

I would say to move this from mbed-hal-st-stm32f4 to mbed-hal-st-stm32f401re and mbed-hal-st-stm32f429zi respectively.

bogdanm commented 9 years ago

Ah yes, that is a better idea, thank you. We can move it back to the generic layer later on.

0xc0170 commented 9 years ago

@betzw I believe we can fix this for all f4. I remembered having this exact problem and found the solution to this. Here's the code snippet: https://github.com/mbedmicro/mbed/blob/master/libraries/mbed/targets/cmsis/TARGET_STM/TARGET_STM32F4/TARGET_NUCLEO_F446RE/hal_tick.c#L75

Btw, the same problem is in hal_tick.c which needs the same fix.

@niklas-arm If you agree, we could apply same fix in this PR to lp ticker, and then fix hal_tick.c which is in cmsis stm32f4,there's an issue for that: https://github.com/ARMmbed/cmsis-core-stm32f4/issues/6. That one triggered the discussion and the above fix.

niklarm commented 9 years ago

@0xc0170 I agree and this fix is working for the F429 right now. I will amend this PR accordingly.

I propose we keep the 2000 ticks per second, which allows correct operation until 131MHz timer input clock speed without any modification of MINAR_PLATFORM_TIME_BASE for every single target. This would then not require any adaption for faster microcontrollers, like for the STM32F7 which runs at 216MHz CPU speed, or max. 108MHz APB1 clock speed. (The F7 also has a dedicated Low Power Timer in hardware).