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

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

Closed
FT9R opened this issue Jul 23, 2024 · 7 comments
Closed
Assignees
Labels
bug Something isn't working hal HAL-LL driver-related issue or pull-request. internal bug tracker Issue confirmed and logged into the internal bug tracking system tim Timer

Comments

@FT9R
Copy link

FT9R commented Jul 23, 2024

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 ALABSTM self-assigned this Jul 24, 2024
@ALABSTM ALABSTM added bug Something isn't working hal HAL-LL driver-related issue or pull-request. tim Timer labels Jul 24, 2024
@ALABSTM
Copy link
Collaborator

ALABSTM commented Jul 24, 2024

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
Copy link
Author

FT9R commented Jul 25, 2024

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
Copy link
Collaborator

ALABSTM commented Aug 7, 2024

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
Copy link
Author

FT9R commented Aug 8, 2024

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
Copy link
Collaborator

ALABSTM commented Aug 19, 2024

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
Copy link
Collaborator

ALABSTM commented Aug 19, 2024

ST Internal Reference: 188927

@ALABSTM ALABSTM added the internal bug tracker Issue confirmed and logged into the internal bug tracking system label Aug 19, 2024
@RJMSTM
Copy link
Contributor

RJMSTM commented Jan 15, 2025

fixed in : 43edf38

@RJMSTM RJMSTM closed this as completed Jan 15, 2025
@github-project-automation github-project-automation bot moved this from In progress to Done in stm32cube-mcu-hal-dashboard Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working hal HAL-LL driver-related issue or pull-request. internal bug tracker Issue confirmed and logged into the internal bug tracking system tim Timer
Projects
Development

No branches or pull requests

3 participants