Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33621 )
Change subject: soc/amd/stoneyridge: Add merlinfalcon configuration ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/#/c/33621/3/src/soc/amd/stoneyridge/Kconfig File src/soc/amd/stoneyridge/Kconfig:
https://review.coreboot.org/#/c/33621/3/src/soc/amd/stoneyridge/Kconfig@25 PS3, Line 25: : ## Merlinfalcon only supports FP4. Meh, I could do without this comment. It's already in the help text, and you could embellish that if you feel it's necessary.
https://review.coreboot.org/#/c/33621/3/src/soc/amd/stoneyridge/Kconfig@32 PS3, Line 32: config SOC_AMD_PSP_SELECTABLE_SMU_FW Move this item to below CPU_SPECIFIC_OPTIONS somewhere. I'd probably put it just above SOC_AMD_SMU_FANLESS.
https://review.coreboot.org/#/c/33621/3/src/soc/amd/stoneyridge/Kconfig@37 PS3, Line 37: Some PSP implementations allow storing SMU firmware into cbfs and : calling the PSP to load the blobs at the proper time. : : The soc/<codename> should select this if its PSP supports the feature : and each mainboard can choose to select an appropriate fanless or : fanned set of blobs. Ask your AMD representative whether your APU : is considered fanless. You're free to override the default help text with anything you want. If MF or ST is selected, then someone checks the help for this option, I think this might be confusing. You could say that some ST implementations support "storing SMU firmware...". And that MF doesn't support it.
https://review.coreboot.org/#/c/33621/3/src/soc/amd/stoneyridge/Makefile.inc File src/soc/amd/stoneyridge/Makefile.inc:
https://review.coreboot.org/#/c/33621/3/src/soc/amd/stoneyridge/Makefile.inc... PS3, Line 204: endif This starts to get confusing. Instead of the additions in lines 166, 168, 171, 174, 196, 198, 201, 203 see if simply inserting at the following at 205 works for you.
ifeq ("$(wildcard $(SMUFWM_FN_FILE))","") SMUFWM_FN_FILE= SMUFIRMWARE2_FN_FILE= endif
https://review.coreboot.org/#/c/33621/3/src/soc/amd/stoneyridge/Makefile.inc... PS3, Line 225: ifeq ($(CONFIG_SOC_AMD_PSP_SELECTABLE_SMU_FW),y) You probably won't need this. I believe the add_opt_prefix should assign emptiness to the variables if there's no input file.