Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31524 )
Change subject: timestamp: Move timestamp_should_run() call ......................................................................
Patch Set 2:
Patch Set 2: Code-Review+1
Patch Set 2:
(1 comment)
Patch Set 1: Code-Review+1
LGTM but I'm not really familiar with the need for timestamp_should_run().
That implementation of get_us_since_boot() should not depend on COLLECT_TIMESTAMPS. Maybe it could be somehere in TSC/APIC/MONOTONIC_TIMER instead?
That seems somewhat difficult... I don't think you can provide that (at least implemented the way it is now) without pulling in most of the timestamp code anyway.
I just feel it's mixing "an optional debug feature" with the need for actual time reference monotonic timers can provide without the collection. But it's not the scope here anyways.
This function was implemented here because all the information needed are available here. In general it is not important to me haveing it here, I just need the functionality it provides. Adding it here was the proper way for me.
Yes. It's just that it was never well defined when basetime is stamped in relation to power-on and what happens with early soft resets. I think AMD's mostly make the first stamp in romstage. The boards you use may vary between bootblock and romstage, and if there are ME / FSP resets the relation to realtime is vague.
Timer code could use some TLC anyways, there is a bit of mess on udelay() and TSC at least.