EGU to SWI occasionally missing flags

[Benjamin]

I have an application built on the 52840 which is using a software interrupt triggered by the event generator unit. Occasionally my SWI function is being passed a zero for the flags value, despite the EGU being triggered by one of the task addresses.

More specifically, I have 4 timers each triggering GPIOTE tasks on 4 compare events via 16 PPI channels. For each timer, on one of the compare events, I have the PPI fork task set to an EGU trigger corresponding to the timer (all on EGU0, since they all call the same function). The SWI has a dedicated IRQ priority.

Everything works great except for the occasional hard fault if I don't explicitly check for the flags value being non-zero. My timers are running at 16MHz, but the SWI is only triggered every few milliseconds on each channel. I'm wondering if maybe IRQ collisions from the different EGU triggers are causing the flags value to somehow get 'lost'.

This is the setup code:

static nrfx_swi_t swi;

static nrfx_gpiote_out_config_t gpiote_config = NRFX_GPIOTE_CONFIG_OUT_TASK_TOGGLE(false);

static void gate_init(void) {
    nrfx_gpiote_init();

    uint32_t pin;
    uint32_t task_addr;
    uint32_t evt_addr;
    nrf_ppi_channel_t ppi_channel;

#define SET_GATE_PPI \
APP_ERROR_CHECK(nrfx_ppi_channel_alloc(&ppi_channel)); \
APP_ERROR_CHECK(nrfx_ppi_channel_assign(ppi_channel, evt_addr, task_addr)); \
APP_ERROR_CHECK(nrfx_ppi_channel_enable(ppi_channel))

    // Configure Software Interrupt to call update_pulses
    APP_ERROR_CHECK(nrfx_swi_alloc(&swi, update_pulses, PULSE_UPDATES_IRQ_PRIORITY));

    for (uint8_t channel = 0; channel < DEVICE_CHANNEL_COUNT; channel++) {        egu_task_addrs[channel] = nrfx_swi_task_trigger_address_get(swi, channel);
        // Gate A pin
        pin = gate_pins[channel][0];
        APP_ERROR_CHECK(nrfx_gpiote_out_init(pin, &gpiote_config));
        task_addr = nrfx_gpiote_out_task_addr_get(pin);
        // Gate A rising edge
        evt_addr = nrfx_timer_compare_event_address_get(&timers[channel], 0);
        SET_GATE_PPI;
        ppi_channels[channel][0] = ppi_channel;
        // Gate A falling edge
        evt_addr = nrfx_timer_compare_event_address_get(&timers[channel], 1);
        SET_GATE_PPI;
        ppi_channels[channel][1] = ppi_channel;
        // Enable gate
        nrfx_gpiote_out_task_enable(pin);

        // Gate B pin
        pin = gate_pins[channel][1];
        APP_ERROR_CHECK(nrfx_gpiote_out_init(pin, &gpiote_config));
        task_addr = nrfx_gpiote_out_task_addr_get(pin);
        // Gate B rising edge
        evt_addr = nrfx_timer_compare_event_address_get(&timers[channel], 2);
        SET_GATE_PPI;
        ppi_channels[channel][2] = ppi_channel;
        // Gate B falling edge
        evt_addr = nrfx_timer_compare_event_address_get(&timers[channel], 3);
        SET_GATE_PPI;
        ppi_channels[channel][3] = ppi_channel;
        // Enable gate
        nrfx_gpiote_out_task_enable(pin);
    }
}

And this is the SWI function:

static uint32_t volatile no_flags_count = 0;

static void update_pulses(uint8_t __unused _swi, uint16_t flags) {
    if (!flags) {
        no_flags_count++;
        return;
    }
    // https://stackoverflow.com/a/18050458/161366
    uint8_t channel = 31 - __builtin_clz(flags);
    
    /* Update timer compares */
}

When I check no_flags_count in a debugger it's always non-zero.

[bjorn-spockeli]

Hi Benjamin, 

please note that in the worst case there might be up-to two 16MHz clock cycles delay from an event to the task being triggered, see PPI — Programmable peripheral interconnect

