Patrick Georgi merged this change.

View Change

Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Subrata Banik: Looks good to me, approved
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@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/34116
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-by: Subrata Banik <subrata.banik@intel.com>
---
M src/soc/intel/cannonlake/finalize.c
M src/soc/intel/cannonlake/pmc.c
2 files changed, 7 insertions(+), 2 deletions(-)

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 change 34116. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie5ade00ac2aca697608f1bdea9764b71c26e2112
Gerrit-Change-Number: 34116
Gerrit-PatchSet: 2
Gerrit-Owner: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org>
Gerrit-Reviewer: Duncan Laurie <dlaurie@chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub@google.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki@gmail.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Paul Fagerburg <pfagerburg@chromium.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi@intel.com>
Gerrit-Reviewer: Shelley Chen <shchen@google.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-MessageType: merged