Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34112 )
Change subject: arch/non-x86: Flip HAVE_MONOTONIC_TIMER default
......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34112/3//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/34112/3//COMMIT_MSG@9
PS3, Line 9: Also Enables qualcomm/ipq_806x monotonic timer.
Better move into a separate follow-up patch in case something
is wrong with the implementation?
https://review.coreboot.org/c/coreboot/+/34112/3/src/Kconfig
File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/34112/3/src/Kconfig@509
PS3, Line 509: default
> Hmm... no? I don't know what you would want this to look like.
I guess he meant
depends on !NO_MONOTONIC_TIMER
and that would remove the
# HAVE_MONOTONIC_TIMER is not set
from .config files. Same effect would have a single default line
default y if !ARCH_X86 && !NO_MONOTONIC_TIMER
https://review.coreboot.org/c/coreboot/+/34112/3/src/cpu/allwinner/a10/Kcon…
File src/cpu/allwinner/a10/Kconfig:
https://review.coreboot.org/c/coreboot/+/34112/3/src/cpu/allwinner/a10/Kcon…
PS3, Line 13: select NO_MONOTONIC_TIMER
> Did you see the "implementation" file removed here.
There is a patch on Gerrit dropping the whole port. We were only
waiting for coreboot 4.10...
--
To view, visit https://review.coreboot.org/c/coreboot/+/34112
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9dfa9b92dc63375465e3bb87b73eeefad601c810
Gerrit-Change-Number: 34112
Gerrit-PatchSet: 3
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
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-Reviewer: Philipp Hug <philipp(a)hug.cx>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Sun, 07 Jul 2019 11:14:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Mike Banon has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33870
Change subject: src/mainboard/lenovo/g505s: Disable SeaBIOS options not supported by hardware
......................................................................
src/mainboard/lenovo/g505s: Disable SeaBIOS options not supported by hardware
G505S does not have any SAS or NVMe controllers and could not have a TPM,
so it makes sense to disable the related SeaBIOS options for this laptop.
This reduces the size of compiled SeaBIOS by 129344-110048 = 19296 bytes.
Signed-off-by: Mike Banon <mikebdp2(a)gmail.com>
Change-Id: Ib0183b7786ecd77bb0df923bc84908275f2fe14c
---
M src/mainboard/lenovo/g505s/Kconfig
A src/mainboard/lenovo/g505s/config_seabios
2 files changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/33870/1
diff --git a/src/mainboard/lenovo/g505s/Kconfig b/src/mainboard/lenovo/g505s/Kconfig
index 883ef27..b80019e 100644
--- a/src/mainboard/lenovo/g505s/Kconfig
+++ b/src/mainboard/lenovo/g505s/Kconfig
@@ -55,4 +55,8 @@
string
default "1002,990b"
+config PAYLOAD_CONFIGFILE
+ string
+ default "$(top)/src/mainboard/$(MAINBOARDDIR)/config_seabios" if PAYLOAD_SEABIOS
+
endif # BOARD_LENOVO_G505S
diff --git a/src/mainboard/lenovo/g505s/config_seabios b/src/mainboard/lenovo/g505s/config_seabios
new file mode 100644
index 0000000..1959fa3
--- /dev/null
+++ b/src/mainboard/lenovo/g505s/config_seabios
@@ -0,0 +1,7 @@
+###
+### SeaBIOS custom configuration for Lenovo G505S
+###
+# CONFIG_MEGASAS is not set
+# CONFIG_NVME is not set
+# CONFIG_TCGBIOS is not set
+#
--
To view, visit https://review.coreboot.org/c/coreboot/+/33870
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib0183b7786ecd77bb0df923bc84908275f2fe14c
Gerrit-Change-Number: 33870
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Banon <mikebdp2(a)gmail.com>
Gerrit-MessageType: newchange
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34085 )
Change subject: soc/intel/icelake: Refer to soc/soc_chip.h rather chip.h
......................................................................
Patch Set 2: Code-Review+1
(2 comments)
Looks good, just a few minor details.
https://review.coreboot.org/c/coreboot/+/34085/2//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/34085/2//COMMIT_MSG@7
PS2, Line 7: rather
rather *than*
https://review.coreboot.org/c/coreboot/+/34085/2/src/soc/intel/icelake/acpi…
File src/soc/intel/icelake/acpi.c:
https://review.coreboot.org/c/coreboot/+/34085/2/src/soc/intel/icelake/acpi…
PS2, Line 33: #include <soc/soc_chip.h>
Please move these includes so that they are alphabetically ordered
--
To view, visit https://review.coreboot.org/c/coreboot/+/34085
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9e3b5126173e7cec8f2809a38b92c82c9ed5327d
Gerrit-Change-Number: 34085
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <lean.sheng.tan(a)intel.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sun, 07 Jul 2019 10:28:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34112 )
Change subject: arch/non-x86: Flip HAVE_MONOTONIC_TIMER default
......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34112/3/src/Kconfig
File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/34112/3/src/Kconfig@509
PS3, Line 509: default
> Minor: Would 'depends' be better here?
Hmm... no? I don't know what you would want this to look like.
https://review.coreboot.org/c/coreboot/+/34112/3/src/cpu/allwinner/a10/Kcon…
File src/cpu/allwinner/a10/Kconfig:
https://review.coreboot.org/c/coreboot/+/34112/3/src/cpu/allwinner/a10/Kcon…
PS3, Line 13: select NO_MONOTONIC_TIMER
> Does this chip have a monotonic timer or not?
Did you see the "implementation" file removed here.
--
To view, visit https://review.coreboot.org/c/coreboot/+/34112
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9dfa9b92dc63375465e3bb87b73eeefad601c810
Gerrit-Change-Number: 34112
Gerrit-PatchSet: 3
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
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-Reviewer: Philipp Hug <philipp(a)hug.cx>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sun, 07 Jul 2019 10:25:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34112 )
Change subject: arch/non-x86: Flip HAVE_MONOTONIC_TIMER default
......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34112/3/src/Kconfig
File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/34112/3/src/Kconfig@509
PS3, Line 509: default
Minor: Would 'depends' be better here?
https://review.coreboot.org/c/coreboot/+/34112/3/src/cpu/allwinner/a10/Kcon…
File src/cpu/allwinner/a10/Kconfig:
https://review.coreboot.org/c/coreboot/+/34112/3/src/cpu/allwinner/a10/Kcon…
PS3, Line 13: select NO_MONOTONIC_TIMER
Does this chip have a monotonic timer or not?
--
To view, visit https://review.coreboot.org/c/coreboot/+/34112
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9dfa9b92dc63375465e3bb87b73eeefad601c810
Gerrit-Change-Number: 34112
Gerrit-PatchSet: 3
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
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-Reviewer: Philipp Hug <philipp(a)hug.cx>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sun, 07 Jul 2019 10:20:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34116 )
Change subject: soc/intel/cannonlake: Use SA_DEV_ROOT instead of PCH_DEV_PMC
......................................................................
soc/intel/cannonlake: Use SA_DEV_ROOT instead of PCH_DEV_PMC
PMC device gets hidden from PCI bus after FSP-S call. Thus, it gets
removed from the root bus as leftover unused device. With change
903b40a8a46 ("soc/intel: Replace uses of dev_find_slot()"), all uses
of dev_find_slot() were replaced by pcidev_path_on_root() which relies
on scanning of root bus to find the requested device. Since PMC device
is removed from the root bus, pcidev_path_on_root() returns NULL for
it thus resulting in configuration being skipped for the PMC
ultimately resulting in S3 failures.
Since the PCH_DEV_PMC was just used to get to chip config, this change
replaces the use of PCH_DEV_PMC with SA_DEV_ROOT.
BUG=b:136861224
TEST=Verified that S3 works fine on hatch.
Change-Id: Ie5ade00ac2aca697608f1bdea9764b71c26e2112
Signed-off-by: Furquan Shaikh <furquan(a)google.com>
---
M src/soc/intel/cannonlake/finalize.c
M src/soc/intel/cannonlake/pmc.c
2 files changed, 7 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/34116/1
diff --git a/src/soc/intel/cannonlake/finalize.c b/src/soc/intel/cannonlake/finalize.c
index 4dfd15b..d099d77 100644
--- a/src/soc/intel/cannonlake/finalize.c
+++ b/src/soc/intel/cannonlake/finalize.c
@@ -68,8 +68,13 @@
*
* Disabling ACPI PM timer is necessary for XTAL OSC shutdown.
* Disabling ACPI PM timer also switches off TCO
+ *
+ * SA_DEV_ROOT device is used here instead of PCH_DEV_PMC since it is
+ * just required to get to chip config. PCH_DEV_PMC is hidden by this
+ * point and hence removed from the root bus. pcidev_path_on_root thus
+ * returns NULL for PCH_DEV_PMC device.
*/
- dev = PCH_DEV_PMC;
+ dev = SA_DEV_ROOT;
config = dev->chip_info;
pmcbase = pmc_mmio_regs();
if (config->PmTimerDisabled) {
diff --git a/src/soc/intel/cannonlake/pmc.c b/src/soc/intel/cannonlake/pmc.c
index 6834aa2..0b23568 100644
--- a/src/soc/intel/cannonlake/pmc.c
+++ b/src/soc/intel/cannonlake/pmc.c
@@ -153,7 +153,7 @@
static void pmc_init(void *unused)
{
- struct device *dev = PCH_DEV_PMC;
+ struct device *dev = SA_DEV_ROOT;
config_t *config = dev->chip_info;
rtc_init();
--
To view, visit https://review.coreboot.org/c/coreboot/+/34116
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie5ade00ac2aca697608f1bdea9764b71c26e2112
Gerrit-Change-Number: 34116
Gerrit-PatchSet: 1
Gerrit-Owner: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: newchange
Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34118 )
Change subject: soc/intel/icelake: Use SA_DEV_ROOT instead of PCH_DEV_PMC
......................................................................
soc/intel/icelake: Use SA_DEV_ROOT instead of PCH_DEV_PMC
PMC device gets hidden from PCI bus after FSP-S call. Thus, it gets
removed from the root bus as leftover unused device. With change
903b40a8a46 ("soc/intel: Replace uses of dev_find_slot()"), all uses
of dev_find_slot() were replaced by pcidev_path_on_root() which relies
on scanning of root bus to find the requested device. Since PMC device
is removed from the root bus, pcidev_path_on_root() returns NULL for
it thus resulting in configuration being skipped for the PMC
ultimately resulting in S3 failures.
Since the PCH_DEV_PMC was just used to get to chip config, this
change replaces the use of PCH_DEV_PMC with SA_DEV_ROOT.
BUG=b:136861224
Change-Id: Id68db8382b7b98e8e2e4a65ded1a6fb3bd057051
Signed-off-by: Furquan Shaikh <furquan(a)google.com>
---
M src/soc/intel/icelake/finalize.c
M src/soc/intel/icelake/pmc.c
2 files changed, 7 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/34118/1
diff --git a/src/soc/intel/icelake/finalize.c b/src/soc/intel/icelake/finalize.c
index e061cda..b838c19 100644
--- a/src/soc/intel/icelake/finalize.c
+++ b/src/soc/intel/icelake/finalize.c
@@ -69,8 +69,13 @@
*
* Disabling ACPI PM timer is necessary for XTAL OSC shutdown.
* Disabling ACPI PM timer also switches off TCO
+ *
+ * SA_DEV_ROOT device is used here instead of PCH_DEV_PMC since it is
+ * just required to get to chip config. PCH_DEV_PMC is hidden by this
+ * point and hence removed from the root bus. pcidev_path_on_root thus
+ * returns NULL for PCH_DEV_PMC device.
*/
- dev = PCH_DEV_PMC;
+ dev = SA_DEV_ROOT;
config = dev->chip_info;
pmcbase = pmc_mmio_regs();
if (config->PmTimerDisabled) {
diff --git a/src/soc/intel/icelake/pmc.c b/src/soc/intel/icelake/pmc.c
index 8f61d70..25913a8 100644
--- a/src/soc/intel/icelake/pmc.c
+++ b/src/soc/intel/icelake/pmc.c
@@ -136,7 +136,7 @@
static void pmc_init(void *unused)
{
- struct device *dev = PCH_DEV_PMC;
+ struct device *dev = SA_DEV_ROOT;
config_t *config = dev->chip_info;
rtc_init();
--
To view, visit https://review.coreboot.org/c/coreboot/+/34118
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id68db8382b7b98e8e2e4a65ded1a6fb3bd057051
Gerrit-Change-Number: 34118
Gerrit-PatchSet: 1
Gerrit-Owner: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newchange