Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32381 )
Change subject: soc/intel/braswell: add default option to use public FSP ......................................................................
Patch Set 6: Code-Review+2
(3 comments)
Looks good. Left some inline comments how to reduce the (pre-existing) Kconfig noise.
https://review.coreboot.org/#/c/32381/6/src/drivers/intel/fsp1_1/Kconfig File src/drivers/intel/fsp1_1/Kconfig:
https://review.coreboot.org/#/c/32381/6/src/drivers/intel/fsp1_1/Kconfig@52 PS6, Line 52: depends on HAVE_FSP_BIN How about `select` instead? Then this would be enabled by default for systems that support it and automatically build tested.
https://review.coreboot.org/#/c/32381/6/src/drivers/intel/fsp1_1/Kconfig@58 PS6, Line 58: string "Intel FSP binary path and filename" IIRC, we can hide the prompt conditionally, something like
string prompt "Intel FSP binary path and filename" if !FSP_USE_REPO
https://review.coreboot.org/#/c/32381/6/src/mainboard/google/cyan/variants/c... File src/mainboard/google/cyan/variants/celes/ramstage.c:
https://review.coreboot.org/#/c/32381/6/src/mainboard/google/cyan/variants/c... PS6, Line 23: //BSW D-stepping PERPORTRXISET 2 (low strength) NB. Now that I read the comment I wonder if the upstream FSP does it automatically.