On some platforms, if low power ticker interrupt is set to very close value (e.g. timestamp < current tick + 3), then interrupt may not fire. This is one use case of lp ticker wrapper, but not all platforms use the wrapper. Some platforms cheat a bit and in this case, simply schedules interrupt a bit later. The problem has been found while working on the low-level lp ticker wrapper for ST boards which run lp ticker using LPTIM. These platforms have such limitation.
Failing test: tests-mbed_drivers-lp_timeout (Test Case: Zero delay)
In the test scenarion, the lp ticker callback is attached with 0.0 s delay in the loop. The new events are put in the front of the lp ticker event list and interrupt reschedule is performed. Usually, the new event is already expired, interrupt fires immediately and next event from the list is then scheduled (e.g. system tick). When the next event (e.g. system tick) is very close to the current time it might be scheduled a bit later (because of lp ticker limitation). Let's assume that system tick has been delayed by 3 ticks and while inserting new zero delay event, absolute system tick time on the event list has already expired. In this case, zero delay event may be added after the expired system tick event and no reschedule is performed (because the head of the list has not changed). Interrupt also didn't fire yet since it has been delayed, so after return from attach_callback(0) we are still waiting for the delayed interrupt and zero delay callback has not been called instantly.
This may also affect other platforms which use such delays (Cypress, NORDIC, etc.).
The proposition is to add extra condition while adding an event to the event list. If the inserted event is already expired, then perform reschedule immediately.
Wait until dispatching is finished before scheduling the next ticker
interrupt. This prevents unnecissary calls to set_interrupt from
periodic elements being added back.
This is particularly useful for the low power ticker on devices with
LPTICKER_DELAY_TICKS set to a non-zero value. This is because the low
power ticker cannot be reschduled immediately and needs to fall back
onto the microsecond ticker which temporarily locks deep sleep.
When computing the next set_interrupt time in the common ticker layer
the absolute time in microseconds is rounded down to the closes low
power tick. Because of this the low power ticker interrupt fires one
cycle too early. This causes ticker_irq_handler to run even though
there are no events ready to run.
To prevent this unnecessary interrupt this patch changes the
computation for the next set_interrupt time to round up rather than
down.
When ticker is not driven by the 1 MHz clock and HAL driver need to perform conversion between microseconds and ticks, then the interrupt might be scheduled in the past. For details see: https://github.com/ARMmbed/mbed-os/issues/6054.
This patch provides fix for such case. Interrupt is fired immidiatelly when last read tick is equal to the calculated tick when interrupt should be generated.
Allow tickers to specify their native frequency and number of bits.
This allows the conversion to happen in common code rather than in
each vendor's implementation.
fire_interrupt function should be used for events in the past. As we have now
64bit timestamp, we can figure out what is in the past, and ask a target to invoke
an interrupt immediately. The previous attemps in the target HAL tickers were not ideal, as it can wrap around easily (16 or 32 bit counters). This new
functionality should solve this problem.
set_interrupt for tickers in HAL code should not handle anything but the next match interrupt. If it was in the past is handled by the upper layer.
It is possible that we are setting next event to the close future, so once it is set it is already in the past. Therefore we add a check after set interrupt to verify it is in future.
If it is not, we fire interrupt immediately. This results in
two events - first one immediate, correct one. The second one might be scheduled in far future (almost entire ticker range),
that should be discarded.
The specification for the fire_interrupts are:
- should set pending bit for the ticker interrupt (as soon as possible),
the event we are scheduling is already in the past, and we do not want to skip
any events
- no arguments are provided, neither return value, not needed
- ticker should be initialized prior calling this function (no need to check if it is already initialized)
All our targets provide this new functionality, removing old misleading if (timestamp is in the past) checks.
ticker_insert_event() can crash on KLXX (and probably other platforms) if an event is inserted with a timestamp before the current real time.
The problem is easy to trigger: you just need to set up a Ticker object, and then disable interrupts for slightly longer than the Ticker object's interval. It's generally bad practice to disable interrupts for too long, but there are some cases where it's unavoidable, and anyway it would be better for the core library function not to crash. The case where I had an unavoidably long interrupts-off interval was writing flash with the FTFA. The FTFA hardware prohibits flash reads while an FTFA command is in progress, so interrupts must be disabled for the whole duration of each command to ensure that there are no instruction fetches from flash-resident ISRs in the course of the execution. An FTFA "erase sector" command takes a fairly long time (milliseconds), and I have a fairly high frequency Ticker (1ms).
The problem and the fix are pretty straightforward. ticker_insert_event() searches the linked list to figure out where to insert the new event, looking for a spot earlier than any event currently queued. If the event is in the past, it'll usually end up at the head of the list. When the routine sees that the new event belongs at the head of the list, it calls data->interface->set_interrupt() to schedule the interrupt for the event, since it's the new soonest event. The KLXX version of us_ticker_set_interrupt() then looks to see if the event is in the past, which we've stipulated that it is, so rather than actually setting the interrupt, it simply calls the handler directly. The first thing the Ticker interrupt handler does is re-schedule itself, so we re-enter ticker_insert_event() at this point. This is where the problem comes in: we didn't finish updating the linked list before we called set_interrupt() and thus before we recursed back into ticker_insert_event(). We set the head of the list to the new event but we didn't set the new event's 'next' pointer.
The fix is simply to finish updating the list before we call set_interrupt(), which we can do by moving the obj->next initialization ahead of the head pointer update.
Przemyslaw Stekiel
Thomas Kemmer
Martin Kojtal
Deepika
Cruz Monrreal
Russ Butler
Marc Emmers
Martin Kojtal
Vincent Coubard
mjrgh
Christopher Haster
Sam Grove