STMicroelectronics / STM32CubeH7

STM32Cube MCU Full Package for the STM32H7 series - (HAL + LL Drivers, CMSIS Core, CMSIS Device, MW libraries plus a set of Projects running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits))
https://www.st.com/en/embedded-software/stm32cubeh7.html
Other
490 stars 302 forks source link

SysTick interrupt priority confusion #199

Closed escherstair closed 1 year ago

escherstair commented 2 years ago

As written here

Arm Cortex-M processors offer very versatile interrupt priority management, but unfortunately, the multiple priority numbering conventions used in managing the interrupt priorities are often counter-intuitive, inconsistent, and confusing, which can lead to bugs

When using an OS, the priority for SysTick is crucial to have everything working properly. And here comes the confusion. Here I can see https://github.com/STMicroelectronics/STM32CubeH7/blob/e9472e471cc8974c4a2596fc86e565241871c20e/Drivers/STM32H7xx_HAL_Driver/Src/stm32h7xx_hal.c#L255-L256

But in several documentation (as an example here) I read that SysTick should run at the lowest possible priority.

Looking here https://github.com/STMicroelectronics/STM32CubeH7/blob/e9472e471cc8974c4a2596fc86e565241871c20e/Drivers/STM32H7xx_HAL_Driver/Src/stm32h7xx_hal.c#L170 I see that the priority it's set to TICK_INT_PRIORITY which is 0xF https://github.com/STMicroelectronics/STM32CubeH7/blob/e9472e471cc8974c4a2596fc86e565241871c20e/Drivers/STM32H7xx_HAL_Driver/Inc/stm32h7xx_hal_conf_template.h#L166

I think that the documentation for HAL_InitTick() is wrong.Can someone double check, please?

ALABSTM commented 2 years ago

Hi @escherstair,

Thank you for this report. We will get back to you with a feedback as soon as possible. Please excuse the delay it may take us sometimes to reply. Thank you for your comprehension.

With regards,

ALABSTM commented 2 years ago

Hi @escherstair,

Thank you for your question. Actually, as you mentioned and in compliance with existing documentation, the SysTick should have the lowest priority possible.

The extract you mentioned cannot be understood correctly without the preceding key sentence which conditions such practice: Care must be taken if HAL_Delay() is called from a peripheral ISR process.

The complete paragraph is the one below: https://github.com/STMicroelectronics/STM32CubeH7/blob/b340b13929e36a3427b8d94e8b1006022f82273f/Drivers/STM32H7xx_HAL_Driver/Src/stm32h7xx_hal.c#L254-L256

I hope this answers your question.

With regards,

escherstair commented 2 years ago

I think now I understood, and so I propose tho change https://github.com/STMicroelectronics/STM32CubeH7/blob/b340b13929e36a3427b8d94e8b1006022f82273f/Drivers/STM32H7xx_HAL_Driver/Src/stm32h7xx_hal.c#L254-L256 into

  *       The Systick must run at the lowest **possible** priority, and the concept of **possible** is crucial.
  *       Care must be taken if HAL_Delay() is called from a peripheral ISR process,
  *       In this case the SysTick interrupt must have higher priority (numerically lower)
  *       than the peripheral interrupt. Otherwise the caller ISR process will be blocked.
  *       And so, the lowest possible priority must be higher than the peripheral interrupt priority.

Do you agree to my proposal?

ALABSTM commented 2 years ago

Hi @escherstair,

Thank you for your proposal. However, we prefer stick to the current one. Indeed, as you already mentioned, the TICK_INT_PRIORITY is by default defined to be the lowest possible.

https://github.com/STMicroelectronics/STM32CubeH7/blob/e9472e471cc8974c4a2596fc86e565241871c20e/Drivers/STM32H7xx_HAL_Driver/Inc/stm32h7xx_hal_conf_template.h#L166

I would say the capitalized 'T' in the middle of the sentence (at line 255) is probably the cause of the confusion. Actually, the word "The" itself reveals to be redundant with the second one.

https://github.com/STMicroelectronics/STM32CubeH7/blob/b340b13929e36a3427b8d94e8b1006022f82273f/Drivers/STM32H7xx_HAL_Driver/Src/stm32h7xx_hal.c#L255

A fix will be published to have this updated. Thank you again for your proposal and thank you for your comprehension.

With regards,

ALABSTM commented 2 years ago

ST Internal Reference: 126437