Shaunak Saha has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44262 )
Change subject: soc/intel/tigerlake: move mainboard_silicon_init_params ......................................................................
soc/intel/tigerlake: move mainboard_silicon_init_params
This patch arranges mainboard_silicon_init_params before fetching any config variables from devicetree. This would allow the variant specific devicetree overrides to get consumed so that FSP UPD parameters are initialized properly before SiliconInit.
BUG=b:158573805 TEST=Test that UPD values are set properly with variant specific overrides of config's .
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: Idfce528efa7806e292071e092fb129b53a94a145 --- M src/soc/intel/tigerlake/fsp_params.c 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/44262/1
diff --git a/src/soc/intel/tigerlake/fsp_params.c b/src/soc/intel/tigerlake/fsp_params.c index a61a025..5178d91 100644 --- a/src/soc/intel/tigerlake/fsp_params.c +++ b/src/soc/intel/tigerlake/fsp_params.c @@ -107,6 +107,8 @@ if (CONFIG(USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI)) params->CpuMpPpi = (uintptr_t) mp_fill_ppi_services_data();
+ mainboard_silicon_init_params(params); + /* D3Hot and D3Cold for TCSS */ params->D3HotEnable = !config->TcssD3HotDisable; cpu_id = cpu_get_cpuid(); @@ -311,7 +313,6 @@
/* EnableMultiPhaseSiliconInit for running MultiPhaseSiInit */ params->EnableMultiPhaseSiliconInit = 1; - mainboard_silicon_init_params(params); }
/*
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44262 )
Change subject: soc/intel/tigerlake: move mainboard_silicon_init_params ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44262/1/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44262/1/src/soc/intel/tigerlake/fsp... PS1, Line 110: mainboard_silicon_init_params(params); Shouldn't this be done as the very first thing in this function? i.e. on line 95? That will ensure that mainboard can override all configurations at runtime.
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44262
to look at the new patch set (#2).
Change subject: soc/intel/tigerlake: move mainboard_silicon_init_params ......................................................................
soc/intel/tigerlake: move mainboard_silicon_init_params
This patch arranges mainboard_silicon_init_params before fetching any config variables from devicetree. This would allow the variant specific devicetree overrides to get consumed so that FSP UPD parameters are initialized properly before SiliconInit.
BUG=b:158573805 TEST=Test that UPD values are set properly with variant specific overrides of config's .
Signed-off-by: Shaunak Saha shaunak.saha@intel.com Change-Id: Idfce528efa7806e292071e092fb129b53a94a145 --- M src/soc/intel/tigerlake/fsp_params.c 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/44262/2
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44262 )
Change subject: soc/intel/tigerlake: move mainboard_silicon_init_params ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44262/1/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44262/1/src/soc/intel/tigerlake/fsp... PS1, Line 110: mainboard_silicon_init_params(params);
Shouldn't this be done as the very first thing in this function? i.e. […]
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44262 )
Change subject: soc/intel/tigerlake: move mainboard_silicon_init_params ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44262/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44262/2//COMMIT_MSG@10 PS2, Line 10: variant : specific devicetree overrides Where are these coming from? It's all compiled into one static.c/.h devicetree for each board
https://review.coreboot.org/c/coreboot/+/44262/2/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44262/2/src/soc/intel/tigerlake/fsp... PS2, Line 96: mainboard_silicon_init_params(params); Why did this move? I was pretty sure the reason we had it at the end of the function is that it allows the mainboard to override any parameter that was set here, based on devicetree settings. This allows us to modify UPDs based on SKU-specific diffs.
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/44262?usp=email )
Change subject: soc/intel/tigerlake: move mainboard_silicon_init_params ......................................................................
Abandoned
-
Furquan Shaikh (Code Review) -
Shaunak Saha (Code Review)
-
Stefan Reinauer (Code Review)
-
Tim Wawrzynczak (Code Review)