drivers: apic_timer: move to periodic mode #73185
Conversation
zephyrbot
requested review from
andyross,
ceolin,
dcpleung,
edersondisouza,
najumon1980 and
teburd
npitre
force-pushed
the
apictimer
branch
2 times, most recently
from
c80a9f8 to
d116953
Compare
drivers/timer/apic_timer.c
Outdated
There was a problem hiding this comment.
Might want a BUILD_ASSERT(!defined(CONFIG_TICKLESS_KERNEL)) given that support is being removed, for the benefit of any out-of-tree apps that might be using it.
drivers/timer/apic_timer.c
Outdated
There was a problem hiding this comment.
The warning here about CONFIG_SMP can be removed, as a ticking driver is SMP-safe, or certainly can be made to be so.
drivers/timer/apic_timer.c
Outdated
There was a problem hiding this comment.
Had to stare at this for a while trying to figure out what the synchronization actually was, since obviously the ISR can fire before the value is used anyway. Suggest clarifying here that "total_cycles" is a non-atomic 64 bit read on i386. It's not a data race, it's a multi-word atomicity concern.
Also you can remove the loop on x86_64 (though that's not the target platform for this driver) where the value is atomic.
Let's replicate a common code pattern for this to be abstracted more easily in the future. In addition to duplicating the correctness fixes implemented in the ARM and RISC-V drivers, this eliminates a couple large runtime divisions. Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
Fear no more. The bikeshed has retained all its features with the last push. ;-) |
|
@npitre this patch does enhance apic_tsc.c, look good 👋 |
This adds support for the local APIC in one-shot mode as the timeout event source for those cases where the CPU supports invariant TSC but no TSC deadline capability. It is presented as another timer choice. Existing Kconfig symbols were preserved to minimize board config disturbance. This hybrid approach was implemented kind of backward in the apic_timer driver but it is far cleaner to carry this here. Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
This driver is impossible to make time-accurate using single-shot mode. Time accuracy may be obtained only by using periodic mode, meaning it is not tickless capable either. Let's simplify the code by only supporting periodic mode and strip out the TSC stuff. Any hardware with TSC capability should now use the apic-tsc driver instead. Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
This driver is impossible to make time-accurate using single-shot
mode. Time accuracy may be obtained only by using periodic mode, meaning
it is not tickless capable either. Let's simplify the code by only
supporting periodic mode and strip out the TSC stuff. Any hardware with
TSC capability should use the apic_tsc driver instead.
There is apparently some hardware out there with a TSC but without the
ability to set a deadline comparator. In such case it would be much
cleaner to add the ability to use the APIC ICR in one-shot mode as a
fallback IRQ source to the apic_tsc driver instead of relying on the
apic_timer driver.