Hello Patrick Rudolph, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34725
to look at the new patch set (#4).
Change subject: soc/intel/skylake: Use new power-failure-state API
......................................................................
soc/intel/skylake: Use new power-failure-state API
Also move pmc_soc_restore_power_failure() which was guarded twice to
not be included in SMM, where the only call lives. Once all platforms
moved to the new API, it can be implemented in a central place, avoi-
ding the weak-function trap.
TEST=Manually tested all four cases with Kontron/bSL6 at the end
of this patch series.
Change-Id: Ie72753764ecd876e6cb999fa0074d1114ae5efcf
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
---
M src/soc/intel/skylake/Makefile.inc
M src/soc/intel/skylake/pmc.c
2 files changed, 26 insertions(+), 68 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/34725/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/34725
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie72753764ecd876e6cb999fa0074d1114ae5efcf
Gerrit-Change-Number: 34725
Gerrit-PatchSet: 4
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: newpatchset
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34728 )
Change subject: soc/intel/common: Implement MAINBOARD_POWER_STATE_PREVIOUS
......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34728/3/src/soc/intel/apollolake/p…
File src/soc/intel/apollolake/pmc.c:
https://review.coreboot.org/c/coreboot/+/34728/3/src/soc/intel/apollolake/p…
PS3, Line 125: PCI_DEV_INVALID
Please add a comment here as to why you're using PCI_DEV_INVALID
--
To view, visit https://review.coreboot.org/c/coreboot/+/34728
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I22bd71e4f3b67309c1aa0cb6faeb5959521bf656
Gerrit-Change-Number: 34728
Gerrit-PatchSet: 3
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Comment-Date: Wed, 07 Aug 2019 21:29:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33759 )
Change subject: soc/amd/picasso: Create a hybrid romstage to begin in DRAM
......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33759/9/src/soc/amd/picasso/romsta…
File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/33759/9/src/soc/amd/picasso/romsta…
PS9, Line 49: || (CONFIG_DCACHE_RAM_BASE >= CONFIG_ROMSTAGE_ADDR \
Comparisons should place the constant on the right side of the test
--
To view, visit https://review.coreboot.org/c/coreboot/+/33759
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id8c6175de34a0728ad41085e9c7cd310bd280976
Gerrit-Change-Number: 33759
Gerrit-PatchSet: 9
Gerrit-Owner: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Richard Spiegel <richard.spiegel(a)silverbackltd.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Wed, 07 Aug 2019 21:28:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Hello Edward O'Callaghan, Julius Werner, Richard Spiegel, build bot (Jenkins), Furquan Shaikh, Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33759
to look at the new patch set (#9).
Change subject: soc/amd/picasso: Create a hybrid romstage to begin in DRAM
......................................................................
soc/amd/picasso: Create a hybrid romstage to begin in DRAM
Add the support files to begin execution in romstage and located in
DRAM. Details for this implementation are found in
Documentation/amd/picasso/family17.md.
Combine steps typically found in bootblock, containing the reset
vector and protected mode enable, with the parts of romstage
that enable the console and cbmem.
Duplicate the ROMSTAGE_ADDR and ROMSTAGE_MAX_SIZE items into Kconfig
and give them safe default values in DRAM. The DCACHE values are
kept and DRAM is used as a CAR substitute.
Add a romstage.ld file that positions the reset vector and describes
the additional items needed for the hybrid romstage.
Change-Id: Id8c6175de34a0728ad41085e9c7cd310bd280976
Signed-off-by: Marshall Dawson <marshalldawson3rd(a)gmail.com>
---
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/Makefile.inc
M src/soc/amd/picasso/include/soc/cpu.h
M src/soc/amd/picasso/include/soc/romstage.h
A src/soc/amd/picasso/include/soc/romstage.ld
A src/soc/amd/picasso/reset_vector.S
M src/soc/amd/picasso/romstage.c
7 files changed, 400 insertions(+), 72 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/33759/9
--
To view, visit https://review.coreboot.org/c/coreboot/+/33759
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id8c6175de34a0728ad41085e9c7cd310bd280976
Gerrit-Change-Number: 33759
Gerrit-PatchSet: 9
Gerrit-Owner: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Richard Spiegel <richard.spiegel(a)silverbackltd.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34478 )
Change subject: soc/amd/picasso: Set HAVE_BOOTBLOCK=n
......................................................................
soc/amd/picasso: Set HAVE_BOOTBLOCK=n
Change-Id: Iaf370e04adb04eb81555a57e81812ebe3339971d
Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
---
M src/soc/amd/picasso/Kconfig
M src/soc/amd/picasso/Makefile.inc
D src/soc/amd/picasso/bootblock/bootblock.c
D src/soc/amd/picasso/cache_as_ram.S
4 files changed, 4 insertions(+), 46 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/34478/1
diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig
index 5ca0c91..0ba90ef 100644
--- a/src/soc/amd/picasso/Kconfig
+++ b/src/soc/amd/picasso/Kconfig
@@ -72,6 +72,10 @@
int
default 200
+config HAVE_BOOTBLOCK
+ bool
+ default n
+
# TODO: Sync these with definitions in PI vendorcode.
# DCACHE_RAM_BASE must equal BSP_STACK_BASE_ADDR.
# DCACHE_RAM_SIZE must equal BSP_STACK_SIZE.
diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc
index 93a8bbb..7f37192 100644
--- a/src/soc/amd/picasso/Makefile.inc
+++ b/src/soc/amd/picasso/Makefile.inc
@@ -37,13 +37,6 @@
subdirs-y += ../../../cpu/x86/pae
subdirs-y += ../../../cpu/x86/smm
-# TODO: Make coreboot modifications so bootblock can be removed. This soc
-# also selects C_ENVIRONMENT_BOOTBLOCK to enforce certain codepaths
-# in romstage. As a result, the bootblock build also needs a
-# dummy cache_as_ram.S
-bootblock-y += cache_as_ram.S
-bootblock-y += bootblock/bootblock.c
-
romstage-y += i2c.c
romstage-y += romstage.c
romstage-y += gpio.c
diff --git a/src/soc/amd/picasso/bootblock/bootblock.c b/src/soc/amd/picasso/bootblock/bootblock.c
deleted file mode 100644
index 62e4e15..0000000
--- a/src/soc/amd/picasso/bootblock/bootblock.c
+++ /dev/null
@@ -1,19 +0,0 @@
-/*
- * This file is part of the coreboot project.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; version 2 of the License.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-#include <bootblock_common.h>
-
-asmlinkage void bootblock_c_entry(uint64_t base_timestamp)
-{
- /* This function is here for building/linking only */
-}
diff --git a/src/soc/amd/picasso/cache_as_ram.S b/src/soc/amd/picasso/cache_as_ram.S
deleted file mode 100644
index 2869062..0000000
--- a/src/soc/amd/picasso/cache_as_ram.S
+++ /dev/null
@@ -1,20 +0,0 @@
-/*
- * This file is part of the coreboot project.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; version 2 of the License.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-/*
- * TODO: This is a dummy file for making bootblock build and link. At some
- * point, this should be removed from picasso since bootblock is
- * ignored.
- */
-.global bootblock_pre_c_entry
-bootblock_pre_c_entry:
--
To view, visit https://review.coreboot.org/c/coreboot/+/34478
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaf370e04adb04eb81555a57e81812ebe3339971d
Gerrit-Change-Number: 34478
Gerrit-PatchSet: 1
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-MessageType: newchange
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34724 )
Change subject: soc/intel/common: Implement power-failure-state handling
......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34724/4/src/soc/intel/common/block…
File src/soc/intel/common/block/pmc/pmclib.c:
https://review.coreboot.org/c/coreboot/+/34724/4/src/soc/intel/common/block…
PS4, Line 602: state == MAINBOARD_POWER_STATE_ON
> For the moment, yes. I tried to keep it close to the current behavior […]
Current behavior is if CONFIG_MAINBOARD_POWER_FAILURE_STATE is set to MAINBOARD_POWER_STATE_PREVIOUS, then the code just leaves the AFTERG3 bit untouched. But, I see that the meaning of MAINBOARD_POWER_STATE_PREVIOUS isn't supposed to be that based on CB:34728.
It makes sense to squash the two CLs together since it is easier to see what the overall logic is.
--
To view, visit https://review.coreboot.org/c/coreboot/+/34724
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic6970a79d9b95373c2855f4c92232d2aa05963bb
Gerrit-Change-Number: 34724
Gerrit-PatchSet: 4
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Wed, 07 Aug 2019 21:16:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31352 )
Change subject: soc/intel/apl: Implement power-failure-state API
......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/31352/8/src/soc/intel/apollolake/p…
File src/soc/intel/apollolake/pmc.c:
https://review.coreboot.org/c/coreboot/+/31352/8/src/soc/intel/apollolake/p…
PS8, Line 110: pmc_set_power_failure_state(PCI_DEV_INVALID);
Please add a comment here as to why you're using PCI_DEV_INVALID
https://review.coreboot.org/c/coreboot/+/31352/8/src/soc/intel/apollolake/p…
PS8, Line 130: PCI_DEV_INVALID
Please add a comment here as to why you're using PCI_DEV_INVALID
--
To view, visit https://review.coreboot.org/c/coreboot/+/31352
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibf218b90088a45349c54f4b881e895bb852e88bb
Gerrit-Change-Number: 31352
Gerrit-PatchSet: 8
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Comment-Date: Wed, 07 Aug 2019 21:06:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34726 )
Change subject: soc/intel/{cnl,icl}: Use new power-failure-state API
......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34726/3/src/soc/intel/icelake/pmc.c
File src/soc/intel/icelake/pmc.c:
https://review.coreboot.org/c/coreboot/+/34726/3/src/soc/intel/icelake/pmc.…
PS3, Line 39: 1
same as skylake; symbolic constant
https://review.coreboot.org/c/coreboot/+/34726/3/src/soc/intel/icelake/pmc.…
PS3, Line 41: 1
same as skylake; symbolic constant
--
To view, visit https://review.coreboot.org/c/coreboot/+/34726
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib13eac00002232d4377f683ad92b04a0907529f3
Gerrit-Change-Number: 34726
Gerrit-PatchSet: 3
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Comment-Date: Wed, 07 Aug 2019 21:04:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment