Felix Held 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 25:
(5 comments)
the cfg files for stoneyridge are probably still missing in this patch; those should also be inside the coreboot tree and not in one of the blobs repos, since those aren't blobs
https://review.coreboot.org/c/coreboot/+/42859/25/src/soc/amd/picasso/fw.cfg File src/soc/amd/picasso/fw.cfg:
https://review.coreboot.org/c/coreboot/+/42859/25/src/soc/amd/picasso/fw.cfg... PS25, Line 2: # Move to amd_blob remove; where it's now is where it probably should be located
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
https://review.coreboot.org/c/coreboot/+/42859/25/src/southbridge/amd/pi/hud... File src/southbridge/amd/pi/hudson/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/42859/25/src/southbridge/amd/pi/hud... PS25, Line 208: #cbfs-files-y += apu/amdfw same here
https://review.coreboot.org/c/coreboot/+/42859/25/util/amdfwtool/amdfwtool.h File util/amdfwtool/amdfwtool.h:
https://review.coreboot.org/c/coreboot/+/42859/25/util/amdfwtool/amdfwtool.h... PS25, Line 4: //#include "config.h" remove
https://review.coreboot.org/c/coreboot/+/42859/25/util/amdfwtool/amdfwtool.c File util/amdfwtool/amdfwtool.c:
https://review.coreboot.org/c/coreboot/+/42859/25/util/amdfwtool/amdfwtool.c... PS25, Line 1339: // case 'b': please remove lines instead of commenting them out. what's removed will still be in the git history, so there's no downside to just remove those lines