Change in coreboot[master]: mb/{google,intel}/*: select SOC_INTEL_CSE_LITE_SKU only if CHROMEOS
Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/49059 )
Change subject: mb/{google,intel}/*: select SOC_INTEL_CSE_LITE_SKU only if CHROMEOS ......................................................................
mb/{google,intel}/*: select SOC_INTEL_CSE_LITE_SKU only if CHROMEOS
Selecting SOC_INTEL_CSE_LITE_SKU without conditioning on CHROMEOS force-selects CHROMEOS, per src/soc/intel/common/block/cse/Kconfig.
Conditioning on CHROMEOS allows for non-ChromeOS targets to be built.
Change-Id: I6959f35e1285b2fab7ea1f83a5ccfcb065c12397 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/mainboard/google/dedede/Kconfig M src/mainboard/google/volteer/Kconfig.name M src/mainboard/intel/jasperlake_rvp/Kconfig M src/mainboard/intel/tglrvp/Kconfig 4 files changed, 17 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/49059/1
diff --git a/src/mainboard/google/dedede/Kconfig b/src/mainboard/google/dedede/Kconfig index 94a0a90..6e5650d 100644 --- a/src/mainboard/google/dedede/Kconfig +++ b/src/mainboard/google/dedede/Kconfig @@ -25,7 +25,7 @@ select MAINBOARD_HAS_TPM2 select SOC_INTEL_JASPERLAKE select SOC_INTEL_COMMON_BLOCK_DTT - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS select HAVE_SPD_IN_CBFS if !BOARD_GOOGLE_DEDEDE select DRIVERS_INTEL_MIPI_CAMERA select SOC_INTEL_COMMON_BLOCK_IPU diff --git a/src/mainboard/google/volteer/Kconfig.name b/src/mainboard/google/volteer/Kconfig.name index 8777994..9c79c2e 100644 --- a/src/mainboard/google/volteer/Kconfig.name +++ b/src/mainboard/google/volteer/Kconfig.name @@ -3,24 +3,24 @@ config BOARD_GOOGLE_DELBIN bool "-> Delbin" select BOARD_GOOGLE_BASEBOARD_VOLTEER - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS select DRIVERS_GENESYSLOGIC_GL9755
config BOARD_GOOGLE_ELDRID bool "-> Eldrid" select BOARD_GOOGLE_BASEBOARD_VOLTEER - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS
config BOARD_GOOGLE_HALVOR bool "-> Halvor" select BOARD_GOOGLE_BASEBOARD_VOLTEER - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS select INTEL_CAR_NEM
config BOARD_GOOGLE_LINDAR bool "-> Lindar" select BOARD_GOOGLE_BASEBOARD_VOLTEER - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS select INTEL_CAR_NEM select CHROMEOS_DSM_CALIB select DRIVERS_I2C_RT1011 @@ -28,38 +28,38 @@ config BOARD_GOOGLE_MALEFOR bool "-> Malefor" select BOARD_GOOGLE_BASEBOARD_VOLTEER - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS select INTEL_CAR_NEM
config BOARD_GOOGLE_TERRADOR bool "-> Terrador" select BOARD_GOOGLE_BASEBOARD_VOLTEER - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS
config BOARD_GOOGLE_TODOR bool "-> Todor" select BOARD_GOOGLE_BASEBOARD_VOLTEER - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS select INTEL_CAR_NEM
config BOARD_GOOGLE_TRONDO bool "-> Trondo" select BOARD_GOOGLE_BASEBOARD_VOLTEER - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS select INTEL_CAR_NEM
config BOARD_GOOGLE_VOLTEER bool "-> Volteer" select BOARD_GOOGLE_BASEBOARD_VOLTEER select VARIANT_HAS_MIPI_CAMERA - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS select INTEL_CAR_NEM
config BOARD_GOOGLE_VOLTEER2 bool "-> Volteer2" select BOARD_GOOGLE_BASEBOARD_VOLTEER select VARIANT_HAS_MIPI_CAMERA - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS select DRIVERS_GENESYSLOGIC_GL9755 select DRIVER_I2C_TPM_ACPI
@@ -68,14 +68,14 @@ bool "-> Volteer2_Ti50" select BOARD_GOOGLE_BASEBOARD_VOLTEER select VARIANT_HAS_MIPI_CAMERA - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS select DRIVERS_GENESYSLOGIC_GL9755 select DRIVER_I2C_TPM_ACPI
config BOARD_GOOGLE_VOXEL bool "-> Voxel" select BOARD_GOOGLE_BASEBOARD_VOLTEER - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS
config BOARD_GOOGLE_BOLDAR bool "-> Boldar" @@ -85,13 +85,13 @@ config BOARD_GOOGLE_ELEMI bool "-> Elemi" select BOARD_GOOGLE_BASEBOARD_VOLTEER - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS
config BOARD_GOOGLE_VOEMA bool "-> Voema" select BOARD_GOOGLE_BASEBOARD_VOLTEER select VARIANT_HAS_MIPI_CAMERA - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS
config BOARD_GOOGLE_DROBIT bool "-> Drobit" diff --git a/src/mainboard/intel/jasperlake_rvp/Kconfig b/src/mainboard/intel/jasperlake_rvp/Kconfig index 7bfd158..eb63834 100644 --- a/src/mainboard/intel/jasperlake_rvp/Kconfig +++ b/src/mainboard/intel/jasperlake_rvp/Kconfig @@ -20,7 +20,7 @@ select SOC_INTEL_COMMON_BLOCK_IPU select SOC_INTEL_JASPERLAKE select SOC_INTEL_COMMON_BLOCK_DTT - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS
config MAINBOARD_DIR string diff --git a/src/mainboard/intel/tglrvp/Kconfig b/src/mainboard/intel/tglrvp/Kconfig index 9df542d..24f990c 100644 --- a/src/mainboard/intel/tglrvp/Kconfig +++ b/src/mainboard/intel/tglrvp/Kconfig @@ -20,7 +20,7 @@ select EC_ACPI select PCIEXP_HOTPLUG select HAVE_SPD_IN_CBFS - select SOC_INTEL_CSE_LITE_SKU + select SOC_INTEL_CSE_LITE_SKU if CHROMEOS select MAINBOARD_HAS_TPM2 select MAINBOARD_HAS_SPI_TPM_CR50 select SPI_TPM
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49059 )
Change subject: mb/{google,intel}/*: select SOC_INTEL_CSE_LITE_SKU only if CHROMEOS ......................................................................
Patch Set 1:
I'm not convinced that the dependency on CHROMEOS is correct. Mapped to reality that would imply that one switches the CSE SKU when switching CHROMEOS off?
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49059 )
Change subject: mb/{google,intel}/*: select SOC_INTEL_CSE_LITE_SKU only if CHROMEOS ......................................................................
Patch Set 1:
Patch Set 1:
I'm not convinced that the dependency on CHROMEOS is correct. Mapped to reality that would imply that one switches the CSE SKU when switching CHROMEOS off?
I'm inclined to agree. Looking at cse_lite.c, it would seem there's an implicit dependency on vboot, not necessarily ChromeOS. Dropping 'depends on CHROMEOS' from SOC_INTEL_CSE_LITE_SKU would allow certain boards to build w/o CHROMEOS selected, but not sure that would result in a bootable image
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49059 )
Change subject: mb/{google,intel}/*: select SOC_INTEL_CSE_LITE_SKU only if CHROMEOS ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/49059/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/49059/1//COMMIT_MSG@11 PS1, Line 11: What build error do you get?
Hello V Sowmya, build bot (Jenkins), Nico Huber, Furquan Shaikh, Sridhar Siricilla, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/49059
to look at the new patch set (#2).
Change subject: soc/intel/common/cse: Drop dependency on CHROMEOS for SOC_INTEL_CSE_LITE_SKU ......................................................................
soc/intel/common/cse: Drop dependency on CHROMEOS for SOC_INTEL_CSE_LITE_SKU
The CSE lite SKU has 2 CSE firmware boot partitions vs 3 for the "normal" SKU; this has nothing to do with building for ChromeOS or not, and by having this dependency, boards with select the CSE lite SKU are unable to build with CONFIG_CHROMEOS unset due to Kconfig dependency issues.
Test: build google/wyvern with CONFIG_CHROMEOS not set.
Change-Id: I6959f35e1285b2fab7ea1f83a5ccfcb065c12397 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/mainboard/google/hatch/Kconfig M src/soc/intel/common/block/cse/Kconfig 2 files changed, 1 insertion(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/49059/2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49059 )
Change subject: soc/intel/common/cse: Drop dependency on CHROMEOS for SOC_INTEL_CSE_LITE_SKU ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/49059/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/49059/1//COMMIT_MSG@11 PS1, Line 11:
What build error do you get?
Most failing boards here: https://qa.coreboot.org/job/coreboot-gerrit/157687/ e.g. board.GOOGLE_BOTEN
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49059 )
Change subject: soc/intel/common/cse: Drop dependency on CHROMEOS for SOC_INTEL_CSE_LITE_SKU ......................................................................
Patch Set 2: Code-Review+1
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49059 )
Change subject: soc/intel/common/cse: Drop dependency on CHROMEOS for SOC_INTEL_CSE_LITE_SKU ......................................................................
Patch Set 2:
we chatted a little on IRC, just leaving some comments here for posterity:
the CSE Lite SKU FW is more of an artifact of the shipped firmware, not necessarily the build itself, so once a board ships with CSE lite FW, that's what you're pretty much stuck with; IOW, yes this seems semantically correct to me, the CSE_LITE_SKU shouldn't be dependent on CHROMEOS, per se, from a build perspective
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49059 )
Change subject: soc/intel/common/cse: Drop dependency on CHROMEOS for SOC_INTEL_CSE_LITE_SKU ......................................................................
Patch Set 2: Code-Review+1
Patch Set 2:
we chatted a little on IRC, just leaving some comments here for posterity:
the CSE Lite SKU FW is more of an artifact of the shipped firmware, not necessarily the build itself, so once a board ships with CSE lite FW, that's what you're pretty much stuck with; IOW, yes this seems semantically correct to me, the CSE_LITE_SKU shouldn't be dependent on CHROMEOS, per se, from a build perspective
I think this is fine, but I'd like Furquan to chime in if possible to make sure I'm not missing something
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49059 )
Change subject: soc/intel/common/cse: Drop dependency on CHROMEOS for SOC_INTEL_CSE_LITE_SKU ......................................................................
Patch Set 2: Code-Review+1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49059 )
Change subject: soc/intel/common/cse: Drop dependency on CHROMEOS for SOC_INTEL_CSE_LITE_SKU ......................................................................
Patch Set 2:
Patch Set 2: Code-Review+1
Patch Set 2:
we chatted a little on IRC, just leaving some comments here for posterity:
the CSE Lite SKU FW is more of an artifact of the shipped firmware, not necessarily the build itself, so once a board ships with CSE lite FW, that's what you're pretty much stuck with; IOW, yes this seems semantically correct to me, the CSE_LITE_SKU shouldn't be dependent on CHROMEOS, per se, from a build perspective
I think this is fine, but I'd like Furquan to chime in if possible to make sure I'm not missing something
That makes sense to me. As you captured, CSE Lite FW is an artifact of the shipped firmware. The dependency is more in terms of "Only Chrome OS boards use CSE Lite FW". But, that does not translate to "depends on CHROMEOS" for the build system. So, dropping the build time dependency seems correct.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49059 )
Change subject: soc/intel/common/cse: Drop dependency on CHROMEOS for SOC_INTEL_CSE_LITE_SKU ......................................................................
Patch Set 2: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49059 )
Change subject: soc/intel/common/cse: Drop dependency on CHROMEOS for SOC_INTEL_CSE_LITE_SKU ......................................................................
Patch Set 2: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49059 )
Change subject: soc/intel/common/cse: Drop dependency on CHROMEOS for SOC_INTEL_CSE_LITE_SKU ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/49059/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/49059/1//COMMIT_MSG@11 PS1, Line 11:
Most failing boards here: […]
Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49059 )
Change subject: soc/intel/common/cse: Drop dependency on CHROMEOS for SOC_INTEL_CSE_LITE_SKU ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/49059/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/49059/1//COMMIT_MSG@11 PS1, Line 11:
Done
For the record:
warning: (BOARD_GOOGLE_BASEBOARD_DEDEDE && BOARD_GOOGLE_BASEBOARD_PUFF && BOARD_GOOGLE_DELBIN && BOARD_GOOGLE_ELDRID && BOARD_GOOGLE_HALVOR && BOARD_GOOGLE_LINDAR && BOARD_GOOGLE_MALEFOR && BOARD_GOOGLE_TERRADOR && BOARD_GOOGLE_TODOR && BOARD_GOOGLE_TRONDO && BOARD_GOOGLE_VOLTEER && BOARD_GOOGLE_VOLTEER2 && BOARD_GOOGLE_VOLTEER2_TI50 && BOARD_GOOGLE_VOXEL && BOARD_GOOGLE_ELEMI && BOARD_GOOGLE_VOEMA && BOARD_SPECIFIC_OPTIONS && BOARD_SPECIFIC_OPTIONS) selects SOC_INTEL_CSE_LITE_SKU which has unmet direct dependencies (SOC_INTEL_COMMON && SOC_INTEL_COMMON_BLOCK && CHROMEOS)
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/49059 )
Change subject: soc/intel/common/cse: Drop dependency on CHROMEOS for SOC_INTEL_CSE_LITE_SKU ......................................................................
soc/intel/common/cse: Drop dependency on CHROMEOS for SOC_INTEL_CSE_LITE_SKU
The CSE lite SKU has 2 CSE firmware boot partitions vs 3 for the "normal" SKU; this has nothing to do with building for ChromeOS or not, and by having this dependency, boards with select the CSE lite SKU are unable to build with CONFIG_CHROMEOS unset due to Kconfig dependency issues.
Test: build google/wyvern with CONFIG_CHROMEOS not set.
Change-Id: I6959f35e1285b2fab7ea1f83a5ccfcb065c12397 Signed-off-by: Matt DeVillier matt.devillier@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/49059 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-by: Karthik Ramasubramanian kramasub@google.com Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/google/hatch/Kconfig M src/soc/intel/common/block/cse/Kconfig 2 files changed, 1 insertion(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Angel Pons: Looks good to me, approved Karthik Ramasubramanian: Looks good to me, but someone else must approve Tim Wawrzynczak: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/hatch/Kconfig b/src/mainboard/google/hatch/Kconfig index 548a8a6..d088341 100644 --- a/src/mainboard/google/hatch/Kconfig +++ b/src/mainboard/google/hatch/Kconfig @@ -11,7 +11,7 @@ select RT8168_SET_LED_MODE select ROMSTAGE_SPD_SMBUS select SPD_READ_BY_WORD - select SOC_INTEL_CSE_LITE_SKU if CHROMEOS + select SOC_INTEL_CSE_LITE_SKU select DRIVERS_INTEL_DPTF select DPTF_USE_EISA_HID
diff --git a/src/soc/intel/common/block/cse/Kconfig b/src/soc/intel/common/block/cse/Kconfig index fec112b..70e6763 100644 --- a/src/soc/intel/common/block/cse/Kconfig +++ b/src/soc/intel/common/block/cse/Kconfig @@ -16,7 +16,6 @@ config SOC_INTEL_CSE_LITE_SKU bool default n - depends on CHROMEOS select ME_REGION_ALLOW_CPU_READ_ACCESS help Enables CSE Lite SKU
-
Angel Pons (Code Review) -
Furquan Shaikh (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Matt DeVillier (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Tim Wawrzynczak (Code Review)