Bao Zheng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42859 )
Change subject: amdfwtool: Take a config file instead of command line parameters ......................................................................
Patch Set 64:
(4 comments)
https://review.coreboot.org/c/coreboot/+/42859/63/util/amdfwtool/amdfwtool.c File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/42859/63/util/amdfwtool/amdfwtool.c... PS63, Line 1468: "Config file is parsed error: %s\n",
This is not clear/proper English. […]
Done
https://review.coreboot.org/c/coreboot/+/42859/63/util/amdfwtool/amdfwtool.c... PS63, Line 1469: strerror(errno))
Calling strerror() here would be bad as proces_config() returns 0 for failure and 1 for success. […]
Done
https://review.coreboot.org/c/coreboot/+/42859/63/util/amdfwtool/data_parse.... File util/amdfwtool/data_parse.c:
https://review.coreboot.org/c/coreboot/+/42859/63/util/amdfwtool/data_parse.... PS63, Line 208: fw_type = AMD_FW_PSP_WHITELIST;
I would not add whitelist file unless it is whitelist configuration. […]
Done. Removed. Actually the white list file has been removed from the configuration file and gone back to command options.
https://review.coreboot.org/c/coreboot/+/42859/63/util/amdfwtool/data_parse.... PS63, Line 399: return 0; /* Stop parsing. */
It would be nice to print which line it choked on - something like: printf("AMDFWTOOL config file li […]
Done. And if no line with FIRMWARE_LOCATION, also stop parsing.