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

Timers: HAL_TIM_PWM_Stop() disable timer even channel 5 or channel 6 are enabled #106

Open
KamilDuljas opened this issue Oct 23, 2023 · 3 comments
Assignees
Labels
bug Something isn't working hal HAL-LL driver-related issue or pull-request. internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system tim TIM-related issue or pull-request.

Comments

@KamilDuljas
Copy link

TIM_CCER_CCxE_MASK makro should be contain 6 channels, not 4. Missing TIM_CCER_CC5E and TIM_CCER_CC6E
image

Example:

HAL_TIM_PWM_Start(&htim1,TIM_CHANNEL_1);
HAL_TIM_PWM_Start(&htim1,TIM_CHANNEL_2);
HAL_TIM_PWM_Start(&htim1,TIM_CHANNEL_5);

HAL_TIM_PWM_Stop(&htim1,TIM_CHANNEL_1);
// TIM_CR1_CEN is '1' (TIM_BDTR_MOE is '1' for  advanced timers)
HAL_TIM_PWM_Stop(&htim1,TIM_CHANNEL_2);
// TIM_CR1_CEN is '0' but CHANNEL 5 is still active!!!
HAL_TIM_PWM_Stop(&htim1,TIM_CHANNEL_5);

The issue related to macros TIM_CCER_CCxE_MASK and:

/**
  * @brief  Disable the TIM peripheral.
  * @param  __HANDLE__ TIM handle
  * @retval None
  */
#define __HAL_TIM_DISABLE(__HANDLE__) \
  do { \
    if (((__HANDLE__)->Instance->CCER & TIM_CCER_CCxE_MASK) == 0UL) \
    { \
      if(((__HANDLE__)->Instance->CCER & TIM_CCER_CCxNE_MASK) == 0UL) \
      { \
        (__HANDLE__)->Instance->CR1 &= ~(TIM_CR1_CEN); \
      } \
    } \
  } while(0)

/**
  * @brief  Disable the TIM main Output.
  * @param  __HANDLE__ TIM handle
  * @retval None
  * @note The Main Output Enable of a timer instance is disabled only if all the CCx and CCxN channels have been
  *       disabled
  */
#define __HAL_TIM_MOE_DISABLE(__HANDLE__) \
  do { \
    if (((__HANDLE__)->Instance->CCER & TIM_CCER_CCxE_MASK) == 0UL) \
    { \
      if(((__HANDLE__)->Instance->CCER & TIM_CCER_CCxNE_MASK) == 0UL) \
      { \
        (__HANDLE__)->Instance->BDTR &= ~(TIM_BDTR_MOE); \
      } \
    } \
  } while(0)

Also please look closer TIM_CCER_CCxNE_MASK

@KamilDuljas
Copy link
Author

H7 repo is also affected.

@ALABSTM ALABSTM self-assigned this Oct 23, 2023
@ALABSTM ALABSTM added bug Something isn't working hal HAL-LL driver-related issue or pull-request. labels Oct 23, 2023
@KRASTM KRASTM self-assigned this Oct 25, 2024
@KRASTM
Copy link
Contributor

KRASTM commented Oct 28, 2024

Hello @KamilDuljas,

Thank you for the report.

Regarding the issue, and based on the RM either for F7 or H7:

  • For the TIM_CCER_CCxNE_MASK: there are no CC4NE, CC5NE and CC6NE, so I think it's ok for this macro.

  • For the TIM_CCER_CCxE_MASK: I reported this one to our team in order to analyze it.

With regards,

@KRASTM
Copy link
Contributor

KRASTM commented Oct 28, 2024

ST Internal Reference: 195184

@KRASTM KRASTM added internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system tim TIM-related issue or pull-request. labels Oct 28, 2024
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 reported into a ticket in the internal bug tracking system tim TIM-related issue or pull-request.
Projects
Development

No branches or pull requests

3 participants