Attention is currently required from: Martin Roth, Duncan Laurie, Tim Wawrzynczak, Patrick Rudolph. Francois Toguo Fotso has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49943 )
Change subject: soc/intel/tigerlake: Add CrashLog implementation for intel TGL. ......................................................................
Patch Set 5:
(10 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/49943/comment/f9614bd6_b38d2fe6 PS3, Line 7: This change implements CrashLog for intel TGL.
soc/intel/tigerlake: Add CrashLog reporting
Done
Patchset:
PS5: Patrick, your comments are implemented.
File src/soc/intel/tigerlake/acpi.c:
https://review.coreboot.org/c/coreboot/+/49943/comment/c9a003d5_8a895cc4 PS3, Line 417: printk(BIOS_DEBUG, "Crashlog is disabled, Skipping it.\n");
Don't think we need this?
Done
https://review.coreboot.org/c/coreboot/+/49943/comment/8966dca4_aa3f7bb9 PS3, Line 422: printk(BIOS_DEBUG, "Crashlog discovery result: crashlog not found\n");
maybe make this BIOS_SPEW? From looking at the rest of the commit, having no crashlog seems perfectl […]
Done
https://review.coreboot.org/c/coreboot/+/49943/comment/d10d77bf_441ae1da PS3, Line 428: dicovery
"discovery". […]
Done
File src/soc/intel/tigerlake/crashlog_lib.c:
https://review.coreboot.org/c/coreboot/+/49943/comment/efb0e458_1224b8ae PS3, Line 80:
extra whitespace (also at least 4 times more in this file)
Done
https://review.coreboot.org/c/coreboot/+/49943/comment/e9ab1442_9a644f72 PS3, Line 379:
extraneous space
Done
https://review.coreboot.org/c/coreboot/+/49943/comment/7c92fcd2_dae8cc90 PS3, Line 428: *
we usually leave a space between /* and the comment.
Done
File src/soc/intel/tigerlake/include/soc/crashlog_def.h:
https://review.coreboot.org/c/coreboot/+/49943/comment/de669193_eeaffac3 PS3, Line 1: GPL-2.0-only
The license text below says "or any later version". […]
Done
https://review.coreboot.org/c/coreboot/+/49943/comment/ab81f950_7360914f PS3, Line 30: #define BIT_31_SET 0x80000000
some places across the code base call this BIT31, and we also have a BIT(x) macro in types.h. […]
Done