Attention is currently required from: Furquan Shaikh, Angel Pons, Subrata Banik, Aaron Durbin. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51445 )
Change subject: timestamp: Add helper fucntions ......................................................................
Patch Set 3:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/51445/comment/a9d7a87a_75c3518c PS3, Line 7: Add helper fucntions
Actually replace ts[X] with time_stamp_X. Sorry, I used the wrong name from the follow-up CL.
I think the time_from_base in your naming is confusing, because all timestamp_add()s are relative to the base. The real difference in the one you propose is just that you're using a different scale (microseconds instead of TSC ticks).
I still think this might all be easier if we first change the timestamp library to do everything in microseconds. Then you just need a timestamp_rewind_base(0 - ts[3]) with one argument, and then you can timestamp_add() all the other ones directly (which are already in microseconds to the same new base).
File src/include/timestamp.h:
https://review.coreboot.org/c/coreboot/+/51445/comment/cf36defb_7fb72d93 PS3, Line 26: * Add a new raw timestamp in microseconds into timestamp_table at index 'n'.
IIUC, the reason why x86 platforms store in TSC frequency is because the timestamp values are basica […]
Yeah I know, but that's one division instruction, it shouldn't really be noticeable. That's the same thing that happens any time you call a stopwatch function, and all the Arm platforms do that too. I feel like we're getting so far into the woods with all these frequencies (especially with all the new APIs you're proposing) that it would really be a lot easier to just force everything to be in microseconds.
-
Julius Werner (Code Review)