STM32CubeH7 icon indicating copy to clipboard operation
STM32CubeH7 copied to clipboard

SysTick interrupt priority confusion

Open escherstair opened this issue 3 years ago • 5 comments

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?

escherstair avatar Jan 17 '22 14:01 escherstair

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 avatar Jan 30 '22 08:01 ALABSTM

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,

ALABSTM avatar Apr 12 '22 09:04 ALABSTM

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?

escherstair avatar Apr 12 '22 11:04 escherstair

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 avatar Apr 14 '22 09:04 ALABSTM

ST Internal Reference: 126437

ALABSTM avatar Apr 14 '22 09:04 ALABSTM