STMicroelectronics / stm32f4xx_hal_driver

Provides the STM32Cube MCU Component "hal_driver" of the STM32F4 series.
BSD 3-Clause "New" or "Revised" License
109 stars 47 forks source link

Update interrupt flag in TIMx->SR is not cleared upon Time Base configuration #31

Open FT9R opened 1 month ago

FT9R commented 1 month ago

It is cleared only with step through debug session. But if I put breakpoints on line 6816 and line 6822, the second breakpoint will not be reached. The reason is that UIF flag is set, though not immediately.

So maybe this part... https://github.com/STMicroelectronics/stm32f4xx_hal_driver/blob/c2e1406d7ea4b73aa42b98ddeb75a8670b1a5a16/Src/stm32f4xx_hal_tim.c#L6814-L6823 ...should be replaced with:

    /* Generate an update event to reload the Prescaler
       and the repetition counter (only for advanced timer) value immediately */
    TIMx->EGR = TIM_EGR_UG;

    /* Poll for UIF flag with timeout */
    while (timeout--)
    {
        if (HAL_IS_BIT_SET(TIMx->SR, TIM_FLAG_UPDATE))
        {
            /* Clear the update flag */
            CLEAR_BIT(TIMx->SR, TIM_FLAG_UPDATE);

            break;
        }
    }

    if (!timeout)
        ErrorHandler();
ALABSTM commented 1 month ago

Hi @FT9R,

I understand your point. However, the driver is intended to be used in "run" mode, not in "debug" mode. Adding the polling mechanism you suggested may ensure a correct behavior both in "run" and "debug" modes. However, it would affect performance in "run" mode.

I will have a deeper look to your report and get back to you soon enough hopefully.

With regards,

FT9R commented 1 month ago

Hello, @ALABSTM

Thank you for your attention. But actually the problem appears with Release configuration as well. After the timer has been initialized, the UIF is still raised. Therefore, if the update interrupt is enabled, the IRQ handler will be reached. I think that the user expects the interrupt procedure to be executed after the timer has started and the counter has reached the ARR value, rather than after the timer is initialized.

I agree that time base configuration is crucial for time in some applications. So you may consider another way to solve this. For example - clear the update flag before enabling timer interrupt in this part:

https://github.com/STMicroelectronics/stm32f4xx_hal_driver/blob/c2e1406d7ea4b73aa42b98ddeb75a8670b1a5a16/Src/stm32f4xx_hal_tim.c#L475-L476

ALABSTM commented 1 month ago

Hi @FT9R,

I could observe the behavior you reported using TIM's TimeBase example. Even though the breakpoint at line 6822 is not hit, the rest of the example runs correctly.

This means that there is no issue with the driver and that it is most likely related to the way the debugger works. I will dig deeper the question and let you know.

With regards,

FT9R commented 1 month ago

Hello, @ALABSTM

Yeah, with basic timer it works ok. But what about One Pulse Timer?

Please, try this instead of standart main.c's content of example you tried:

#include "main.h"

TIM_HandleTypeDef TimHandle;
uint32_t uwPrescalerValue = 0;
uint8_t Time_Counter = 0;

static void SystemClock_Config(void);
static void Error_Handler(void);

int main(void)
{
  if(HAL_Init()!= HAL_OK)
    Error_Handler();

  SystemClock_Config();

  uwPrescalerValue = (uint32_t) ((SystemCoreClock /2) / 10000) - 1;

  TimHandle.Instance = TIMx;
  TimHandle.Init.Period = 10000 - 1;
  TimHandle.Init.Prescaler = uwPrescalerValue;
  TimHandle.Init.ClockDivision = 0;
  TimHandle.Init.CounterMode = TIM_COUNTERMODE_UP;
  TimHandle.Init.AutoReloadPreload = TIM_AUTORELOAD_PRELOAD_DISABLE;
  if(HAL_TIM_Base_Init(&TimHandle) != HAL_OK)
    Error_Handler();

  /*  Switch to One-pulse mode */
  SET_BIT(TimHandle.Instance->CR1, TIM_CR1_OPM);

  if(HAL_TIM_Base_Start_IT(&TimHandle) != HAL_OK)
    Error_Handler();

  while (1)
  {
  }
}

