Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35032 )
Change subject: timestamps: Improve collection for ENV_ROMSTAGE_OR_BEFORE ......................................................................
Patch Set 7:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35032/7/src/include/timestamp.h File src/include/timestamp.h:
https://review.coreboot.org/c/coreboot/+/35032/7/src/include/timestamp.h@26 PS7, Line 26: * to make *one* call per stage, otherwise some timestamps will be lost.
So is this an intentional change in behavior now? Previously this wasn't one call *per stage*, this […]
Not intentional, just confusions after looking into if cbmem_init() is necessary for ramstage and making wrong choices.
https://review.coreboot.org/c/coreboot/+/35032/7/src/lib/timestamp.c File src/lib/timestamp.c:
https://review.coreboot.org/c/coreboot/+/35032/7/src/lib/timestamp.c@124 PS7, Line 124: return car_get_ptr(glob_ts_table);
I think this should keep doing a cbmem_find() and/or a timestamp_cache_get() if the pointer is curre […]
Why would he have cbmem_find() here now, see patchset #3 comment? I thought you analyzed for ramstage/postcar timestamp_add() before cbmem_initialize() does not need to be supported and CBMEM_HOOK would always have done car_set_ptr() already. That was the logic for .bss cache removal.
https://review.coreboot.org/c/coreboot/+/35032/7/src/lib/timestamp.c@172 PS7, Line 172: timestamp_add_table_entry(ts_table, id, ts_time);
Why are we removing the error message? I think that was useful (since this points to a programming p […]
I'll put it back.