Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35280 )
Change subject: timestamps: Refactor CBMEM hook ......................................................................
Patch Set 5: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/35280/5/src/lib/timestamp.c File src/lib/timestamp.c:
https://review.coreboot.org/c/coreboot/+/35280/5/src/lib/timestamp.c@274 PS5, Line 274: if (CONFIG(ARCH_ROMSTAGE_X86_32) && ENV_ROMSTAGE) nit: There's no real reason to check for x86 here. Currently no non-x86 platforms use CBMEM recovery between boots, but if they did they'd also want to reset timestamps. The whole thing could be simplified to
if (is_recovery && !ENV_ROMSTAGE) { ...find or alloc... } else { ...alloc... }
https://review.coreboot.org/c/coreboot/+/35280/5/src/lib/timestamp.c@283 PS5, Line 283: } else nit: would be nice to fix code style while you're here (needs braces)