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 53:
(3 comments)
https://review.coreboot.org/c/coreboot/+/42859/47/src/soc/amd/picasso/Makefi... File src/soc/amd/picasso/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/42859/47/src/soc/amd/picasso/Makefi... PS47, Line 125: "3rdparty/amd_blobs/picasso/PSP"
I mention this in the other locations as well. […]
Done. FIRMWARE_LOCATE=$(shell grep -e FIRMWARE_LOCATE $(CONFIG_AMDFW_CONFIG_FILE) | awk '{print $$2}') Use command to extract value from config file.
https://review.coreboot.org/c/coreboot/+/42859/47/src/soc/amd/picasso/fw.cfg File src/soc/amd/picasso/fw.cfg:
https://review.coreboot.org/c/coreboot/+/42859/47/src/soc/amd/picasso/fw.cfg... PS47, Line 3: # Should be first line
Why does the location need to be the first line? Doesn't the parser look through the entire file fo […]
Done. Add a dedicate loop to search the FIRMWARE_LOCATION
https://review.coreboot.org/c/coreboot/+/42859/47/src/soc/amd/stoneyridge/Ma... File src/soc/amd/stoneyridge/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/42859/47/src/soc/amd/stoneyridge/Ma... PS47, Line 105: ifeq ($(CONFIG_AMD_APU_STONEYRIDGE),y)
Can we put the directory values into a Kconfig option instead of hardcoding them into the makefile?
Done. Changed.