-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix double low power ticker interrupt #7599
Fix double low power ticker interrupt #7599
Conversation
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.
/morph build |
Build : SUCCESSBuild number : 2789 Triggering tests/morph test |
Exporter Build : SUCCESSBuild number : 2419 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To check I understand this correctly:
The schedule_interrupt tick conversion rounded down, that rounded-down tick was in the past, so it scheduled an immediate interrupt. That interrupt triggered, but ticker_irq_handler decided it wasn't time yet, so rescheduled. Repeat, spinning interrupts, until time is reached? Seems like it could be more than just double, potentially?
If that's right, then approved.
Slightly wary of it going in a patch release unless it's causing a real observed problem - vaguely possible someone's timing is somehow relying on that spin to get more accurate scheduling - it was working a bit like wait_ms
.
@c1728p9 Mind ack/nack-ing kjbracey-arm's interpretation? |
@kjbracey-arm We're close enough to the feature release that we could target it to 5.10 without too much issue. @c1728p9 any objections? |
Test : FAILUREBuild number : 2524 |
This failed as it failed to fetch the test app from AWS
|
/morph test |
Hi @kjbracey-arm yeah, that is correctly. In the setup I was using it was firing twice, but for faster devices it could fire many more times. @cmonr, this going into 5.10 sounds good to me. |
Test : FAILUREBuild number : 2530 |
Hmmm, not liking the trend in silly test failures... |
Test : FAILUREBuild number : 2555 |
Failures don't appear to be related to CI. |
Test : SUCCESSBuild number : 2568 |
…interrupt Fix double low power ticker interrupt
Description
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.
Pull request type