Sridhar Siricilla has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39226 )
Change subject: soc/intel: Enables CSE Firmware Custom SKU for hatch ......................................................................
soc/intel: Enables CSE Firmware Custom SKU for hatch
TEST=Verified on hatch
Change-Id: I1c85228a0fa71785e5cc1434cd60d77cd8ffb4a2 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/mainboard/google/hatch/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/me.c 3 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/39226/1
diff --git a/src/mainboard/google/hatch/Kconfig b/src/mainboard/google/hatch/Kconfig index 92d94db..c411077 100644 --- a/src/mainboard/google/hatch/Kconfig +++ b/src/mainboard/google/hatch/Kconfig @@ -25,6 +25,7 @@ select SYSTEM_TYPE_LAPTOP select RT8168_GET_MAC_FROM_VPD select RT8168_SET_LED_MODE + select INTEL_CSE_FW_SKU_CUSTOM
if BOARD_GOOGLE_BASEBOARD_HATCH
diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index b68e93d..b301373 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -84,6 +84,7 @@ select PMC_GLOBAL_RESET_ENABLE_LOCK select SOC_INTEL_COMMON select SOC_INTEL_COMMON_ACPI_WAKE_SOURCE + select SOC_INTEL_COMMON_BASECODE select SOC_INTEL_COMMON_BLOCK select SOC_INTEL_COMMON_BLOCK_ACPI select SOC_INTEL_COMMON_BLOCK_CHIP_CONFIG @@ -141,6 +142,12 @@ Refer to Platform FSP integration guide document to know the exact FSP requirement for Heap setup.
+config INTEL_CSE_FW_SKU_CUSTOM + bool + default n + help + Enables CSE Firmware SKU Lite. + config IFD_CHIPSET string default "cnl" diff --git a/src/soc/intel/cannonlake/me.c b/src/soc/intel/cannonlake/me.c index d41b0b8..33109a7 100644 --- a/src/soc/intel/cannonlake/me.c +++ b/src/soc/intel/cannonlake/me.c @@ -173,4 +173,8 @@ hfsts6.fields.txt_support ? "YES" : "NO"); } BOOT_STATE_INIT_ENTRY(BS_DEV_ENABLE, BS_ON_EXIT, print_me_fw_version, NULL); +#if CONFIG(INTEL_CSE_FW_SKU_CUSTOM) +BOOT_STATE_INIT_ENTRY(BS_DEV_ENABLE, BS_ON_EXIT, update_csme, NULL); +#endif + BOOT_STATE_INIT_ENTRY(BS_OS_RESUME_CHECK, BS_ON_EXIT, dump_me_status, NULL);
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39226 )
Change subject: soc/intel: Enables CSE Firmware Custom SKU for hatch ......................................................................
Patch Set 1:
(1 comment)
Please add more details to the commit message.
https://review.coreboot.org/c/coreboot/+/39226/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39226/1//COMMIT_MSG@7 PS1, Line 7: Enables Please use imperative mood:
Enable CSE …
Hello build bot (Jenkins), Wonkyu Kim, Jamie Ryu, Rizwan Qureshi, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39226
to look at the new patch set (#2).
Change subject: soc/intel: Enable CSE Firmware Custom SKU for hatch ......................................................................
soc/intel: Enable CSE Firmware Custom SKU for hatch
It enable CSE Firmware SKU Custom for hatch board.
TEST=Verified on hatch
Change-Id: I1c85228a0fa71785e5cc1434cd60d77cd8ffb4a2 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/mainboard/google/hatch/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/me.c 3 files changed, 18 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/39226/2
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39226 )
Change subject: soc/intel: Enable CSE Firmware Custom SKU for hatch ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39226/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39226/1//COMMIT_MSG@7 PS1, Line 7: Enables
Please use imperative mood: […]
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39226 )
Change subject: soc/intel: Enable CSE Firmware Custom SKU for hatch ......................................................................
Patch Set 4: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/39226/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39226/4//COMMIT_MSG@9 PS4, Line 9: It enable CSE Firmware SKU Custom for hatch board. How about something like this:
Hatch can use a Custom CSE Firmware SKU, so enable support for it.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39226 )
Change subject: soc/intel: Enable CSE Firmware Custom SKU for hatch ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39226/4/src/mainboard/google/hatch/... File src/mainboard/google/hatch/Kconfig:
PS4: Mainboard changes should be separated from SoC changes. Also, we don't really want to enable this for hatch right away. This should just be a Draft change for testing.
https://review.coreboot.org/c/coreboot/+/39226/4/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/39226/4/src/soc/intel/cannonlake/Kc... PS4, Line 87: SOC_INTEL_COMMON_BASECODE I don't think we should be adding cse related code to basecode. Please see my comment here: https://review.coreboot.org/c/coreboot/+/35403/67/src/soc/intel/common/basec...
https://review.coreboot.org/c/coreboot/+/39226/4/src/soc/intel/cannonlake/Kc... PS4, Line 144: : config INTEL_CSE_FW_SKU_CUSTOM : bool : default n : help : Enables CSE Firmware SKU Custom. I don't think this should be added to every Intel SoC. Instead it should be put in soc/intel/common/block/cse/Kconfig -- something like : SOC_INTEL_CSE_CUSTOM_SKU. And mainboards that use this SKU can select it accordingly.
https://review.coreboot.org/c/coreboot/+/39226/4/src/soc/intel/cannonlake/me... File src/soc/intel/cannonlake/me.c:
https://review.coreboot.org/c/coreboot/+/39226/4/src/soc/intel/cannonlake/me... PS4, Line 178: enable_cse_fw_sku_custom I don't think we need to add this to specific SoC. It can simply be put in soc/intel/common/block/cse/custom_sku.c
Thus we don't need to add an indirection in specific SoC.
https://review.coreboot.org/c/coreboot/+/39226/4/src/soc/intel/cannonlake/me... PS4, Line 183: BS_DEV_ENABLE What is the reason behind making this call during this specific boot state?
Hello build bot (Jenkins), Wonkyu Kim, Furquan Shaikh, Jamie Ryu, Rizwan Qureshi, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39226
to look at the new patch set (#5).
Change subject: soc/intel: Enable CSE Firmware Custom SKU ......................................................................
soc/intel: Enable CSE Firmware Custom SKU
It enable CSE Firmware SKU Custom.
TEST=Verified on hatch
Change-Id: I1c85228a0fa71785e5cc1434cd60d77cd8ffb4a2 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/cannonlake/me.c 1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/39226/5
Hello build bot (Jenkins), Wonkyu Kim, Furquan Shaikh, Jamie Ryu, Rizwan Qureshi, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39226
to look at the new patch set (#6).
Change subject: soc/intel: Enable CSE Firmware Custom SKU ......................................................................
soc/intel: Enable CSE Firmware Custom SKU
It enable CSE Firmware SKU Custom.
TEST=Verified on hatch
Change-Id: I1c85228a0fa71785e5cc1434cd60d77cd8ffb4a2 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/cannonlake/me.c 1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/39226/6
Hello build bot (Jenkins), Wonkyu Kim, Furquan Shaikh, Jamie Ryu, Rizwan Qureshi, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39226
to look at the new patch set (#7).
Change subject: soc/intel: Enable CSE Firmware Custom SKU ......................................................................
soc/intel: Enable CSE Firmware Custom SKU
It enable CSE Firmware SKU Custom.
TEST=Verified on hatch
Change-Id: I1c85228a0fa71785e5cc1434cd60d77cd8ffb4a2 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/soc/intel/cannonlake/me.c 1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/39226/7
Hello build bot (Jenkins), Wonkyu Kim, Furquan Shaikh, Jamie Ryu, Rizwan Qureshi, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39226
to look at the new patch set (#8).
Change subject: mb/google/hatch: Enable CSE Firmware Custom SKU for hatch ......................................................................
mb/google/hatch: Enable CSE Firmware Custom SKU for hatch
Hatch can use a Custom CSE Firmware SKU, so enable support for it.
TEST=Verified on hatch
Change-Id: I1c85228a0fa71785e5cc1434cd60d77cd8ffb4a2 Signed-off-by: Sridhar Siricilla sridhar.siricilla@intel.com --- M src/mainboard/google/hatch/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/39226/8
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39226 )
Change subject: mb/google/hatch: Enable CSE Firmware Custom SKU for hatch ......................................................................
Patch Set 8:
(6 comments)
https://review.coreboot.org/c/coreboot/+/39226/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39226/4//COMMIT_MSG@9 PS4, Line 9: It enable CSE Firmware SKU Custom for hatch board.
How about something like this: […]
Done
https://review.coreboot.org/c/coreboot/+/39226/4/src/mainboard/google/hatch/... File src/mainboard/google/hatch/Kconfig:
PS4:
Mainboard changes should be separated from SoC changes. […]
Agreed.As of now, I am not enabling the CSE FW SKU CUSTOM for any board.
https://review.coreboot.org/c/coreboot/+/39226/4/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/39226/4/src/soc/intel/cannonlake/Kc... PS4, Line 87: SOC_INTEL_COMMON_BASECODE
I don't think we should be adding cse related code to basecode. […]
Implemented.
https://review.coreboot.org/c/coreboot/+/39226/4/src/soc/intel/cannonlake/Kc... PS4, Line 144: : config INTEL_CSE_FW_SKU_CUSTOM : bool : default n : help : Enables CSE Firmware SKU Custom.
I don't think this should be added to every Intel SoC. […]
Ack
https://review.coreboot.org/c/coreboot/+/39226/4/src/soc/intel/cannonlake/me... File src/soc/intel/cannonlake/me.c:
https://review.coreboot.org/c/coreboot/+/39226/4/src/soc/intel/cannonlake/me... PS4, Line 178: enable_cse_fw_sku_custom
I don't think we need to add this to specific SoC. […]
Done
https://review.coreboot.org/c/coreboot/+/39226/4/src/soc/intel/cannonlake/me... PS4, Line 183: BS_DEV_ENABLE
What is the reason behind making this call during this specific boot state?
Inline with other CSE operations, it's called here. But, this can be called during late ROM STAGE (after memmory initialization).
Sridhar Siricilla has removed Furquan Shaikh from this change. ( https://review.coreboot.org/c/coreboot/+/39226 )
Change subject: mb/google/hatch: Enable CSE Firmware Custom SKU for hatch ......................................................................
Removed reviewer Furquan Shaikh.
Sridhar Siricilla has removed Patrick Rudolph from this change. ( https://review.coreboot.org/c/coreboot/+/39226 )
Change subject: mb/google/hatch: Enable CSE Firmware Custom SKU for hatch ......................................................................
Removed reviewer Patrick Rudolph.
Sridhar Siricilla has removed Jamie Ryu from this change. ( https://review.coreboot.org/c/coreboot/+/39226 )
Change subject: mb/google/hatch: Enable CSE Firmware Custom SKU for hatch ......................................................................
Removed reviewer Jamie Ryu.
Sridhar Siricilla has removed Paul Menzel from this change. ( https://review.coreboot.org/c/coreboot/+/39226 )
Change subject: mb/google/hatch: Enable CSE Firmware Custom SKU for hatch ......................................................................
Removed reviewer Paul Menzel.
Sridhar Siricilla has removed Wonkyu Kim from this change. ( https://review.coreboot.org/c/coreboot/+/39226 )
Change subject: mb/google/hatch: Enable CSE Firmware Custom SKU for hatch ......................................................................
Removed reviewer Wonkyu Kim.
Sridhar Siricilla has removed Rizwan Qureshi from this change. ( https://review.coreboot.org/c/coreboot/+/39226 )
Change subject: mb/google/hatch: Enable CSE Firmware Custom SKU for hatch ......................................................................
Removed reviewer Rizwan Qureshi.
Sridhar Siricilla has removed Angel Pons from this change. ( https://review.coreboot.org/c/coreboot/+/39226 )
Change subject: mb/google/hatch: Enable CSE Firmware Custom SKU for hatch ......................................................................
Removed reviewer Angel Pons.
Sridhar Siricilla has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/39226 )
Change subject: mb/google/hatch: Enable CSE Firmware Custom SKU for hatch ......................................................................
Abandoned
This is no longer required.
-
Angel Pons (Code Review) -
Furquan Shaikh (Code Review)
-
Paul Menzel (Code Review)
-
Sridhar Siricilla (Code Review)