void HAL_TIM_PeriodElapsedCallback(TIM_HandleTypeDef *htim)
{
  if(++Time_Counter >= 2)
    Error_Handler();
}

static void Error_Handler(void)
{
  while(1)
  {
    /*
    Set breakpoint here and look at "Time_Counter" variable.
    We expect it to be equal to 1 because we have One Pulse Timer.
    But actually it is equal to 2.
    */
  }
}

static void SystemClock_Config(void)
{
  RCC_ClkInitTypeDef RCC_ClkInitStruct;
  RCC_OscInitTypeDef RCC_OscInitStruct;

  __HAL_RCC_PWR_CLK_ENABLE();

  __HAL_PWR_VOLTAGESCALING_CONFIG(PWR_REGULATOR_VOLTAGE_SCALE1);

  RCC_OscInitStruct.OscillatorType = RCC_OSCILLATORTYPE_HSE;
  RCC_OscInitStruct.HSEState = RCC_HSE_ON;
  RCC_OscInitStruct.PLL.PLLState = RCC_PLL_ON;
  RCC_OscInitStruct.PLL.PLLSource = RCC_PLLSOURCE_HSE;
  RCC_OscInitStruct.PLL.PLLM = 8;
  RCC_OscInitStruct.PLL.PLLN = 336;
  RCC_OscInitStruct.PLL.PLLP = RCC_PLLP_DIV2;
  RCC_OscInitStruct.PLL.PLLQ = 7;
  HAL_RCC_OscConfig(&RCC_OscInitStruct);

  RCC_ClkInitStruct.ClockType = (RCC_CLOCKTYPE_SYSCLK | RCC_CLOCKTYPE_HCLK | RCC_CLOCKTYPE_PCLK1 | RCC_CLOCKTYPE_PCLK2);
  RCC_ClkInitStruct.SYSCLKSource = RCC_SYSCLKSOURCE_PLLCLK;
  RCC_ClkInitStruct.AHBCLKDivider = RCC_SYSCLK_DIV1;
  RCC_ClkInitStruct.APB1CLKDivider = RCC_HCLK_DIV4;
  RCC_ClkInitStruct.APB2CLKDivider = RCC_HCLK_DIV2;
  HAL_RCC_ClockConfig(&RCC_ClkInitStruct, FLASH_LATENCY_5);

  if (HAL_GetREVID() >= 0x1001)
  {
    __HAL_FLASH_PREFETCH_BUFFER_ENABLE();
  }
}
ALABSTM commented 2 weeks ago

Hi @FT9R,

After further debugging, I start believing you are right about the fact this might be an issue and that a polling mechanism to check the flag status before clearing it would be better than the simple if statement currently used.

Here is what I noticed. The TIM_FLAG_UPDATE is not cleared by the line you pointed out in TIM_Base_SetConfig()...

https://github.com/STMicroelectronics/stm32f4xx_hal_driver/blob/c2e1406d7ea4b73aa42b98ddeb75a8670b1a5a16/Src/stm32f4xx_hal_tim.c#L6821-L6822

Rather by this one in HAL_TIM_IRQHandler().

https://github.com/STMicroelectronics/stm32f4xx_hal_driver/blob/c2e1406d7ea4b73aa42b98ddeb75a8670b1a5a16/Src/stm32f4xx_hal_tim.c#L3956

What happened in my case is that the call to HAL_TIM_Base_Start_IT() enabled the timer's interrupt mechanism, and it was only when HAL_TIM_IRQHandler() has been called for the first time that the TIM_FLAG_UPDATE has been cleared.

I am not sure about the rationale from lines 6816 to 6823. I will log an internal tracker at the attention of our development teams. In case both of us misunderstood or missed something, they will tell us. Otherwise, there may actually be a bug to fix. Let's wait and see. I will keep you informed.

With regards,

ALABSTM commented 2 weeks ago

ST Internal Reference: 188927