cancel
Showing results for 
Search instead for 
Did you mean: 

STM32G0 HAL EXTI callback implementation is buggy.

Takemasa
Associate III
  • CubeIDE v1.1.0
  • STM32Cube FW_G0 v1.3.0

The STM32G0 HAL EXTI call back implementation is wrong. According to the UM2319 rev 1 Section 3.11.2, it say :

the user must call HAL_GPIO_EXTI_IRQHandler() from stm32g0xx_it.c and implement HAL_GPIO_EXTI_Callback()

The STM32G0 HAL doesn't follow this flow, while all other series of HAL ( ex: F0, F4, F7, G4, L1 ) HAL follows this flow.

In the actual code, the STM32G0 HAL driver implements the EXTI interrupt handler as following in the stm32g0xx_hal_gpio.c :

/**
  * @brief  Handle EXTI interrupt request.
  * @param  GPIO_Pin Specifies the port pin connected to corresponding EXTI line.
  * @retval None
  */
void HAL_GPIO_EXTI_IRQHandler(uint16_t GPIO_Pin)
{
  /* EXTI line interrupt detected */
  if (__HAL_GPIO_EXTI_GET_RISING_IT(GPIO_Pin) != 0x00u)
  {
    __HAL_GPIO_EXTI_CLEAR_RISING_IT(GPIO_Pin);
    HAL_GPIO_EXTI_Rising_Callback(GPIO_Pin);
  }
 
  if (__HAL_GPIO_EXTI_GET_FALLING_IT(GPIO_Pin) != 0x00u)
  {
    __HAL_GPIO_EXTI_CLEAR_FALLING_IT(GPIO_Pin);
    HAL_GPIO_EXTI_Falling_Callback(GPIO_Pin);
  }
}
 
/**
  * @brief  EXTI line detection callback.
  * @param  GPIO_Pin Specifies the port pin connected to corresponding EXTI line.
  * @retval None
  */
__weak void HAL_GPIO_EXTI_Rising_Callback(uint16_t GPIO_Pin)
{
  /* Prevent unused argument(s) compilation warning */
  UNUSED(GPIO_Pin);
 
  /* NOTE: This function should not be modified, when the callback is needed,
           the HAL_GPIO_EXTI_Rising_Callback could be implemented in the user file
   */
}
 
/**
  * @brief  EXTI line detection callback.
  * @param  GPIO_Pin Specifies the port pin connected to corresponding EXTI line.
  * @retval None
  */
__weak void HAL_GPIO_EXTI_Falling_Callback(uint16_t GPIO_Pin)
{
  /* Prevent unused argument(s) compilation warning */
  UNUSED(GPIO_Pin);
 
  /* NOTE: This function should not be modified, when the callback is needed,
           the HAL_GPIO_EXTI_Falling_Callback could be implemented in the user file
   */
}

On the other hand, the STM32F0 HAL driver implements the EXTI interrupt handler as following in the stm32f0xx_hal_gpio.c :

/**
  * @brief  Handle EXTI interrupt request.
  * @param  GPIO_Pin Specifies the port pin connected to corresponding EXTI line.
  * @retval None
  */
void HAL_GPIO_EXTI_IRQHandler(uint16_t GPIO_Pin)
{
  /* EXTI line interrupt detected */
  if(__HAL_GPIO_EXTI_GET_IT(GPIO_Pin) != 0x00u)
  { 
    __HAL_GPIO_EXTI_CLEAR_IT(GPIO_Pin);
    HAL_GPIO_EXTI_Callback(GPIO_Pin);
  }
}
 
/**
  * @brief  EXTI line detection callback.
  * @param  GPIO_Pin Specifies the port pin connected to corresponding EXTI line.
  * @retval None
  */
__weak void HAL_GPIO_EXTI_Callback(uint16_t GPIO_Pin)
{
  /* Prevent unused argument(s) compilation warning */
  UNUSED(GPIO_Pin);
 
  /* NOTE: This function should not be modified, when the callback is needed,
            the HAL_GPIO_EXTI_Callback could be implemented in the user file
   */ 
}

The STM32G0 HAL splits the EXTI into two type ( rising and falling ). As a result, it lost the compatibility to the other series code. The worst thing is, it looses compatibility silently . Because the callback is declared as weak, as a nature, it doesn't print any error while the user code is dangling.

Please fix this issue. HAL stands for Hardware Abstraction Layer. This API implementation spoil the "abstraction" of HAL.

Regards,

Takemasa

4 REPLIES 4
berendi
Principal

This API implementation spoil the "abstraction" of HAL.

Was there any kind of abstraction in HAL to begin with? :o

Sometimes there are incompatible changes between HAL versions for the same MCU, while code using direct register accesses works unchanged across different series more often than not.

EXTI functionality works exactly the same way on all STM32 MCUs I know of, only a few registers are renamed on the G0. You'd had achieved better cross-series compatibility by staying away from HAL.

Overall true, but for G0 they really have added separate rising/falling edge interrupt pending registers.

Piranha
Chief II

Well, the G0 hardware has a functionality that wasn't present on previous series and is not represented by HAL for previous series. What do you want them to do - to put it into hardware and hide with HAL? There are many more incompatible things between series even in HAL. Even Windows, macOS, Linux, BSD etc. doesn't have absolutely universal abstraction layer...

The HAL is stupid by design. They have implemented one callback function per event type passing peripheral instance in parameter, but flexible and useful design needs exactly the opposite - one callback function per peripheral instance passing events in parameter. EXTI in this regard is especially stupid - you have to figure out in your code, which EXTI line you are processing, pass it to HAL IRQ handler and then in that single callback again figure out for which EXTI line it has been called. And now in HAL they have not only EXTI functionality in GPIO driver, but also a separate EXTI driver, but the most ironic part is that the latter is as stupid as the former one.

And the EXTI peripheral in STM32 is also badly designed. It shouldn't be as a separate peripheral at all. It should be integrated in GPIO, ports should be 32-bit wide and it should detect not only edge but also a state conditions. NXP (LPC) has done it properly...

Hi

>> Well, the G0 hardware has a functionality that wasn't present on previous series and is not represented by HAL for previous series. What do you want them to do

Very simple. "Keep existing API for existing functionality. Do not delete." If extended functionality is needed, add new API which maintains not only new device but also functionality. It is not difficult. We are discussing the ST HAL for ST on-chip-peripheral. Not Windows HAL by Microsoft for third party hardware.