STMicroelectronics / stm32g4xx_hal_driver

Provides the STM32Cube MCU Component "hal_driver" of the STM32G4 series.
BSD 3-Clause "New" or "Revised" License
14 stars 10 forks source link

IS_TIM_PERIOD assertion fails when using dithering #18

Open jacobq opened 2 weeks ago

jacobq commented 2 weeks ago

When I configure an advanced timer (e.g., TIM8) to use dithering, the corresponding period/ARR, counter/CNT, and CCRx registers should be extended from 16 bits to 20 bits. However, the HAL's assertions like IS_TIM_PERIOD fail for values > 65,535 (= 2^16 - 1) even though they are less than 1,048,559 (0xFFFEF, which is given as the maximum allowed value on page 1131/2138). See also STM32CubeIDE/MX restricts timer period/arr to 12-bit values when using dithering on ST's community forum, which references that this is being tracked internally with ticket number 169221.

I think this could be fixed with a change like the following. Replace this:

#define IS_TIM_PERIOD(__HANDLE__, __PERIOD__) ((IS_TIM_32B_COUNTER_INSTANCE(((__HANDLE__)->Instance)) == 0U) ? \ 
                                                (((__PERIOD__) > 0U) && ((__PERIOD__) <= 0x0000FFFFU)) :        \ 
                                                ((__PERIOD__) > 0U)) 

with the following modified version that includes a check for dithering being enabled (very similar to LL_TIM_IsEnabledDithering):

#define IS_TIM_PERIOD(__HANDLE__, __PERIOD__) \
( \
    IS_TIM_32B_COUNTER_INSTANCE(((__HANDLE__)->Instance)) == 0U ? \
    ( \
        /* 16-bit counter, possibly extended with dithering */ \
        READ_BIT(__HANDLE__->Instance->CR1, TIM_CR1_DITHEN) == 0U ? \
        ( \
            /* Dithering disabled */ \
            ((__PERIOD__) > 0U) && ((__PERIOD__) <= 0x0000FFFFU) \
        ) : \
        ( \
            /* Dithering enabled: integer part <= 65534 & dithered part <= 15 */ \
            ((__PERIOD__) > 0U) && ((__PERIOD__) <= 0x000FFFEFU) \
        ) \
    ) : \
    ( \
        /* 32-bit counter */ \
        (__PERIOD__) > 0U \
    ) \
)

Figure 305 from RM0440 shows register layout

KRASTM commented 2 weeks ago

Hello @jacobq,

Thank you for the report.

Regarding your issue, and according to RM0440, for the counter/CNT and when Dithering mode is activated The register only holds the non-dithered part in CNT [15:0]. The fractional part is not available, so the max value is 0xFFFF and that's why you encounter a failure with the assert param with a value >65,535, but for the other registers are extended from 16 to 20 bits. Beside, there is a TIM Dithering example with STM32G474E that's work fine, you can check it for more details.

However, I will share your idea and fix with our team in order to analyze it.

With regards,

jacobq commented 1 week ago

...for the counter/CNT and when Dithering mode is activated The register only holds the non-dithered part in CNT [15:0]. The fractional part is not available, so the max value is 0xFFFF.

That's fine for the CNT register (I edited the issue to strike-out that part where I was mistaken). However, the code generated by STM32CubeMX/IDE causes IS_TIM_PERIOD to be used with htim->Init.Period (e.g., in HAL_TIM_Base_Init), which is the ARR register and does include the dithered portion.

KRASTM commented 1 week ago

ST Internal Reference: 190068