ICU driver and 32 bits timer

[alex31]

Hello,

ICU driver behave the same with 32 bits timers than with 16 bits timer, is it by design ?

If I want 32 bits timer with ICU to not overflow like a 16 bit timer, i have to manually set ICUDx.tim->ARR to 0xFFFFFFFF after icustart.

Perhaps it should be done in hal_icu_lld.c if the timer is a 32 bits timer ?

Alexandre

[Giovanni]

Hi,

Moving to "bug reports".

Giovanni

[FXCoder]

Hi Alexandre
Do you have a callback on overflow setup?
Which MCU?
Which TIM?
—
Bob

[alex31]

Hi,

Do you have a callback on overflow setup?

No callback, but i don't think it's matter.

Which MCU?

F4 and F7, probably it's the same problem with all TIMv1 STM32 that expose tim2 and/or tim5 which are the 32 bits timers.

Which TIM?

I use tim2 but there should be the same problem with tim5

In fact, Auto Reload Register ARR is set at only one place in the lld code, for all kind of timers :

Code: Select all

os/hal/ports/STM32/LLD/TIMv1/hal_icu_lld.c:552:  icup->tim->ARR  = 0xFFFF;

I don't know if the correct fix is to use 0xFFFFFFFF value for 32 bits timers, or, perhaps more useful, expose this value in the configuration structure (at least in hardware dependant part, but probably in the hardware independant part since all timers that can do input capture have an auto reload register.

Alexandre

[Giovanni]

Probably it should go in configuration.

Giovanni

[FXCoder]

Hi,
Since the TIM ARR has reserved address space for 32 bit data for all timers and the struct definition for ARR is always 32 bit, I figure there should be no harm in just making that value 0xFFFFFFFF for all timers for now?

BTW Alexandre there is a bug with ICU overflow if attempting to use the callback to scale the timer size up.
So in the spirit of over doing it I've written a revised ICU which can scale all timers to 32 bit (and 64 bit), handles overflow scaling and as a consequence, also keeps a flag of 16 or 32 bit timer type in the ICU driver object.
I'll pull that out of storage, make a few minor tidy up changes that need to be done, and then send it in for review by Giovanni.

--
Bob

[Giovanni]

Hi,

STM32 ICU driver now allows to setup the ARR register in the configuration structure, the default value should be 0xFFFFFFFFU. In 19.x I simply changed the constant to 0xFFFFFFFF in order to not break things for users.

Bob, about that callback problem, any news? should I attempt a fix?

Giovanni

[FXCoder]

Hi Giovanni,
#0 OK on the ARR change. For overflow handling it is redundant to change the size of ARR value between 16 and 32 bit timers.
The compiler will generate a word write (mov.w) due to the TIM struct defining ARR as uint32_t.
So writing FFFF into "thin air" of the non-existent upper 16 bits of ARR of a 16 bit timer
should not be an issue (unless there is some bus monitor watching for such things )

#1 Its worth noting that given the way the ICU driver works, an ICU overflow callback will occur on the ARR match which is one clock before the timer actually overflows (wraps around). So it is really an "overflow coming up at next clock" callback.
This may or may not be a big deal depending on use case. For a really (really) slow ICU use case you'd need to know this.

#2 I have the revised ICU with the overflow bug fix and the optional (#define enabled) timer normalization (all TIMs are seen as 32 bit or even 64 bit).
This work was done prior the timer additions made recently.
I'll add those changes, test and post a zip today.
--
Bob

[FXCoder]

Hi Giovanni,
Attached is a zip of the updated ICU.
To test I had to bring up the new *.inc format on the F413 where I have a board that uses ICU in normal and also overflow scaling.
So that took a little while for the TIM support (I have yet to finish the EXTI and UART parts of stm32_isr.x).
I tested in polled mode on TIM12 for scaling and have yet to get the TIM12 interrupt config completed/functioning.
The other normal ICU on TIM4 of the board tested OK using interrupt so I've just got something not right for TIM12 in the F4 settings.

Anyway here is the overview of the updated ICU...
To handle the overflow bug w/o using the new overflow scaling features there is now a macro icuIsCurrentCountValidX(...) which checks if an overflow occurred. So when an ICU width or period value is read it is only valid if icuIsCurrentCountValidX(...) returns true.

The new scaling feature(s) are enabled using STM32_ICU_NORMALISE_TIMER_WIDTHS (scales all timers to 32 bit) or STM32_ICU_ENABLE_EXTENDED_WIDTH (extends scaling to 64 bit) and these too should be checked with the macro which is based on the scaled widths only (not the underlying timer h/w width).
There is another macro icuGetCounterWidthX(...) to get the ICU width as the countable width of the ICU based on the mode (original, scaled, extended).

See the attached zip.
Please review and test.
I'll sort the TIM12 thing out tomorrow.

Also in the walk through I found one typo in the current ICU source that is now fixed as part of the zip files.
--
Bob

Code: Select all

Index: hal_icu_lld.h
===================================================================
--- hal_icu_lld.h   (revision 13617)
+++ hal_icu_lld.h   (working copy)
@@ -556,7 +556,7 @@
 #endif
 
 #if STM32_ICU_USE_TIM11
-#if defined(STM32_TIM151_IS_USED)
+#if defined(STM32_TIM11_IS_USED)
 #error "ICUD11 requires TIM11 but the timer is already used"
 #else
 #define STM32_TIM11_IS_USED

[FXCoder]

Update...
TIM12 problem resolved so both polled and interrupt mode tested (scaling a 16 -> 32 bit).
--
Bob