Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39882 )
Change subject: fsp2_0: Clean up around `config FSP_USE_REPO` ......................................................................
fsp2_0: Clean up around `config FSP_USE_REPO`
We can make our lifes much easier by removing its dependency on `ADD_FSP_BINARIES`. Instead, we imply the latter if the repository is to be used. We can also hide a lot of unnecessary prompts in this case.
Also, remove default overrides and selects for the two that are now unnecessary.
Change-Id: I8538f2e966adc9da0fbea2250c954d86e42dfeb3 Signed-off-by: Nico Huber nico.huber@secunet.com --- M Documentation/mainboard/asrock/h110m-dvs.md M configs/config.intel_coffeelake_rvp11.fsp_car M src/drivers/intel/fsp2_0/Kconfig M src/mainboard/libretrend/lt1000/Kconfig M src/mainboard/protectli/vault_kbl/Kconfig M src/mainboard/razer/blade_stealth_kbl/Kconfig M src/mainboard/system76/lemp9/Kconfig M src/mainboard/up/squared/Kconfig M util/abuild/abuild 9 files changed, 18 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/39882/1
diff --git a/Documentation/mainboard/asrock/h110m-dvs.md b/Documentation/mainboard/asrock/h110m-dvs.md index 66d491d..4d26cfd 100644 --- a/Documentation/mainboard/asrock/h110m-dvs.md +++ b/Documentation/mainboard/asrock/h110m-dvs.md @@ -31,8 +31,6 @@ touch .config ./util/scripts/config --enable VENDOR_ASROCK ./util/scripts/config --enable BOARD_ASROCK_H110M_DVS -./util/scripts/config --enable CONFIG_ADD_FSP_BINARIES -./util/scripts/config --enable CONFIG_FSP_USE_REPO ./util/scripts/config --set-str REALTEK_8168_MACADDRESS "xx:xx:xx:xx:xx:xx" make olddefconfig ``` diff --git a/configs/config.intel_coffeelake_rvp11.fsp_car b/configs/config.intel_coffeelake_rvp11.fsp_car index 33192c4..6898217 100644 --- a/configs/config.intel_coffeelake_rvp11.fsp_car +++ b/configs/config.intel_coffeelake_rvp11.fsp_car @@ -2,8 +2,6 @@ CONFIG_VENDOR_INTEL=y CONFIG_INTEL_GMA_VBT_FILE="3rdparty/fsp/CoffeeLakeFspBinPkg/SampleCode/Vbt/Vbt.bin" CONFIG_BOARD_INTEL_COFFEELAKE_RVP11=y -CONFIG_ADD_FSP_BINARIES=y CONFIG_USE_CANNONLAKE_FSP_CAR=y CONFIG_RUN_FSP_GOP=y -CONFIG_FSP_USE_REPO=y CONFIG_PAYLOAD_NONE=y diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index 1e1cc19..b3b99bd 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -31,11 +31,19 @@
if PLATFORM_USES_FSP2_0
-config ADD_FSP_BINARIES - bool "Add Intel FSP 2.0 binaries to CBFS" +config FSP_USE_REPO + bool "Use binaries of the Intel FSP repository on GitHub" + depends on HAVE_INTEL_FSP_REPO + default y help - Add the FSP-M and FSP-S binaries to CBFS. Currently coreboot does not - use the FSP-T binary and it is not added. + When selecting this option, the SoC must set FSP_HEADER_PATH + and FSP_FD_PATH correctly so FSP splitting works. + +config ADD_FSP_BINARIES + bool "Add Intel FSP 2.0 binaries to CBFS" if !FSP_USE_REPO + default y if FSP_USE_REPO + help + Add the FSP-M and FSP-S binaries to CBFS.
config FSP_T_CBFS string "Name of FSP-T in CBFS" @@ -50,31 +58,23 @@ string "Name of FSP-M in CBFS" default "fspm.bin"
-config FSP_USE_REPO - bool "Use the IntelFSP based binaries" - depends on ADD_FSP_BINARIES - depends on HAVE_INTEL_FSP_REPO - default y - help - When selecting this option, the SoC must set FSP_HEADER_PATH - and FSP_FD_PATH correctly so FSP splitting works. - config FSP_T_FILE - string "Intel FSP-T (temp RAM init) binary path and filename" + string "Intel FSP-T (temp RAM init) binary path and filename" if !FSP_USE_REPO + depends on ADD_FSP_BINARIES depends on FSP_CAR default "$(obj)/Fsp_T.fd" if FSP_USE_REPO help - The path and filename of the Intel FSP-M binary for this platform. + The path and filename of the Intel FSP-T binary for this platform.
config FSP_M_FILE - string "Intel FSP-M (memory init) binary path and filename" + string "Intel FSP-M (memory init) binary path and filename" if !FSP_USE_REPO depends on ADD_FSP_BINARIES default "$(obj)/Fsp_M.fd" if FSP_USE_REPO help The path and filename of the Intel FSP-M binary for this platform.
config FSP_S_FILE - string "Intel FSP-S (silicon init) binary path and filename" + string "Intel FSP-S (silicon init) binary path and filename" if !FSP_USE_REPO depends on ADD_FSP_BINARIES default "$(obj)/Fsp_S.fd" if FSP_USE_REPO help diff --git a/src/mainboard/libretrend/lt1000/Kconfig b/src/mainboard/libretrend/lt1000/Kconfig index b4a4e49..9c4223a 100644 --- a/src/mainboard/libretrend/lt1000/Kconfig +++ b/src/mainboard/libretrend/lt1000/Kconfig @@ -44,12 +44,4 @@ hex default 0x600000
-config ADD_FSP_BINARIES - bool - default y - -config FSP_USE_REPO - bool - default y - endif diff --git a/src/mainboard/protectli/vault_kbl/Kconfig b/src/mainboard/protectli/vault_kbl/Kconfig index bfafc0b..8c09a60 100644 --- a/src/mainboard/protectli/vault_kbl/Kconfig +++ b/src/mainboard/protectli/vault_kbl/Kconfig @@ -48,12 +48,4 @@ hex default 0x600000
-config ADD_FSP_BINARIES - bool - default y - -config FSP_USE_REPO - bool - default y - endif diff --git a/src/mainboard/razer/blade_stealth_kbl/Kconfig b/src/mainboard/razer/blade_stealth_kbl/Kconfig index 903d7ba..532bd76 100644 --- a/src/mainboard/razer/blade_stealth_kbl/Kconfig +++ b/src/mainboard/razer/blade_stealth_kbl/Kconfig @@ -14,8 +14,6 @@ select DRIVERS_I2C_HID select HAVE_ACPI_RESUME select HAVE_ACPI_TABLES - select ADD_FSP_BINARIES - select FSP_USE_REPO
# For now no way to choose the correct the available RAM config BOARD_RAZER_BLADE_STEALTH_KBL_16GB diff --git a/src/mainboard/system76/lemp9/Kconfig b/src/mainboard/system76/lemp9/Kconfig index a612503..d814633 100644 --- a/src/mainboard/system76/lemp9/Kconfig +++ b/src/mainboard/system76/lemp9/Kconfig @@ -2,7 +2,6 @@
config BOARD_SPECIFIC_OPTIONS def_bool y - select ADD_FSP_BINARIES select BOARD_ROMSIZE_KB_16384 select EC_ACPI select HAVE_ACPI_RESUME diff --git a/src/mainboard/up/squared/Kconfig b/src/mainboard/up/squared/Kconfig index 5db76fd..542e8c3 100644 --- a/src/mainboard/up/squared/Kconfig +++ b/src/mainboard/up/squared/Kconfig @@ -3,8 +3,6 @@ config BOARD_SPECIFIC_OPTIONS def_bool y select USE_BLOBS - select ADD_FSP_BINARIES - select FSP_USE_REPO select HAVE_ACPI_TABLES select HAVE_ACPI_RESUME select INTEL_GMA_HAVE_VBT diff --git a/util/abuild/abuild b/util/abuild/abuild index 9688c8c..f55dadc 100755 --- a/util/abuild/abuild +++ b/util/abuild/abuild @@ -720,7 +720,7 @@ shift;; -B|--blobs) shift customizing="${customizing}, blobs" - configoptions="${configoptions}CONFIG_USE_BLOBS=y\nCONFIG_USE_AMD_BLOBS=y\nCONFIG_ADD_FSP_BINARIES=y\nCONFIG_FSP_USE_REPO=y\n" + configoptions="${configoptions}CONFIG_USE_BLOBS=y\nCONFIG_USE_AMD_BLOBS=y\nCONFIG_ADD_FSP_BINARIES=y\n" ;; -A|--any-toolchain) shift customizing="${customizing}, any-toolchain"
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39882 )
Change subject: fsp2_0: Clean up around `config FSP_USE_REPO` ......................................................................
Patch Set 1: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39882 )
Change subject: fsp2_0: Clean up around `config FSP_USE_REPO` ......................................................................
Patch Set 1: Code-Review+1
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39882 )
Change subject: fsp2_0: Clean up around `config FSP_USE_REPO` ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39882/2/src/drivers/intel/fsp2_0/Kc... File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/39882/2/src/drivers/intel/fsp2_0/Kc... PS2, Line 34: FSP_USE_REPO : bool "Use binaries of the Intel FSP repository on GitHub" : depends on HAVE_INTEL_FSP_REPO : default y only place these defaults don't seem to work properly is for GeminiLake, since it inherits from Apollolake but can't use the APL FSP repo
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39882 )
Change subject: fsp2_0: Clean up around `config FSP_USE_REPO` ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39882/2/src/drivers/intel/fsp2_0/Kc... File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/39882/2/src/drivers/intel/fsp2_0/Kc... PS2, Line 34: FSP_USE_REPO : bool "Use binaries of the Intel FSP repository on GitHub" : depends on HAVE_INTEL_FSP_REPO : default y
only place these defaults don't seem to work properly is for GeminiLake, since it inherits from Apol […]
Oh, good catch. That slipt through in the previous commit, it shouldn't select HAVE_INTEL_FSP_REPO.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39882 )
Change subject: fsp2_0: Clean up around `config FSP_USE_REPO` ......................................................................
Patch Set 3: Code-Review+2
looks good now that GLK doesn't select HAVE_FSP_REPO
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39882 )
Change subject: fsp2_0: Clean up around `config FSP_USE_REPO` ......................................................................
Patch Set 4: Code-Review+2
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39882 )
Change subject: fsp2_0: Clean up around `config FSP_USE_REPO` ......................................................................
Patch Set 4: Code-Review+2
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39882 )
Change subject: fsp2_0: Clean up around `config FSP_USE_REPO` ......................................................................
fsp2_0: Clean up around `config FSP_USE_REPO`
We can make our lifes much easier by removing its dependency on `ADD_FSP_BINARIES`. Instead, we imply the latter if the repository is to be used. We can also hide a lot of unnecessary prompts in this case.
Also, remove default overrides and selects for the two that are now unnecessary.
Change-Id: I8538f2e966adc9da0fbea2250c954d86e42dfeb3 Signed-off-by: Nico Huber nico.huber@secunet.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39882 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Felix Singer felixsinger@posteo.net Reviewed-by: Matt DeVillier matt.devillier@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M Documentation/mainboard/asrock/h110m-dvs.md M configs/config.intel_coffeelake_rvp11.fsp_car M src/drivers/intel/fsp2_0/Kconfig M src/mainboard/libretrend/lt1000/Kconfig M src/mainboard/protectli/vault_kbl/Kconfig M src/mainboard/razer/blade_stealth_kbl/Kconfig M src/mainboard/system76/lemp9/Kconfig M src/mainboard/up/squared/Kconfig M util/abuild/abuild 9 files changed, 18 insertions(+), 43 deletions(-)
Approvals: build bot (Jenkins): Verified Matt DeVillier: Looks good to me, approved Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/Documentation/mainboard/asrock/h110m-dvs.md b/Documentation/mainboard/asrock/h110m-dvs.md index 66d491d..4d26cfd 100644 --- a/Documentation/mainboard/asrock/h110m-dvs.md +++ b/Documentation/mainboard/asrock/h110m-dvs.md @@ -31,8 +31,6 @@ touch .config ./util/scripts/config --enable VENDOR_ASROCK ./util/scripts/config --enable BOARD_ASROCK_H110M_DVS -./util/scripts/config --enable CONFIG_ADD_FSP_BINARIES -./util/scripts/config --enable CONFIG_FSP_USE_REPO ./util/scripts/config --set-str REALTEK_8168_MACADDRESS "xx:xx:xx:xx:xx:xx" make olddefconfig ``` diff --git a/configs/config.intel_coffeelake_rvp11.fsp_car b/configs/config.intel_coffeelake_rvp11.fsp_car index 33192c4..6898217 100644 --- a/configs/config.intel_coffeelake_rvp11.fsp_car +++ b/configs/config.intel_coffeelake_rvp11.fsp_car @@ -2,8 +2,6 @@ CONFIG_VENDOR_INTEL=y CONFIG_INTEL_GMA_VBT_FILE="3rdparty/fsp/CoffeeLakeFspBinPkg/SampleCode/Vbt/Vbt.bin" CONFIG_BOARD_INTEL_COFFEELAKE_RVP11=y -CONFIG_ADD_FSP_BINARIES=y CONFIG_USE_CANNONLAKE_FSP_CAR=y CONFIG_RUN_FSP_GOP=y -CONFIG_FSP_USE_REPO=y CONFIG_PAYLOAD_NONE=y diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index 1e1cc19..b3b99bd 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -31,11 +31,19 @@
if PLATFORM_USES_FSP2_0
-config ADD_FSP_BINARIES - bool "Add Intel FSP 2.0 binaries to CBFS" +config FSP_USE_REPO + bool "Use binaries of the Intel FSP repository on GitHub" + depends on HAVE_INTEL_FSP_REPO + default y help - Add the FSP-M and FSP-S binaries to CBFS. Currently coreboot does not - use the FSP-T binary and it is not added. + When selecting this option, the SoC must set FSP_HEADER_PATH + and FSP_FD_PATH correctly so FSP splitting works. + +config ADD_FSP_BINARIES + bool "Add Intel FSP 2.0 binaries to CBFS" if !FSP_USE_REPO + default y if FSP_USE_REPO + help + Add the FSP-M and FSP-S binaries to CBFS.
config FSP_T_CBFS string "Name of FSP-T in CBFS" @@ -50,31 +58,23 @@ string "Name of FSP-M in CBFS" default "fspm.bin"
-config FSP_USE_REPO - bool "Use the IntelFSP based binaries" - depends on ADD_FSP_BINARIES - depends on HAVE_INTEL_FSP_REPO - default y - help - When selecting this option, the SoC must set FSP_HEADER_PATH - and FSP_FD_PATH correctly so FSP splitting works. - config FSP_T_FILE - string "Intel FSP-T (temp RAM init) binary path and filename" + string "Intel FSP-T (temp RAM init) binary path and filename" if !FSP_USE_REPO + depends on ADD_FSP_BINARIES depends on FSP_CAR default "$(obj)/Fsp_T.fd" if FSP_USE_REPO help - The path and filename of the Intel FSP-M binary for this platform. + The path and filename of the Intel FSP-T binary for this platform.
config FSP_M_FILE - string "Intel FSP-M (memory init) binary path and filename" + string "Intel FSP-M (memory init) binary path and filename" if !FSP_USE_REPO depends on ADD_FSP_BINARIES default "$(obj)/Fsp_M.fd" if FSP_USE_REPO help The path and filename of the Intel FSP-M binary for this platform.
config FSP_S_FILE - string "Intel FSP-S (silicon init) binary path and filename" + string "Intel FSP-S (silicon init) binary path and filename" if !FSP_USE_REPO depends on ADD_FSP_BINARIES default "$(obj)/Fsp_S.fd" if FSP_USE_REPO help diff --git a/src/mainboard/libretrend/lt1000/Kconfig b/src/mainboard/libretrend/lt1000/Kconfig index b4a4e49..9c4223a 100644 --- a/src/mainboard/libretrend/lt1000/Kconfig +++ b/src/mainboard/libretrend/lt1000/Kconfig @@ -44,12 +44,4 @@ hex default 0x600000
-config ADD_FSP_BINARIES - bool - default y - -config FSP_USE_REPO - bool - default y - endif diff --git a/src/mainboard/protectli/vault_kbl/Kconfig b/src/mainboard/protectli/vault_kbl/Kconfig index bfafc0b..8c09a60 100644 --- a/src/mainboard/protectli/vault_kbl/Kconfig +++ b/src/mainboard/protectli/vault_kbl/Kconfig @@ -48,12 +48,4 @@ hex default 0x600000
-config ADD_FSP_BINARIES - bool - default y - -config FSP_USE_REPO - bool - default y - endif diff --git a/src/mainboard/razer/blade_stealth_kbl/Kconfig b/src/mainboard/razer/blade_stealth_kbl/Kconfig index 903d7ba..532bd76 100644 --- a/src/mainboard/razer/blade_stealth_kbl/Kconfig +++ b/src/mainboard/razer/blade_stealth_kbl/Kconfig @@ -14,8 +14,6 @@ select DRIVERS_I2C_HID select HAVE_ACPI_RESUME select HAVE_ACPI_TABLES - select ADD_FSP_BINARIES - select FSP_USE_REPO
# For now no way to choose the correct the available RAM config BOARD_RAZER_BLADE_STEALTH_KBL_16GB diff --git a/src/mainboard/system76/lemp9/Kconfig b/src/mainboard/system76/lemp9/Kconfig index a612503..d814633 100644 --- a/src/mainboard/system76/lemp9/Kconfig +++ b/src/mainboard/system76/lemp9/Kconfig @@ -2,7 +2,6 @@
config BOARD_SPECIFIC_OPTIONS def_bool y - select ADD_FSP_BINARIES select BOARD_ROMSIZE_KB_16384 select EC_ACPI select HAVE_ACPI_RESUME diff --git a/src/mainboard/up/squared/Kconfig b/src/mainboard/up/squared/Kconfig index 5db76fd..542e8c3 100644 --- a/src/mainboard/up/squared/Kconfig +++ b/src/mainboard/up/squared/Kconfig @@ -3,8 +3,6 @@ config BOARD_SPECIFIC_OPTIONS def_bool y select USE_BLOBS - select ADD_FSP_BINARIES - select FSP_USE_REPO select HAVE_ACPI_TABLES select HAVE_ACPI_RESUME select INTEL_GMA_HAVE_VBT diff --git a/util/abuild/abuild b/util/abuild/abuild index 9688c8c..f55dadc 100755 --- a/util/abuild/abuild +++ b/util/abuild/abuild @@ -720,7 +720,7 @@ shift;; -B|--blobs) shift customizing="${customizing}, blobs" - configoptions="${configoptions}CONFIG_USE_BLOBS=y\nCONFIG_USE_AMD_BLOBS=y\nCONFIG_ADD_FSP_BINARIES=y\nCONFIG_FSP_USE_REPO=y\n" + configoptions="${configoptions}CONFIG_USE_BLOBS=y\nCONFIG_USE_AMD_BLOBS=y\nCONFIG_ADD_FSP_BINARIES=y\n" ;; -A|--any-toolchain) shift customizing="${customizing}, any-toolchain"
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39882 )
Change subject: fsp2_0: Clean up around `config FSP_USE_REPO` ......................................................................
Patch Set 5:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/2103 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2102 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2101
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
Felix Singer (Code Review)
-
Matt DeVillier (Code Review)
-
Nico Huber (Code Review)