Attention is currently required from: Paul Menzel, Julius Werner. Mattias Nissler has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62639 )
Change subject: util/cbmem: add an option to append timestamp ......................................................................
Patch Set 4:
(4 comments)
File util/cbmem/cbmem.c:
https://review.coreboot.org/c/coreboot/+/62639/comment/b4589820_6d85d251 PS3, Line 32: #if defined(__x86_64__)
Can we make this […]
Done
https://review.coreboot.org/c/coreboot/+/62639/comment/0f96777f_2ffc2a29 PS3, Line 467: return tsc_freq_khz / 1000;
I'm not sure this is correct, tbh. […]
I did try this code out on volteer/brya platforms, and it actually produces a frequency that is incorrect, i.e. not the TSC frequency, and that seemed wrong with respect to the firmware side code. So this change was really a best effort to improve things. I guess ancient coreboot wouldn't be affected given that the TSC_INFO table entry was introduced in 2016, but you're correct that there's a risk of breaking things, so let me back this out.
https://review.coreboot.org/c/coreboot/+/62639/comment/f8170dd0_eb0007c8 PS3, Line 557: #if defined(__x86_64__)
here too
Done
https://review.coreboot.org/c/coreboot/+/62639/comment/4306585a_a222dc04 PS3, Line 583: if (!strcmp(timestamp_ids[i].name, name))
How do you expect this to be used... […]
Ah, that's much better, thanks for the suggestion! Done.
-
Mattias Nissler (Code Review)