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 27:
(3 comments)
https://review.coreboot.org/c/coreboot/+/42859/19/src/soc/amd/picasso/Kconfi... File src/soc/amd/picasso/Kconfig:
https://review.coreboot.org/c/coreboot/+/42859/19/src/soc/amd/picasso/Kconfi... PS19, Line 428: #config AMD_PUBKEY_FILE : # string : # default "3rdparty/amd_blobs/picasso/PSP/AmdPubKeyRV.bin"
please don't just comment out things; remove instead
Removed.
https://review.coreboot.org/c/coreboot/+/42859/25/src/soc/amd/stoneyridge/Ma... File src/soc/amd/stoneyridge/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/42859/25/src/soc/amd/stoneyridge/Ma... PS25, Line 273: #cbfs-files-y += apu/amdfw
this looks wrong to me
This is for splitting the patch into smaller ones. Cause the change has not apply to the Stoney, the Jenkins would report error. Otherwise all the changes should be in one big patch.
https://review.coreboot.org/c/coreboot/+/42859/23/util/amdfwtool/data_parse.... File util/amdfwtool/data_parse.c:
https://review.coreboot.org/c/coreboot/+/42859/23/util/amdfwtool/data_parse.... PS23, Line 535: if (strcmp(&(oneline[match[1].rm_so]), "FIRMWARE_LOCATE") == 0) {
braces {} are not necessary for single statement blocks
add a word below and braces are kept.