Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34802 )
Change subject: ifdtool: Add validate option to ifdtool ......................................................................
Patch Set 6:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34802/6/Documentation/ifdtool/layou... File Documentation/ifdtool/layout.md:
PS6: I think there's a line length limit for .md files, but I am not sure if it's specified anywhere
https://review.coreboot.org/c/coreboot/+/34802/6/Documentation/ifdtool/layou... PS6, Line 9: SI_ Maybe add quotes or backticks to highlight this prefix
https://review.coreboot.org/c/coreboot/+/34802/6/Documentation/ifdtool/layou... PS6, Line 9: silicon initialization Same as SI_
https://review.coreboot.org/c/coreboot/+/34802/6/Documentation/ifdtool/layou... PS6, Line 18: elsewhere I guess this implies the EC firmware is either on the EC itself or on a dedicated flash chip?
https://review.coreboot.org/c/coreboot/+/34802/6/util/ifdtool/ifdtool.c File util/ifdtool/ifdtool.c:
https://review.coreboot.org/c/coreboot/+/34802/6/util/ifdtool/ifdtool.c@1671 PS6, Line 1671: mode_validate) == 0) { Very minor: Looks like this would fit on the previous line without exceeding the 96-character length limit