Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31493 )
Change subject: cbfstool: Add ifittool ......................................................................
Patch Set 9:
(6 comments)
https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c File util/cbfstool/fit.c:
https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@249 PS9, Line 249: int this could just return void since the return code has no meaning
https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@299 PS9, Line 299: mcu_header->total_size I know this is left over from the original, but if this were left out and it just used "? :" then this would fit on one line.
https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@353 PS9, Line 353: The There
https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@354 PS9, Line 354: Type 2 entry I know it is defined in the header, but it would be useful to use FIT_TYPE_BIOS_ACM (etc) in these comments so it was easier to follow without having to remember the numbering.
https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@399 PS9, Line 399: entry->size_reserved = lcp_policy_size; This is overwriting the setting from the previous line, except it is not converting to multiple of 16 bytes which does not seem like what you want.
https://review.coreboot.org/#/c/31493/9/util/cbfstool/fit.c@446 PS9, Line 446: struct microcode_entry *mcu This threw me for a minute trying to find where it was declared, style guide suggests this should be at the top of the function.