GD32F130 GPIO EXTI4-15 interrupt handler is slow, would be nice to be able to override it

[robcazzaro]

I'm working to help implement a SimpleFOC algorithm for GD32F130-based hoverboards, and running into timing issues with interrupt handling. The GD32F130C8 used on those boards runs at only 48MHz, so every instruction counts when responding to an interrupt.

The current code in this repository is suboptimal in two areas especially when using pins with a high number (e.g. PC14)

void exti_callbackHandler(uint32_t pinNum)
{
    exti_line_enum linex = (exti_line_enum)BIT(pinNum);
    if (RESET != exti_interrupt_flag_get(linex)) {
        exti_interrupt_flag_clear(linex);
        if (NULL != gpio_exti_infor[pinNum].callback) {
            gpio_exti_infor[pinNum].callback();
        }
    }
}

[...]
#elif defined(GD32F3x0) || defined(GD32F1x0)
void EXTI0_1_IRQHandler(void)
{
    uint32_t i;
    for (i = 0; i <= 1; i++) {
        exti_callbackHandler(i);
    }
}

void EXTI2_3_IRQHandler(void)
{
    uint32_t i;
    for (i = 2; i <= 3; i++) {
        exti_callbackHandler(i);
    }
}

void EXTI4_15_IRQHandler(void)
{
    uint32_t i;
    for (i = 4; i <= 15; i++) {
        exti_callbackHandler(i);
    }
}

EXTI4_15_IRQHandler() calls exti_callbackHandler 11 times before finally servicing PC14 and invoking the relevant callback, plus one more time after the callback. As a generic implementation, that works well. But considering in our project we use PC14 and PB11 for Hall sensors, that adds a non-trivial amount of overhead.

Knowing we have only 2 possible EXTI sources, our code could be as follows

void EXTI4_15_IRQHandler(void)
{
    exti_callbackHandler(11);
    exti_callbackHandler(14);
}

Would it be possible to define the EXTI IRQ handlers with __attribute__((weak)) so that they can be overridden by our implementation? I would like to build as much as possible on top of the un-modified GD32 Arduino core. Not sure if that would conflict with the .weak EXTI4_15_IRQHandler definition in startup_gd32f1x0.S, though. I found this warning "If multiple weak definitions are available, the linker generates an error message, unless the linker option --muldefweak is used. In this case, the linker chooses one for use by all calls.", so I'm not sure how that would work. Maybe a global define to enable IRQ handler redefinition?

[maxgerhardt]

Well the first thing we can do is make the generic implementation faster. Instead of indiscrimentely calling into exti_callbackHandler(i); for i = 4 to 15 in some cases, we can first capture the value of which exact interrupt(s) actually fired by reading the full register is read internally by exti_interrupt_flag_get(linex)). Looping over every bit of a 32-bit value (or even 16 bit value) will be faster than doing all these calls. Of course that will have to be verified by actual latency measurements.

Additionally we can mark the EXTI handler implementations in the core all as weak, so even if there's 2 implementations (one weak from our core and one from the user), the user one's will be preferred.

[robcazzaro]

Thanks! Just a quick note on the weak implementation: EXTI4_15_IRQHandler is already defined as weak here or here (I'm never sure which one is actually used)

I don't know if the linker complains if there are only 2 definitions (one is startup.s, one in gpio_interrupt.c), both weak. Once I add my definition, it will be ok for me, but the average user is unlikely to redefine it, so both definitions are weak and it's unclear to me which one the linker will choose. I'm sure you understand this all much better than I do, though 😉