Change in coreboot[master]: soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage"
Sheng-Liang Pan has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48872 )
Change subject: soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage" ......................................................................
soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage"
This change will move "pmc_gpe_init()" from bootblock to verstage.
BUG=b:174118027 BRANCH=octopus TEST=build and verfiy no "TPM IRQ" error in AP log.
Signed-off-by: Pan Sheng-Liang sheng-liang.pan@quanta.corp-partner.google.com Change-Id: Icc98025b9a351078ce05a583ddafcdafaad83fd2 --- M Makefile.inc M src/device/Makefile.inc M src/security/vboot/vboot_common.h M src/security/vboot/verstage.c M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/apollolake/bootblock/bootblock.c A src/soc/intel/apollolake/verstage.c 7 files changed, 18 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/48872/1
diff --git a/Makefile.inc b/Makefile.inc index 1212dae..c7146be 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -629,7 +629,6 @@ ramstage-y+=$(DEVICETREE_STATIC_C) romstage-y+=$(DEVICETREE_STATIC_C) verstage-y+=$(DEVICETREE_STATIC_C) -bootblock-y+=$(DEVICETREE_STATIC_C) postcar-y+=$(DEVICETREE_STATIC_C) smm-y+=$(DEVICETREE_STATIC_C)
diff --git a/src/device/Makefile.inc b/src/device/Makefile.inc index bb125af..44d2db9 100644 --- a/src/device/Makefile.inc +++ b/src/device/Makefile.inc @@ -11,7 +11,6 @@ ramstage-srcs += $(wildcard src/mainboard/$(MAINBOARDDIR)/hda_verb.c) endif
-bootblock-y += device_const.c postcar-y += device_const.c smm-y += device_const.c verstage-y += device_const.c diff --git a/src/security/vboot/vboot_common.h b/src/security/vboot/vboot_common.h index 512da0e..9f2c279 100644 --- a/src/security/vboot/vboot_common.h +++ b/src/security/vboot/vboot_common.h @@ -44,6 +44,7 @@ void verstage_main(void); void verstage_mainboard_early_init(void); void verstage_mainboard_init(void); +void verstage_soc_init(void);
/* Check boot modes */ #if CONFIG(VBOOT) && !ENV_SMM diff --git a/src/security/vboot/verstage.c b/src/security/vboot/verstage.c index d2a9705..49749d8 100644 --- a/src/security/vboot/verstage.c +++ b/src/security/vboot/verstage.c @@ -11,10 +11,17 @@ /* Default empty implementation. */ }
+void __weak verstage_soc_init(void) +{ + /* Default empty implementation. */ +} + + void main(void) { console_init(); exception_init(); + verstage_soc_init(); verstage_mainboard_init();
if (CONFIG(VBOOT_RETURN_FROM_VERSTAGE)) { diff --git a/src/soc/intel/apollolake/Makefile.inc b/src/soc/intel/apollolake/Makefile.inc index 64889e5..3a675dc 100644 --- a/src/soc/intel/apollolake/Makefile.inc +++ b/src/soc/intel/apollolake/Makefile.inc @@ -90,14 +90,17 @@ verstage-y += pmutil.c verstage-y += reset.c verstage-y += spi.c +verstage-y += verstage.c
ifeq ($(CONFIG_SOC_INTEL_GEMINILAKE),y) bootblock-y += gpio_glk.c +verstage-y += gpio_glk.c romstage-y += gpio_glk.c smm-y += gpio_glk.c ramstage-y += gpio_glk.c else bootblock-y += gpio_apl.c +verstage-y += gpio_glk.c romstage-y += gpio_apl.c smm-y += gpio_apl.c ramstage-y += gpio_apl.c diff --git a/src/soc/intel/apollolake/bootblock/bootblock.c b/src/soc/intel/apollolake/bootblock/bootblock.c index 14e9b11..8d1b6da 100644 --- a/src/soc/intel/apollolake/bootblock/bootblock.c +++ b/src/soc/intel/apollolake/bootblock/bootblock.c @@ -94,9 +94,6 @@
fast_spi_cache_bios_region();
- /* Initialize GPE for use as interrupt status */ - pmc_gpe_init(); - /* Program TCO Timer Halt */ tco_configure();
diff --git a/src/soc/intel/apollolake/verstage.c b/src/soc/intel/apollolake/verstage.c new file mode 100644 index 0000000..4f76810 --- /dev/null +++ b/src/soc/intel/apollolake/verstage.c @@ -0,0 +1,7 @@ +#include <security/vboot/vboot_common.h> +#include <intelblocks/pmclib.h> + +void verstage_soc_init(void) +{ + pmc_gpe_init(); +}
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48872 )
Change subject: soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage" ......................................................................
Patch Set 1:
(8 comments)
https://review.coreboot.org/c/coreboot/+/48872/1/src/soc/intel/apollolake/ve... File src/soc/intel/apollolake/verstage.c:
https://review.coreboot.org/c/coreboot/+/48872/1/src/soc/intel/apollolake/ve... PS1, Line 1: #include <security/vboot/vboot_common.h> DOS line endings
https://review.coreboot.org/c/coreboot/+/48872/1/src/soc/intel/apollolake/ve... PS1, Line 2: #include <intelblocks/pmclib.h> DOS line endings
https://review.coreboot.org/c/coreboot/+/48872/1/src/soc/intel/apollolake/ve... PS1, Line 3: DOS line endings
https://review.coreboot.org/c/coreboot/+/48872/1/src/soc/intel/apollolake/ve... PS1, Line 4: void verstage_soc_init(void) DOS line endings
https://review.coreboot.org/c/coreboot/+/48872/1/src/soc/intel/apollolake/ve... PS1, Line 5: { DOS line endings
https://review.coreboot.org/c/coreboot/+/48872/1/src/soc/intel/apollolake/ve... PS1, Line 6: pmc_gpe_init(); DOS line endings
https://review.coreboot.org/c/coreboot/+/48872/1/src/soc/intel/apollolake/ve... PS1, Line 6: pmc_gpe_init(); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/48872/1/src/soc/intel/apollolake/ve... PS1, Line 7: } DOS line endings
Hello build bot (Jenkins), Vadim Bendebury, Patrick Georgi, Martin Roth, Marx Wang, Kane Chen, Duncan Laurie, Marco Chen, Andrey Petrov, Patrick Rudolph, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48872
to look at the new patch set (#2).
Change subject: soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage" ......................................................................
soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage"
This change will move "pmc_gpe_init()" from bootblock to verstage.
BUG=b:174118027 BRANCH=octopus TEST=build and verfiy no "TPM IRQ" error in AP log.
Signed-off-by: Pan Sheng-Liang sheng-liang.pan@quanta.corp-partner.google.com Change-Id: Icc98025b9a351078ce05a583ddafcdafaad83fd2 --- M Makefile.inc M src/device/Makefile.inc M src/security/vboot/vboot_common.h M src/security/vboot/verstage.c M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/apollolake/bootblock/bootblock.c A src/soc/intel/apollolake/verstage.c 7 files changed, 18 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/48872/2
Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48872 )
Change subject: soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage" ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48872/2/src/soc/intel/apollolake/ve... File src/soc/intel/apollolake/verstage.c:
https://review.coreboot.org/c/coreboot/+/48872/2/src/soc/intel/apollolake/ve... PS2, Line 1: #include <security/vboot/vboot_common.h> We need license header here - /* SPDX-License-Identifier: GPL-2.0-or-later */
Hello build bot (Jenkins), Vadim Bendebury, Patrick Georgi, Martin Roth, Marx Wang, Kane Chen, Duncan Laurie, Marco Chen, Andrey Petrov, Patrick Rudolph, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48872
to look at the new patch set (#3).
Change subject: soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage" ......................................................................
soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage"
This change will move "pmc_gpe_init()" from bootblock to verstage.
BUG=b:174118027 BRANCH=octopus TEST=build and verfiy no "TPM IRQ" error in AP log.
Signed-off-by: Pan Sheng-Liang sheng-liang.pan@quanta.corp-partner.google.com Change-Id: Icc98025b9a351078ce05a583ddafcdafaad83fd2 --- M Makefile.inc M src/device/Makefile.inc M src/security/vboot/vboot_common.h M src/security/vboot/verstage.c M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/apollolake/bootblock/bootblock.c A src/soc/intel/apollolake/verstage.c 7 files changed, 20 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/48872/3
Sheng-Liang Pan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48872 )
Change subject: soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage" ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48872/2/src/soc/intel/apollolake/ve... File src/soc/intel/apollolake/verstage.c:
https://review.coreboot.org/c/coreboot/+/48872/2/src/soc/intel/apollolake/ve... PS2, Line 1: #include <security/vboot/vboot_common.h>
We need license header here - /* SPDX-License-Identifier: GPL-2. […]
done
Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48872 )
Change subject: soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage" ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48872/3/src/device/Makefile.inc File src/device/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48872/3/src/device/Makefile.inc@a14 PS3, Line 14: bootblock-y += device_const.c Should we remove this line for GLK only but not for all platforms? And the Jenkins did show that other platforms will get failed to build.
i386-elf-ld.bfd: /cb-build/coreboot-gerrit.0/chromeos/GOOGLE_BOBBA/bootblock/soc/intel/apollolake/uart.o: in function `soc_uart_console_to_device':
/home/coreboot/node-root/workspace/coreboot-gerrit/src/soc/intel/apollolake/uart.c:77: undefined reference to `pcidev_path_on_root'
For Octopus, looks like pcidev_path_on_root in device_const.c is still used by uart.c above.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48872 )
Change subject: soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage" ......................................................................
Patch Set 3:
I think what we really need is a way to ensure that only the relevant pieces of devicetree really gets included in the early stages (i.e. bootblock) here. Let me see if I can make a quick pass at what changes would be required.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48872 )
Change subject: soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage" ......................................................................
Patch Set 3:
Patch Set 3:
I think what we really need is a way to ensure that only the relevant pieces of devicetree really gets included in the early stages (i.e. bootblock) here. Let me see if I can make a quick pass at what changes would be required.
Pushed a series here: https://review.coreboot.org/c/coreboot/+/49208/. The last patch still needs some more work.
Attention is currently required from: Sheng-Liang Pan. Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48872 )
Change subject: soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage" ......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
Patch Set 3:
Patch Set 3:
I think what we really need is a way to ensure that only the relevant pieces of devicetree really gets included in the early stages (i.e. bootblock) here. Let me see if I can make a quick pass at what changes would be required.
Pushed a series here: https://review.coreboot.org/c/coreboot/+/49208/. The last patch still needs some more work.
Thank you for the great support here so will you suggest to abandon this direction here?
Attention is currently required from: Marco Chen, Sheng-Liang Pan. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48872 )
Change subject: soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage" ......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
Patch Set 3: […]
Yes, the series I pointed to is almost ready to land. A few comments need to be addressed. I have confirmed that the series frees up ~17K of space in bootblock for GLK.
Sheng-Liang Pan has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/48872 )
Change subject: soc/intel/common: move "pmc_gpe_init" to the beginning of "verstage" ......................................................................
Abandoned
abandon CL since Furquan's CL already landed.
-
build bot (Jenkins) (Code Review) -
Furquan Shaikh (Code Review)
-
Marco Chen (Code Review)
-
Sheng-Liang Pan (Code Review)