Ticker interrupt not scheduled when interrupt has been set in the past

[mprse]

Description

• Type: Bug/Question
• Priority: Major

---

Bug

Target
ALL

Toolchain:
ALL

Description:
While working with PR #6052 I debug the ticker interrupt scheduling process on K64F board and found that interrupt is not fired when interrupt has been scheduled in the past. Please see the following part of code:

static void schedule_interrupt(const ticker_data_t *const ticker)
{
    ticker_event_queue_t *queue = ticker->queue;
    update_present_time(ticker);

    if (ticker->queue->head) {
        us_timestamp_t present = ticker->queue->present_time;
        us_timestamp_t match_time = ticker->queue->head->timestamp;

        // if the event at the head of the queue is in the past then schedule
        // it immediately.
        if (match_time <= present) {
            ticker->interface->fire_interrupt();
            return;
        }

        timestamp_t match_tick = compute_tick(ticker, match_time);
        ticker->interface->set_interrupt(match_tick);
        timestamp_t cur_tick = ticker->interface->read();

        if (_ticker_match_interval_passed(queue->tick_last_read, cur_tick, match_tick)) {   <----- ISSUE
            ticker->interface->fire_interrupt();
        }
    } else {
        uint32_t match_tick =
                (queue->tick_last_read + queue->max_delta) & queue->bitmask;
        ticker->interface->set_interrupt(match_tick);
    }
}

Expected behavior

In case when interrupt has been scheduled in the past _ticker_match_interval_passed() function should return true and interrupt should be fired by means of ticker->interface->fire_interrupt();.

I found in my debug log the following case:

• timestamp in ticks passed to set_interrupt() function: 020394758.
• current time in ticks reported by set_interrupt() function: 020394759.
• last tick read (queue->tick_last_read): 020394758.

It looks like this is the case when interrupt has been set in the past (one tick), so _ticker_match_interval_passed() routine should return true and interrupt should be fired by means of ticker->interface->fire_interrupt();.

Actual behavior

Interrupt is not fired by means of ticker->interface->fire_interrupt();.

Analysis

Please see the _ticker_match_interval_passed() function:

int _ticker_match_interval_passed(timestamp_t prev_tick, timestamp_t cur_tick, timestamp_t match_tick)
{
    if (match_tick > prev_tick) {
        return (cur_tick >= match_tick) || (cur_tick < prev_tick);
    } else {
        return (cur_tick < prev_tick) && (cur_tick >= match_tick);
    }
}

In our case:

• cur_tick > match_tick
• match_tick == prev_tick

Result: false

Question

@c1728p9
The logic of this function is quite complicated and I'm not sure if I correctly understand entire flow, but it looks like in mentioned case _ticker_match_interval_passed() function should return true.
Is logic of this function valid?

In our case (cur_tick >= match_tick) condition is fulfilled, so maybe (match_tick > prev_tick) should be changed to (match_tick >= prev_tick)?

[0xc0170]

cc @pan-

[c1728p9]

Hi @mprse good catch in tracking down this bug! I can imagine that this was tricky to reproduce.

The function _ticker_match_interval_passed appears to be working as intended.
You mentioned that it does not fire in this scenario:

prev_tick = 020394758
cur_tick = 020394759
match_tick = 020394758

The ticker value changing from 020394758 to 020394759 should only fire a match interrupt for the value 020394759 according to the ticker specification.

I think the root cause of this is the rounding that take place when converting from microseconds to ticks, particularly when the ticker is running slower than microseconds. Two separate microsecond time values could reside within the same tick. The would bypass the code which checks if the microsecond match time is now or in the past allowing set_interrupt to be called with a value guaranteed not to fire at the right time.

To fix this extra logic needs to be added to handle this special case. Something like this:

        timestamp_t match_tick = compute_tick(ticker, match_time);
+       if (match_tick == queue->tick_last_read) {
+           // Match time has already expired so fire immediately
+           ticker->interface->fire_interrupt();
+           return;
+       }
        ticker->interface->set_interrupt(match_tick);
        timestamp_t cur_tick = ticker->interface->read();

[mprse]

@c1728p9 thanks for clarification. I didn't mentioned it straight forward, but you are right that this issue is related to conversion from microseconds to ticks. It can be detected when defined ticker frequency is not 1 MHz (in our case 32 KHz).

The issue has been detected by tests-mbed_drivers-lp_ticker test ( Test multi ticker test case) while working on lp ticker K64F driver.

I checked your solution and it gives positive results. I'll test it and create a patch.
Thanks.

[mprse]

This issue is solved and can be closed.