Attention is currently required from: Tim Wawrzynczak, Paul Menzel. Ricardo Quesada has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56406 )
Change subject: util/elogtool: add tool to print elog events ......................................................................
Patch Set 6:
(18 comments)
Patchset:
PS6: Thanks for the review. Fixed.
could you PTALA? ty!
File util/cbfstool/elogtool.c:
https://review.coreboot.org/c/coreboot/+/56406/comment/1931bac2_81c5fa6a PS4, Line 54:
from
Not sure I got the comment, but I noticed a typo: EW_ELOG -> RW_ELOG
and added more info the error message.
https://review.coreboot.org/c/coreboot/+/56406/comment/d117ad0d_5fa82468 PS4, Line 64: return 0;
nit: blank line after `}`
Done
https://review.coreboot.org/c/coreboot/+/56406/comment/9235a9d2_e828a41b PS4, Line 72: int
nit: `unsigned int`
Done
https://review.coreboot.org/c/coreboot/+/56406/comment/9a91f6a3_c0bd3f5a PS4, Line 141: input_file = optarg;
nit: blank line after `}`
Done
File util/cbfstool/eventlog.c:
https://review.coreboot.org/c/coreboot/+/56406/comment/1b293d80_5f7d9ee6 PS4, Line 48: /* make certain _all_ members of tm get initialized */
is this comment required?
probably not, fixed.
https://review.coreboot.org/c/coreboot/+/56406/comment/9efbc9b0_6d9e77e9 PS4, Line 190: uint8_t
`const uint8_t`
Done
https://review.coreboot.org/c/coreboot/+/56406/comment/46bd1cac_08352639 PS4, Line 203: (extra >> 8) & 0xff, (extra >> 3) & 0x1f, : (extra & 0x3));
suggestion: […]
Done
https://review.coreboot.org/c/coreboot/+/56406/comment/7d0bfeb3_a28799fd PS4, Line 208: 8
PATH_I2C_MODE10BIT_SHIFT
Done
https://review.coreboot.org/c/coreboot/+/56406/comment/8edadbd4_8b11f006 PS4, Line 208: 0xff
PATH_I2C_MODE10BIT_MASK
Done
https://review.coreboot.org/c/coreboot/+/56406/comment/191a6a4a_77cfa148 PS4, Line 208: 0xff
PATH_I2C_ADDRESS_MASK
Done
https://review.coreboot.org/c/coreboot/+/56406/comment/5299b4b6_e8b128b6 PS4, Line 451: {POST_PRE_HARDWAREMAIN, "Before Hardware Main"}, : {POST_ENTRY_HARDWAREMAIN, "First call in Hardware Main"}, : {POST_CONSOLE_READY, "Console is ready"}, I sorted the entries by value of #define (easy to find missing entries)
https://review.coreboot.org/c/coreboot/+/56406/comment/2d56be85_9427525a PS4, Line 589: return 0;
nit: `break;`
good catch.
https://review.coreboot.org/c/coreboot/+/56406/comment/08040bfa_a242ef7b PS4, Line 602: /* print the timestamp */
is this comment required?
Done
https://review.coreboot.org/c/coreboot/+/56406/comment/7e9274b4_8689ff37 PS4, Line 605: /* print event type */
is this comment required?
Done
https://review.coreboot.org/c/coreboot/+/56406/comment/b442f639_c4af9db1 PS4, Line 608: /* print custom data */
is this comment required?
Done
File util/cbfstool/eventlog.c:
https://review.coreboot.org/c/coreboot/+/56406/comment/08415d79_f08e3e08 PS6, Line 189: ELOG_DEV_PATH_TYPE_NONE, "None"}, : {ELOG_DEV_PATH_TYPE_ROOT, "Root"}, added missing entries
https://review.coreboot.org/c/coreboot/+/56406/comment/d9520b1c_9d10b54a PS6, Line 498: {POST_FSP_MULTI_PHASE_SI_INIT_ENTRY, "FSP-S Init Enter"}, : {POST_FSP_MULTI_PHASE_SI_INIT_EXIT, "FPS-S Init Exit"}, : {POST_FSP_NOTIFY_AFTER_ENUMERATE, "FSP Notify After Enumerate"}, : {POST_FSP_NOTIFY_AFTER_FINALIZE, "FSP Notify After Finalize"}, : {POST_INVALID_ROM, "Invalid ROM"}, : {POST_INVALID_CBFS, "Invalid CBFS"}, : {POST_INVALID_VENDOR_BINARY, "Invalid Vendor Binary"}, : {POST_RAM_FAILURE, "RAM Failure"}, : {POST_HW_INIT_FAILURE, "Hardware Init Failure"}, : {POST_VIDEO_FAILURE, "Video Failure"}, : {POST_TPM_FAILURE, "TPM Failure"}, : {POST_DEAD_CODE, "Dead Code"}, : {POST_RESUME_FAILURE, "Resume Failure"}, : {POST_JUMPING_TO_PAYLOAD, "Before Jump to Payload"}, : {POST_ENTER_ELF_BOOT, "Before ELF Boot"}, : {POST_OS_RESUME, "Before OS Resume"}, : {POST_OS_BOOT, "Before OS Boot"}, : {POST_DIE, "coreboot Dead"}, added missing entries