Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47396 )
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
soc/intel/tigerlake: Check TBT & TCSS ports for wake events
Wakes from TBT ports and TCSS devices will show up as PME_B0_STS wakes, so add checks for wakes from these devices in pch_log_pme_internal_wake_source.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: Ie9904c3c01ea85fcd83218fcfeaa4378b07c1463 --- M src/soc/intel/tigerlake/elog.c 1 file changed, 25 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/47396/1
diff --git a/src/soc/intel/tigerlake/elog.c b/src/soc/intel/tigerlake/elog.c index 88324d6..b8b2cc5 100644 --- a/src/soc/intel/tigerlake/elog.c +++ b/src/soc/intel/tigerlake/elog.c @@ -76,13 +76,17 @@ bool dev_found = false;
const struct pme_map ipme_map[] = { - { PCH_DEVFN_HDA, ELOG_WAKE_SOURCE_PME_HDA }, - { PCH_DEVFN_GBE, ELOG_WAKE_SOURCE_PME_GBE }, - { PCH_DEVFN_SATA, ELOG_WAKE_SOURCE_PME_SATA }, - { PCH_DEVFN_CSE, ELOG_WAKE_SOURCE_PME_CSE }, - { PCH_DEVFN_XHCI, ELOG_WAKE_SOURCE_PME_XHCI }, - { PCH_DEVFN_USBOTG, ELOG_WAKE_SOURCE_PME_XDCI }, - { PCH_DEVFN_CNVI_WIFI, ELOG_WAKE_SOURCE_PME_WIFI }, + { PCH_DEVFN_HDA, ELOG_WAKE_SOURCE_PME_HDA }, + { PCH_DEVFN_GBE, ELOG_WAKE_SOURCE_PME_GBE }, + { PCH_DEVFN_SATA, ELOG_WAKE_SOURCE_PME_SATA }, + { PCH_DEVFN_CSE, ELOG_WAKE_SOURCE_PME_CSE }, + { PCH_DEVFN_XHCI, ELOG_WAKE_SOURCE_PME_XHCI }, + { PCH_DEVFN_USBOTG, ELOG_WAKE_SOURCE_PME_XDCI }, + { PCH_DEVFN_CNVI_WIFI, ELOG_WAKE_SOURCE_PME_WIFI }, + { SA_DEVFN_TCSS_XHCI, ELOG_WAKE_SOURCE_PME_TCSS_XHCI }, + { SA_DEVFN_TCSS_XDCI, ELOG_WAKE_SOURCE_PME_TCSS_XDCI }, + { SA_DEVFN_TCSS_DMA0, ELOG_WAKE_SOURCE_PME_TCSS_DMA0 }, + { SA_DEVFN_TCSS_DMA1, ELOG_WAKE_SOURCE_PME_TCSS_DMA1 }, };
for (i = 0; i < ARRAY_SIZE(ipme_map); i++) { @@ -106,6 +110,20 @@ if (!dev_found) dev_found = pch_xhci_update_wake_event(soc_get_xhci_usb_info());
+ /* Check Thunderbolt ports */ + if (!dev_found) { + for (i = 0; i < NUM_TBT_FUNCTIONS; i++) { + const struct device *dev = pcidev_path_on_root(SA_DEVFN_TBT(i)); + if (!dev) + continue; + + if (pci_dev_is_wake_source(dev)) { + elog_add_event_wake(ELOG_WAKE_SOURCE_PME_TBT, i); + dev_found = true; + } + } + } + if (!dev_found) elog_add_event_wake(ELOG_WAKE_SOURCE_PME_INTERNAL, 0); }
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47396 )
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47396/1/src/soc/intel/tigerlake/elo... File src/soc/intel/tigerlake/elog.c:
https://review.coreboot.org/c/coreboot/+/47396/1/src/soc/intel/tigerlake/elo... PS1, Line 86: SA_DEVFN_TCSS_XHCI Do we care about printing the port # just like it is done for PCH_DEVFN_XHCI?
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47396
to look at the new patch set (#2).
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
soc/intel/tigerlake: Check TBT & TCSS ports for wake events
Wakes from TBT ports and TCSS devices will show up as PME_B0_STS wakes, so add checks for wakes from these devices in pch_log_pme_internal_wake_source.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: Ie9904c3c01ea85fcd83218fcfeaa4378b07c1463 --- M src/soc/intel/tigerlake/Kconfig M src/soc/intel/tigerlake/Makefile.inc M src/soc/intel/tigerlake/elog.c 3 files changed, 38 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/47396/2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47396 )
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47396/1/src/soc/intel/tigerlake/elo... File src/soc/intel/tigerlake/elog.c:
https://review.coreboot.org/c/coreboot/+/47396/1/src/soc/intel/tigerlake/elo... PS1, Line 86: SA_DEVFN_TCSS_XHCI
Do we care about printing the port # just like it is done for PCH_DEVFN_XHCI?
Probably, good call. Will add infra for that.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47396 )
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47396/1/src/soc/intel/tigerlake/elo... File src/soc/intel/tigerlake/elog.c:
https://review.coreboot.org/c/coreboot/+/47396/1/src/soc/intel/tigerlake/elo... PS1, Line 86: SA_DEVFN_TCSS_XHCI
Probably, good call. Will add infra for that.
I think we should be able to reuse the support that we have for PCH_DEVFN_XHCI by providing different set of offsets to be used for USB2 and USB3 ports.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47396 )
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47396/1/src/soc/intel/tigerlake/elo... File src/soc/intel/tigerlake/elog.c:
https://review.coreboot.org/c/coreboot/+/47396/1/src/soc/intel/tigerlake/elo... PS1, Line 86: SA_DEVFN_TCSS_XHCI
I think we should be able to reuse the support that we have for PCH_DEVFN_XHCI by providing differen […]
it could be adapted to take in a device instead of assuming PCH
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47396 )
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47396/1/src/soc/intel/tigerlake/elo... File src/soc/intel/tigerlake/elog.c:
https://review.coreboot.org/c/coreboot/+/47396/1/src/soc/intel/tigerlake/elo... PS1, Line 86: SA_DEVFN_TCSS_XHCI
it could be adapted to take in a device instead of assuming PCH
I think that makes sense. It looks like both north and south XHCI have the exact same structure. So as long as you pass in the device and port register map, the same driver should be able to handle it.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Caveh Jalali, Nick Vaccaro, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47396
to look at the new patch set (#3).
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
soc/intel/tigerlake: Check TBT & TCSS ports for wake events
Wakes from TBT ports and TCSS devices will show up as PME_B0_STS wakes, so add checks for wakes from these devices in pch_log_pme_internal_wake_source.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: Ie9904c3c01ea85fcd83218fcfeaa4378b07c1463 --- M src/soc/intel/tigerlake/elog.c M src/soc/intel/tigerlake/xhci.c 2 files changed, 62 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/47396/3
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47396 )
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47396/3/src/soc/intel/tigerlake/elo... File src/soc/intel/tigerlake/elog.c:
https://review.coreboot.org/c/coreboot/+/47396/3/src/soc/intel/tigerlake/elo... PS3, Line 86: SA_DEVFN_TCSS_XDCI Was SA_DEVFN_TCSS_XHCI intentionally skipped here? I think with the suggestion that I made on previous CL, it would make sense to not include ELOG_WAKE_SOURCE_PME_TCSS_XHCI or PCH_DEVFN_XHCI in this table and let `xhci_update_wake_event` handle it.
https://review.coreboot.org/c/coreboot/+/47396/3/src/soc/intel/tigerlake/elo... PS3, Line 125: dev_found = You lose information about `dev_found` if it was already set to true before this.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Caveh Jalali, Nick Vaccaro, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47396
to look at the new patch set (#4).
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
soc/intel/tigerlake: Check TBT & TCSS ports for wake events
Wakes from TBT ports and TCSS devices will show up as PME_B0_STS wakes, so add checks for wakes from these devices in pch_log_pme_internal_wake_source.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: Ie9904c3c01ea85fcd83218fcfeaa4378b07c1463 --- M src/soc/intel/tigerlake/elog.c 1 file changed, 31 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/47396/4
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47396 )
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47396/3/src/soc/intel/tigerlake/elo... File src/soc/intel/tigerlake/elog.c:
https://review.coreboot.org/c/coreboot/+/47396/3/src/soc/intel/tigerlake/elo... PS3, Line 86: SA_DEVFN_TCSS_XDCI
Was SA_DEVFN_TCSS_XHCI intentionally skipped here? I think with the suggestion that I made on previous CL, it would make sense to not include ELOG_WAKE_SOURCE_PME_TCSS_XHCI or PCH_DEVFN_XHCI in this table and let `xhci_update_wake_event` handle it.
Agreed.
https://review.coreboot.org/c/coreboot/+/47396/3/src/soc/intel/tigerlake/elo... PS3, Line 125: dev_found =
You lose information about `dev_found` if it was already set to true before this.
Ooh true, also line 120, those will have to be OR
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47396 )
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47396/1/src/soc/intel/tigerlake/elo... File src/soc/intel/tigerlake/elog.c:
https://review.coreboot.org/c/coreboot/+/47396/1/src/soc/intel/tigerlake/elo... PS1, Line 86: SA_DEVFN_TCSS_XHCI
I think that makes sense. It looks like both north and south XHCI have the exact same structure. […]
Done
https://review.coreboot.org/c/coreboot/+/47396/3/src/soc/intel/tigerlake/elo... File src/soc/intel/tigerlake/elog.c:
https://review.coreboot.org/c/coreboot/+/47396/3/src/soc/intel/tigerlake/elo... PS3, Line 125: dev_found =
Ooh true, also line 120, those will have to be OR
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47396 )
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
Patch Set 5: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47396 )
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47396/5/src/soc/intel/tigerlake/elo... File src/soc/intel/tigerlake/elog.c:
https://review.coreboot.org/c/coreboot/+/47396/5/src/soc/intel/tigerlake/elo... PS5, Line 96: pci_dev_is_wake_source Not for this change, but I think we need to add a check in pci_dev_is_wake_source to ensure that the PCI config space does not read back all 1s. If it does, bail out early. It will ensure we don't access the config space when the device is power-gated.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47396 )
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47396/5/src/soc/intel/tigerlake/elo... File src/soc/intel/tigerlake/elog.c:
https://review.coreboot.org/c/coreboot/+/47396/5/src/soc/intel/tigerlake/elo... PS5, Line 96: pci_dev_is_wake_source
Not for this change, but I think we need to add a check in pci_dev_is_wake_source to ensure that the […]
I think the header type check in pci_s_find_next_capability will effectively do that, but it doesn't hurt to be explicit either.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47396 )
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47396/5/src/soc/intel/tigerlake/elo... File src/soc/intel/tigerlake/elog.c:
https://review.coreboot.org/c/coreboot/+/47396/5/src/soc/intel/tigerlake/elo... PS5, Line 96: pci_dev_is_wake_source
I think the header type check in pci_s_find_next_capability will effectively do that, but it doesn't […]
Ack.
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47396 )
Change subject: soc/intel/tigerlake: Check TBT & TCSS ports for wake events ......................................................................
soc/intel/tigerlake: Check TBT & TCSS ports for wake events
Wakes from TBT ports and TCSS devices will show up as PME_B0_STS wakes, so add checks for wakes from these devices in pch_log_pme_internal_wake_source.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: Ie9904c3c01ea85fcd83218fcfeaa4378b07c1463 Reviewed-on: https://review.coreboot.org/c/coreboot/+/47396 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/tigerlake/elog.c 1 file changed, 31 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/tigerlake/elog.c b/src/soc/intel/tigerlake/elog.c index 531f5c1..7f40a37 100644 --- a/src/soc/intel/tigerlake/elog.c +++ b/src/soc/intel/tigerlake/elog.c @@ -60,12 +60,13 @@ static void pch_log_pme_internal_wake_source(void) { const struct pme_map ipme_map[] = { - { PCH_DEVFN_HDA, ELOG_WAKE_SOURCE_PME_HDA }, - { PCH_DEVFN_GBE, ELOG_WAKE_SOURCE_PME_GBE }, - { PCH_DEVFN_SATA, ELOG_WAKE_SOURCE_PME_SATA }, - { PCH_DEVFN_CSE, ELOG_WAKE_SOURCE_PME_CSE }, - { PCH_DEVFN_USBOTG, ELOG_WAKE_SOURCE_PME_XDCI }, - { PCH_DEVFN_CNVI_WIFI, ELOG_WAKE_SOURCE_PME_WIFI }, + { PCH_DEVFN_HDA, ELOG_WAKE_SOURCE_PME_HDA }, + { PCH_DEVFN_GBE, ELOG_WAKE_SOURCE_PME_GBE }, + { PCH_DEVFN_SATA, ELOG_WAKE_SOURCE_PME_SATA }, + { PCH_DEVFN_CSE, ELOG_WAKE_SOURCE_PME_CSE }, + { PCH_DEVFN_USBOTG, ELOG_WAKE_SOURCE_PME_XDCI }, + { PCH_DEVFN_CNVI_WIFI, ELOG_WAKE_SOURCE_PME_WIFI }, + { SA_DEVFN_TCSS_XDCI, ELOG_WAKE_SOURCE_PME_TCSS_XDCI }, }; const struct xhci_wake_info xhci_wake_info[] = { { PCH_DEVFN_XHCI, ELOG_WAKE_SOURCE_PME_XHCI }, @@ -86,6 +87,30 @@ } }
+ /* Check Thunderbolt ports */ + for (i = 0; i < NUM_TBT_FUNCTIONS; i++) { + const struct device *dev = pcidev_path_on_root(SA_DEVFN_TBT(i)); + if (!dev) + continue; + + if (pci_dev_is_wake_source(dev)) { + elog_add_event_wake(ELOG_WAKE_SOURCE_PME_TBT, i); + dev_found = true; + } + } + + /* Check DMA devices */ + for (i = 0; i < NUM_TCSS_DMA_FUNCTIONS; i++) { + const struct device *dev = pcidev_path_on_root(SA_DEVFN_TCSS_DMA(i)); + if (!dev) + continue; + + if (pci_dev_is_wake_source(dev)) { + elog_add_event_wake(ELOG_WAKE_SOURCE_PME_TCSS_DMA, i); + dev_found = true; + } + } + /* * Check the XHCI controllers' USB2 & USB3 ports for wake events. There * are cases (GSMI logging for S0ix clears PME_STS_BIT) where the XHCI
-
Furquan Shaikh (Code Review) -
Tim Wawrzynczak (Code Review)