Improper EXTI reset in HAL_GPIO_DeInit()

[jelledevleeschouwer]

Dear @ALABSTM,

TL;DR

The HAL_GPIO_DeInit() contains a bug related to the order in which GPIO configuration and EXTI configuration is reset to its "reset defaults". The issue seems not to be present in other HAL drivers, such as for example in stm32f4xx_hal_driver. In those HAL drivers, disabling the EXTI interrupt comes before resetting the GPIO configuration as it should be.

Scenario

Let's assume you have an active low switch (switch with external pull-up) connected to a GPIO pin and one would like to trigger an EXTI interrupt on the falling edge when closing that switch. But, in the meantime (since number of GPIO pins is limited), that GPIO is shared with another hardware resource. In rest, the GPIO is configured for that latter/second resource. One would typically see the following scenario to switch to input with EXTI interrupt on falling edge:

1. Reconfigure the GPIO for EXTI with GPIO_MODE_IT_FALLING and all boiler plate code using GPIO_Init().
2. Wait for the EXTI interrupt to fire or timeout
3. Restore GPIO configuration to map on that second hardware resource with GPIO_Init().

Problem

In the scenario above, when that hardware resource is for example an output and the output isn't driven, the EXTI interrupt might fire since a '1'-to-'0' transition is observed on the EXTI line and that EXTI interrupt is still enabled.

Cause

The only way to clear EXTI configuration is by using HAL_GPIO_DeInit()

Solution

Use HAL_GPIO_DeInit() to clear EXTI configuration to reset defaults.

Follow-up problem 1

The solution for the problem causes another problem: in HAL_GPIO_DeInit() the I/O mode is reset to analog high impedance before the EXTI is disabled!

stm32l0xx_hal_driver/Src/stm32l0xx_hal_gpio.c

