Martin Roth 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 39:
(4 comments)
https://review.coreboot.org/c/coreboot/+/42859/39/util/amdfwtool/amdfwtool-b... File util/amdfwtool/amdfwtool-b.c:
https://review.coreboot.org/c/coreboot/+/42859/39/util/amdfwtool/amdfwtool-b... PS39, Line 243: 0x2 Nit: some indentation issues, which seems odd to me since I didn't see them in the other copy of the file. /shrug
https://review.coreboot.org/c/coreboot/+/42859/39/util/amdfwtool/amdfwtool.c File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/42859/39/util/amdfwtool/amdfwtool.c... PS39, Line 1086: // Unused values: D Could we keep the list of unused values - we still have quite a long list of arguments.
https://review.coreboot.org/c/coreboot/+/42859/39/util/amdfwtool/amdfwtool.c... PS39, Line 156: printf("-x | --xhci <FILE> Add XHCI blob\n"); : printf("-i | --imc <FILE> Add IMC blob\n"); : printf("-g | --gec <FILE> Add GEC blob\n"); Any reason not to move more of these options to the config file?
https://review.coreboot.org/c/coreboot/+/42859/39/util/amdfwtool/amdfwtool.c... PS39, Line 1456: exit(1); Can we add a way to generated a config file with no values? How would someone know currently what the options to add to the config file are?