Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/23203 )
Change subject: Add support to get layout from fmap (e.g. coreboot rom) ......................................................................
Patch Set 16:
(6 comments)
It seems a little weird to have the bulk of the implementation of fmap-from-rom here while only adding the CLI option in the next patch. Maybe it's a good idea to squash both?
https://review.coreboot.org/#/c/23203/16//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/23203/16//COMMIT_MSG@7 PS16, Line 7: Add support to get layout from fmap (e.g. coreboot rom) Needs to be updated to mention fmap from rom functionality implementation.
https://review.coreboot.org/#/c/23203/16/fmap.h File fmap.h:
https://review.coreboot.org/#/c/23203/16/fmap.h@1 PS16, Line 1: #include <inttypes.h> Probably needs license from cbfstool here
https://review.coreboot.org/#/c/23203/16/fmap.c File fmap.c:
https://review.coreboot.org/#/c/23203/16/fmap.c@1 PS16, Line 1: #include <ctype.h> probably needs licence from cbfstool.
https://review.coreboot.org/#/c/23203/16/layout.c File layout.c:
https://review.coreboot.org/#/c/23203/16/layout.c@22 PS16, Line 22: #include <sys/stat.h> Added headers not needed anymore?
https://review.coreboot.org/#/c/23203/16/layout.c@104 PS16, Line 104: accidental newline?
https://review.coreboot.org/#/c/23203/16/libflashrom.c File libflashrom.c:
https://review.coreboot.org/#/c/23203/16/libflashrom.c@462 PS16, Line 462: offset On Intel targets you probably only want and in some reasons even only can, if for instance ME region is not readable, look into the BIOS region. This again will need lsearch in some cases.
Maybe the start offset and the end of this loop could be made function arguments?