Lines 311 to 336 in b6be1fc

	GPIOx->MODER |= (GPIO_MODER_MODE0 << (position * 2U));
	/* Configure the default Alternate Function in current IO */
	GPIOx->AFR[position >> 3U] &= ~((uint32_t)0xFU << ((uint32_t)(position & (uint32_t)0x07U) * 4U)) ;
	/* Configure the default value for IO Speed */
	GPIOx->OSPEEDR &= ~(GPIO_OSPEEDER_OSPEED0 << (position * 2U));
	/* Configure the default value IO Output Type */
	GPIOx->OTYPER &= ~(GPIO_OTYPER_OT_0 << position) ;
	/* Deactivate the Pull-up oand Pull-down resistor for the current IO */
	GPIOx->PUPDR &= ~(GPIO_PUPDR_PUPD0 << (position * 2U));
	/*------------------------- EXTI Mode Configuration --------------------*/
	/* Clear the External Interrupt or Event for the current IO */
	tmp = SYSCFG->EXTICR[position >> 2U];
	tmp &= (((uint32_t)0x0FU) << (4U * (position & 0x03U)));
	if(tmp == (GPIO_GET_INDEX(GPIOx) << (4U * (position & 0x03U))))
	{
	tmp = ((uint32_t)0x0FU) << (4U * (position & 0x03U));
	SYSCFG->EXTICR[position >> 2U] &= ~tmp;
	/* Clear EXTI line configuration */
	EXTI->IMR &= ~((uint32_t)iocurrent);

The GPIO analog high impedance configuration disables the internal TTL Schmitt trigger producing a '0' on its ouput (See Figure 26 on page 242 in RM0376). Again, a '1'-to-'0' transition is observed on the EXTI line and that EXTI interrupt is still enabled. The EXTI interrupt will fire again.

Cause

The code block responsible for resetting the I/O mode of the GPIO comes before the code block responsible for resetting the EXTI configuration.

Solution

Swap the two code blocks in HAL_GPIO_DeInit() around.

Follow-up problem 2

There is still another problem in the code block responsible for resetting the EXTI configuration itself: the line multiplexer is reset before the EXTI is masked! This means that somehow the input data on GPIOA is '0', again a '1'-to-'0' transition is observed on the EXTI line and that EXTI interrupt is still enabled. The EXTI interrupt will fire again.

Cause

The code resetting the EXTI line multiplexer:

stm32l0xx_hal_driver/Src/stm32l0xx_hal_gpio.c

Lines 332 to 333 in b6be1fc

	tmp = ((uint32_t)0x0FU) << (4U * (position & 0x03U));
	SYSCFG->EXTICR[position >> 2U] &= ~tmp;

comes before the code resetting the EXTI interrupt masks:

stm32l0xx_hal_driver/Src/stm32l0xx_hal_gpio.c

Lines 335 to 341 in b6be1fc

	/* Clear EXTI line configuration */
	EXTI->IMR &= ~((uint32_t)iocurrent);
	EXTI->EMR &= ~((uint32_t)iocurrent);
	/* Clear Rising Falling edge configuration */
	EXTI->RTSR &= ~((uint32_t)iocurrent);
	EXTI->FTSR &= ~((uint32_t)iocurrent);

Solution

Swap the two code blocks around.

What do you think? Could you please take a look at this?

Thanks,
Best regards,

Jelle De Vleeschouwer

[ALABSTM]

Dear @jelledevleeschouwer,

First of all, allow me to thank you for this detailed report. The accuracy of your analysis is remarkable.

We are already aware of this issue and have fixed it internally. The fix will be available in a future release. Unfortunately we cannot share a date for the moment. We count on your patience and comprehension.

However, I would like to ask you about what you described in the "Follow-up problem 2" section. Could you systematically reproduce the issue while still having the the line multiplexer reset before the EXTI is masked?

Thank you in advance for your answer and thank you once more for this detailed report.

With regards,

[ALABSTM]

ST Internal Reference: 77661

[ALABSTM]

Hi @jelledevleeschouwer,

Any reply from your side about what you described in the "Follow-up problem 2" section?

With regards,

[jelledevleeschouwer]

Hi @ALABSTM,

Yes, at the time I could reproduce the problem described in "Follow-up problem 2", but I don't have the time to try and reproduce the issue right now.

But the idea is simple, no? Imagine you have a GPIO (say, PB6) with external pull-up configured as digital input with EXTI interrupt on a falling edge. In rest, on the PB6 GPIO a high level '1' is present. But let's imagine you call HAL_GPIO_DeInit() now with a fix for the first issue as follows:

            /*------------------------- EXTI Mode Configuration --------------------*/
            /* Clear the External Interrupt or Event for the current IO */

            tmp = SYSCFG->EXTICR[position >> 2U];
            tmp &= (((uint32_t)0x0FU) << (4U * (position & 0x03U)));
            if(tmp == (GPIO_GET_INDEX(GPIOx) << (4U * (position & 0x03U))))
            {
                tmp = ((uint32_t)0x0FU) << (4U * (position & 0x03U));
                SYSCFG->EXTICR[position >> 2U] &= ~tmp;

                /* Clear EXTI line configuration */
                EXTI->IMR &= ~((uint32_t)iocurrent);
                EXTI->EMR &= ~((uint32_t)iocurrent);

                /* Clear Rising Falling edge configuration */
                EXTI->RTSR &= ~((uint32_t)iocurrent);
                EXTI->FTSR &= ~((uint32_t)iocurrent);
            }

            /*------------------------- GPIO Mode Configuration --------------------*/
            /* Configure IO Direction in Input Floting Mode */
            GPIOx->MODER |= (GPIO_MODER_MODE0 << (position * 2U));
            
            /* ... */

The code addresses SYSCFG_EXTICR2 in case of GPIO_PIN_6. tmp will be be 0x0100 when entering the if-statement. (GPIO_GET_INDEX(GPIOx) << (4U * (position & 0x03U))) will also yield 0x0100 which makes the code enter the if-statement. In the if-statement bits EXTI6[3:0] are cleared in SYSCFG_EXTICR2, which makes the PA6 GPIO to be selected as the source input for the EXTI6 external interrupt. At this point, the EXTI line is still not masked!

If at this point, the PA6 GPIO was now configured as Digital Input and a low value '0' is produced on that GPIO, a '1'-to-'0' (falling edge) will be detected by the EXTI "edge detect cricuitry" and thus will set the Pending Interrupt flag on line 6 (PIF6) in the EXTI_PR register.

This is the behavior that I observed. Could that be plausible, or am I missing something?

Thanks,
Best regards,
Jelle

[ALABSTM]

Hi @jelledevleeschouwer,

I hope you are fine. I noticed you attached p-r #2 to this issue. The fix you proposed is already covered by the one published in the frame of v1.10.3. I assume the p-r covers both problems 1 and 2 you described above.

Please allow me then to close this issue. Thank you for your comprehension and thank you again for your detailed and well structured report.

With regards,