Mbed-os: STM32L4: Incorrect GPIO Interrupts When Using MBED_TICKLESS

[mattbrown015]

Description

I believe I may have hit the same problem on STM32L4 as previously seen, and fixed, for EFM32.

See InterruptIn not working with EFM32 and tickless #6205 and EFM32: make gpio interrupts faster by offloading expected pin state check to user #6315.

I'm using a 500 us low going pulse to generate an interrupt on a NUCLEO_L433RC_P. In other words I have a InterruptIn with a fall handler.

If I define MBED_TICKLESS the fall handler is no longer called.

I believe the interrupt is waking the STM32L4 but InterruptIn::_irq_handler attempts to call the rise handler (which isn't defined) and not the fall handler.

If I extend the pulse to 3 ms the fall handler gets called but I think a rise handler would also be called.

I shall carry on investigating and see if I can create a similar fix to that seen for the EFM32.

Issue request type

[ ] Question
[ ] Enhancement
[X] Bug

[mattbrown015]

All STM should have tickless mode enabled #7153

[ciarmcom]

ARM Internal Ref: MBOTRIAGE-1208

[cmonr]

@mattbrown015 Unsure about your comment, but are you saying that this is a duplicate of https://github.com/ARMmbed/mbed-os/issues/7153?

[mattbrown015]

Sorry, no, I didn't mean that.

I just meant my problem is in some way related to making it possible to use MBED_TICKLESS on all STM32 targets and I had seen that being discussed somewhere. In other words, you wouldn't want to add MBED_TICKLESS to all STM32 targets and stop everyone's interrupts from working.

Of course I might be wrong or it might only be a problem for some STM32 variants or this might already be in hand.

[0xc0170]

@ARMmbed/team-st-mcd Please review

[mattbrown015]

This change allowed my quick and simplistic test to work:

diff --git a/targets/TARGET_STM/gpio_irq_api.c b/targets/TARGET_STM/gpio_irq_api.c
index 522ee9403..0758a4d09 100644
--- a/targets/TARGET_STM/gpio_irq_api.c
+++ b/targets/TARGET_STM/gpio_irq_api.c
@@ -94,12 +94,25 @@ static void handle_interrupt_in(uint32_t irq_index, uint32_t max_num_pin_line)
                     continue;
                 }

-                // Check which edge has generated the irq
-                if ((gpio->IDR & pin) == 0) {
-                    irq_handler(gpio_channel->channel_ids[gpio_idx], IRQ_FALL);
+                // trying to discern which GPIO IRQ we got
+                gpio_irq_event event = IRQ_NONE;
+                if (LL_EXTI_IsEnabledFallingTrig_0_31(pin) && !LL_EXTI_IsEnabledRisingTrig_0_31(pin)) {
+                    // Only the fall handler is active, so this must be a falling edge
+                    event = IRQ_FALL;
+                } else if (LL_EXTI_IsEnabledRisingTrig_0_31(pin) && !LL_EXTI_IsEnabledFallingTrig_0_31(pin))             
{
+                    // Only the rise handler is active, so this must be a rising edge
+                    event = IRQ_RISE;
                 } else {
-                    irq_handler(gpio_channel->channel_ids[gpio_idx], IRQ_RISE);
+                    // Ambiguous as to which IRQ we've received.
+                    if ((gpio->IDR & pin) == 0) {
+                        event = IRQ_FALL;
+                    } else {
+                        event = IRQ_RISE;
+                    }
                 }
+
+                irq_handler(gpio_channel->channel_ids[gpio_idx], event);
+
                 return;
             }
         }

The intend was to copy what was done in EFM32: make gpio interrupts faster by offloading expected pin state check to user #6315.

[LMESTM]

@mattbrown015 thanks a lot for your early investigations with TICKLESS and interrupts.
I think you proposal makes a lot of sense - would you mind creating a PR with your proposal ?
If you send the PR we will run a complete non-regression tests over STM32 targets to check this doesn't have side effects.
Thanks again !

[mattbrown015]

would you mind creating a PR with your proposal ?

@LMESTM no I wouldn't mind but I haven't done a PR before so there might be quicker approaches! :-)

[LMESTM]

@0xc0170 I'm sure Martin can help you about raising your first PR :-)

@mattbrown015 Let me try to quickly summarize what needs to be done:
1) create a fork of mbed-os in your git repo (I see you already have done this)
2) locally clone this to your computer (git clone https://github.com/mattbrown015/mbed-os.git)
3) create a git branch (git checkout -b fix_stm32_gpio_irq_deepsleep)
4) Make the proposed changes and commit them with some good title and comments that describe what you've done
5) Push this branch to your github account repo of mbed-os (git push origin fix_stm32_gpio_irq_deepsleep)
6) Press the Pull Request button from your github account
7) we'll take over from there for testing but the code changes will be tagged with your name on it ;-)

If any of those steps is troublesome to you, we may do it of course ... just let us know

[LMESTM]

would you mind creating a PR with your proposal ?

@LMESTM no I wouldn't mind but I haven't done a PR before so there might be quicker approaches! :-)

The longer part is about reviewing and testing, not about the PR.
@jeromecoutant will help for the testing part anyway

[0xc0170]

@mattbrown015 Let us know if you need any help. As you shared diff above, I assume you are familiar with git?