On each PPI channel, the signals are synchronized to the 16 MHz clock, to avoid any internal violation of setup and hold timings. As a consequence, events that are synchronous to the 16 MHz clock will be delayed by one clock period, while other asynchronous events will be delayed by up to one 16 MHz clock period.

Also, If I remember correctly the ARM Cortex M4 has a interrupt entry/exit  latency of 12 cycles( the M4 is running at 64MHz). So without accounting for tail chained interrupts, the latency of entering and exiting the ISR is 24 cycles, so the maximum frequency one would be able to enter the ISR would be 2.66 MHz if my math is right. If we take into account tail chained interrupts the latency is 18 cycles, resulting in a maximum frequency of 3.55 MHz. 

And then we're not taking into account the code running in the ISR, which needs to be added. So it could be that this all adds up to something in the ms range.

Best regards

Bjørn

[Benjamin]

At ~2 MHz, that's on the order of microseconds, not milliseconds.

[bjorn-spockeli]

Hi Benjamin, 

Apologies for the very late reply. 

So if I understand the issue correct, the SWI function is not called as often as you are expecting and the flag value passed with the SWI function is often zero?

Could you try to replace the SWI trigger task with a GPIOTE OUT task that toggles a GPIO? Im just thinking that it would be nice to verify that the COMPARE events are triggering as often as you are expecting them to trigger. 

Best regards

Bjørn

[Benjamin]

I dug into this more (and also forgot to update the ticket) and i'm pretty sure what happens is that sometimes a 2nd (or more) SWI IRQ fires while the SWI ISR is running, and when the follow-up ISR is run, the flags value is passed in as 0.

I don't see any missing flags when I only have 1 'channel' enabled, and the frequency of misses seems to increase with the number of channels enabled.

[bjorn-spockeli]

Hi Benjamin, 

OK, so the SWI ISR is triggered and executed as often as you are expecting, but if one or more timers trigger the SWI while the SWI ISR is running, then the user flag passed with the pending interrupts are zero. 

Looking at the nrfx_swi.c code, it does seem feasible that the 

void nrfx_swi_trigger(nrfx_swi_t swi, uint8_t flag_number)
{
    NRFX_ASSERT(swi_is_allocated(swi));

#if NRFX_SWI_EGU_COUNT

    NRF_EGU_Type * p_egu = nrfx_swi_egu_instance_get(swi);
#if (NRFX_SWI_EGU_COUNT < SWI_COUNT)
    if (p_egu == NULL)
    {
        m_swi_flags[swi - NRFX_SWI_EGU_COUNT] |= (1 << flag_number);
        NRFX_IRQ_PENDING_SET(swi_irq_number_get(swi));
    }
    else
#endif // (NRFX_SWI_EGU_COUNT < SWI_COUNT)
    {
        nrf_egu_task_trigger(p_egu,
            nrf_egu_task_trigger_get(p_egu, flag_number));
    }

#else // -> #if !NRFX_SWI_EGU_COUNT

    m_swi_flags[swi - NRFX_SWI_EGU_COUNT] |= (1 << flag_number);
    NRFX_IRQ_PENDING_SET(swi_irq_number_get(swi));

#endif
}

static void swi_irq_handler(nrfx_swi_t swi)
{
#if (NRFX_SWI_FIRST > 0)
    NRFX_ASSERT(swi >= NRFX_SWI_FIRST);
#endif
    NRFX_ASSERT(swi <= NRFX_SWI_LAST);
    nrfx_swi_handler_t handler = m_swi_handlers[swi];
    NRFX_ASSERT(handler != NULL);

    nrfx_swi_flags_t flags = m_swi_flags[swi - NRFX_SWI_EGU_COUNT];
    m_swi_flags[swi - NRFX_SWI_EGU_COUNT] &= ~flags;

    handler(swi, flags);
}

When you're triggering the SWI through PPI and not with the nrfx_swi_trigger() function from SW, then I do not think that the m_swi_flags variable is set. So when the swi_irq_handler is executed, then m_swi_flags could zero. Do you agree?

-Bjørn