Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47226 )
Change subject: soc/intel/jasperlake: Log PM event from an internal device ......................................................................
soc/intel/jasperlake: Log PM event from an internal device
Add support to check for the Power Management (PM) Status bit for various internal devices like USB, CNVi etc. and log them into the event log for debugging purposes.
BUG=b:172279037 TEST=Build and boot to OS in Drawlat. Ensure that the wake up event is logged into the event log for one of the internal devices eg. USB bluetooth. 17 | 2020-11-04 16:57:49 | S0ix Enter 18 | 2020-11-04 16:57:56 | S0ix Exit 19 | 2020-11-04 16:57:56 | Wake Source | PME - XHCI (USB 2.0 port) | 8 20 | 2020-11-04 16:57:56 | Wake Source | GPE # | 109 21 | 2020-11-04 16:59:00 | S0ix Enter 22 | 2020-11-04 16:59:06 | S0ix Exit 23 | 2020-11-04 16:59:06 | Wake Source | PME - XHCI (USB 2.0 port) | 8 24 | 2020-11-04 16:59:06 | Wake Source | GPE # | 109
Change-Id: I9f43675b698bf310f6b98b5e775d1259607abbcd Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/Makefile.inc M src/soc/intel/jasperlake/elog.c A src/soc/intel/jasperlake/xhci.c 4 files changed, 100 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/47226/1
diff --git a/src/soc/intel/jasperlake/Kconfig b/src/soc/intel/jasperlake/Kconfig index e84add0..294f19f8 100644 --- a/src/soc/intel/jasperlake/Kconfig +++ b/src/soc/intel/jasperlake/Kconfig @@ -52,6 +52,7 @@ select SOC_INTEL_COMMON_BLOCK_SMM select SOC_INTEL_COMMON_BLOCK_POWER_LIMIT select SOC_INTEL_COMMON_BLOCK_SMM_IO_TRAP + select SOC_INTEL_COMMON_BLOCK_XHCI_ELOG select SOC_INTEL_COMMON_FSP_RESET select SOC_INTEL_COMMON_PCH_BASE select SOC_INTEL_COMMON_RESET diff --git a/src/soc/intel/jasperlake/Makefile.inc b/src/soc/intel/jasperlake/Makefile.inc index 209d8a2..677b5f3 100644 --- a/src/soc/intel/jasperlake/Makefile.inc +++ b/src/soc/intel/jasperlake/Makefile.inc @@ -44,6 +44,7 @@ ramstage-y += systemagent.c ramstage-y += sd.c ramstage-y += me.c +ramstage-y += xhci.c
smm-y += gpio.c smm-y += p2sb.c @@ -52,6 +53,7 @@ smm-y += smihandler.c smm-y += uart.c smm-y += elog.c +smm-y += xhci.c
verstage-y += gpio.c
diff --git a/src/soc/intel/jasperlake/elog.c b/src/soc/intel/jasperlake/elog.c index 235dc6e..62ea5d3 100644 --- a/src/soc/intel/jasperlake/elog.c +++ b/src/soc/intel/jasperlake/elog.c @@ -2,12 +2,26 @@
#include <bootstate.h> #include <console/console.h> +#include <device/pci_ops.h> #include <stdint.h> #include <elog.h> #include <intelblocks/pmclib.h> +#include <intelblocks/xhci.h> #include <soc/pci_devs.h> #include <soc/pm.h>
+#define PME_STS_BIT (1 << 15) + +struct pme_status_info { +#ifdef __SIMPLE_DEVICE__ + pci_devfn_t dev; +#else + struct device *dev; +#endif + uint8_t reg_offset; + uint32_t elog_event; +}; + static void pch_log_gpio_gpe(u32 gpe0_sts, u32 gpe0_en, int start) { int i; @@ -20,6 +34,67 @@ } }
+static void pch_log_add_elog_event(const struct pme_status_info *info) +{ + /* + * If wake source is XHCI, check for detailed wake source events on + * USB2/3 ports. + */ + if ((info->dev == PCH_DEV_XHCI) && + pch_xhci_update_wake_event(soc_get_xhci_usb_info())) + return; + + elog_add_event_wake(info->elog_event, 0); +} + +static void pch_log_pme_internal_wake_source(void) +{ + size_t i; +#ifdef __SIMPLE_DEVICE__ + pci_devfn_t dev; +#else + struct device *dev; +#endif + uint16_t val; + bool dev_found = false; + + struct pme_status_info pme_status_info[] = { + { PCH_DEV_HDA, 0x54, ELOG_WAKE_SOURCE_PME_HDA }, + { PCH_DEV_GBE, 0xcc, ELOG_WAKE_SOURCE_PME_GBE }, + { PCH_DEV_SATA, 0x74, ELOG_WAKE_SOURCE_PME_SATA }, + { PCH_DEV_CSE, 0x54, ELOG_WAKE_SOURCE_PME_CSE }, + { PCH_DEV_XHCI, 0x74, ELOG_WAKE_SOURCE_PME_XHCI }, + { PCH_DEV_USBOTG, 0x84, ELOG_WAKE_SOURCE_PME_XDCI }, + { PCH_DEV_CNVI_WIFI, 0xcc, ELOG_WAKE_SOURCE_PME_WIFI }, + }; + + for (i = 0; i < ARRAY_SIZE(pme_status_info); i++) { + dev = pme_status_info[i].dev; + if (!dev) + continue; + + val = pci_read_config16(dev, pme_status_info[i].reg_offset); + if ((val == 0xFFFF) || !(val & PME_STS_BIT)) + continue; + + pch_log_add_elog_event(&pme_status_info[i]); + dev_found = true; + } + + /* + * If device is still not found, but the wake source is internal PME, + * try probing XHCI ports to see if any of the USB2/3 ports indicate + * that it was the wake source. This path would be taken in case of GSMI + * logging with S0ix where the pci_pm_resume_noirq runs and clears the + * PME_STS_BIT in controller register. + */ + if (!dev_found) + dev_found = pch_xhci_update_wake_event(soc_get_xhci_usb_info()); + + if (!dev_found) + elog_add_event_wake(ELOG_WAKE_SOURCE_PME_INTERNAL, 0); +} + static void pch_log_wake_source(struct chipset_power_state *ps) { /* Power Button */ @@ -38,9 +113,9 @@ if (ps->gpe0_sts[GPE_STD] & PME_STS) elog_add_event_wake(ELOG_WAKE_SOURCE_PME, 0);
- /* Internal PME (TODO: determine wake device) */ + /* Internal PME */ if (ps->gpe0_sts[GPE_STD] & PME_B0_STS) - elog_add_event_wake(ELOG_WAKE_SOURCE_PME_INTERNAL, 0); + pch_log_pme_internal_wake_source();
/* SMBUS Wake */ if (ps->gpe0_sts[GPE_STD] & SMB_WAK_STS) diff --git a/src/soc/intel/jasperlake/xhci.c b/src/soc/intel/jasperlake/xhci.c new file mode 100644 index 0000000..424751e --- /dev/null +++ b/src/soc/intel/jasperlake/xhci.c @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <intelblocks/xhci.h> + +#define XHCI_USB2_PORT_STATUS_REG 0x480 +#define XHCI_USB3_PORT_STATUS_REG 0x500 +#define XHCI_USB2_PORT_NUM 8 +#define XHCI_USB3_PORT_NUM 6 + +static const struct xhci_usb_info usb_info = { + .usb2_port_status_reg = XHCI_USB2_PORT_STATUS_REG, + .num_usb2_ports = XHCI_USB2_PORT_NUM, + .usb3_port_status_reg = XHCI_USB3_PORT_STATUS_REG, + .num_usb3_ports = XHCI_USB3_PORT_NUM, +}; + +const struct xhci_usb_info *soc_get_xhci_usb_info(void) +{ + return &usb_info; +}
Justin TerAvest has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47226 )
Change subject: soc/intel/jasperlake: Log PM event from an internal device ......................................................................
Patch Set 1: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47226 )
Change subject: soc/intel/jasperlake: Log PM event from an internal device ......................................................................
Patch Set 1: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47226 )
Change subject: soc/intel/jasperlake: Log PM event from an internal device ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47226/1/src/soc/intel/jasperlake/el... File src/soc/intel/jasperlake/elog.c:
https://review.coreboot.org/c/coreboot/+/47226/1/src/soc/intel/jasperlake/el... PS1, Line 16: #ifdef __SIMPLE_DEVICE__ : pci_devfn_t dev; : #else : struct device *dev; : #endif I think we can use `pci_devfn_t` irrespective of __SIMPLE_DEVICE__. Keeps code free of #ifdefs.
https://review.coreboot.org/c/coreboot/+/47226/1/src/soc/intel/jasperlake/el... PS1, Line 21: reg_offset We probably don't need the reg_offset. We should be able to use `pci_dev_is_wake_source()`. Please see CB:47182. I think both the changes can benefit from use of a single structure to capture dev<->elog_wake_source_type and then use `pci_dev_is_wake_source()` to determine if the device is a wake source and log corresponding elog entry.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47226 )
Change subject: soc/intel/jasperlake: Log PM event from an internal device ......................................................................
Patch Set 1: -Code-Review
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47226 )
Change subject: soc/intel/jasperlake: Log PM event from an internal device ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47226/1/src/soc/intel/jasperlake/el... File src/soc/intel/jasperlake/elog.c:
https://review.coreboot.org/c/coreboot/+/47226/1/src/soc/intel/jasperlake/el... PS1, Line 21: reg_offset
We probably don't need the reg_offset. We should be able to use `pci_dev_is_wake_source()`. […]
I added a `pci_s_dev_is_wake_source` function for the simple device model (used in SMM) for elog, would be good to rebase on top of CB:47183 to use that function. Then I can add TCSS IPs to elog for TGL too based on top of yours
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Justin TerAvest, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47226
to look at the new patch set (#2).
Change subject: soc/intel/jasperlake: Log PM event from an internal device ......................................................................
soc/intel/jasperlake: Log PM event from an internal device
Add support to check for the Power Management (PM) Status bit for various internal devices like USB, CNVi etc. and log them into the event log for debugging purposes.
BUG=b:172279037 TEST=Build and boot to OS in Drawlat. Ensure that the wake up event is logged into the event log for one of the internal devices eg. USB bluetooth. 8 | 2020-11-05 15:04:16 | S0ix Enter 9 | 2020-11-05 15:04:29 | S0ix Exit 10 | 2020-11-05 15:04:29 | Wake Source | PME - XHCI (USB 2.0 port) | 8 11 | 2020-11-05 15:04:29 | Wake Source | GPE # | 109 12 | 2020-11-05 15:05:08 | S0ix Enter 13 | 2020-11-05 15:05:14 | S0ix Exit 14 | 2020-11-05 15:05:14 | Wake Source | PME - XHCI (USB 2.0 port) | 8 15 | 2020-11-05 15:05:14 | Wake Source | GPE # | 109
Change-Id: I9f43675b698bf310f6b98b5e775d1259607abbcd Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/Makefile.inc M src/soc/intel/jasperlake/elog.c A src/soc/intel/jasperlake/xhci.c 4 files changed, 81 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/47226/2
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47226 )
Change subject: soc/intel/jasperlake: Log PM event from an internal device ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47226/1/src/soc/intel/jasperlake/el... File src/soc/intel/jasperlake/elog.c:
https://review.coreboot.org/c/coreboot/+/47226/1/src/soc/intel/jasperlake/el... PS1, Line 16: #ifdef __SIMPLE_DEVICE__ : pci_devfn_t dev; : #else : struct device *dev; : #endif
I think we can use `pci_devfn_t` irrespective of __SIMPLE_DEVICE__. Keeps code free of #ifdefs.
Done
https://review.coreboot.org/c/coreboot/+/47226/1/src/soc/intel/jasperlake/el... PS1, Line 21: reg_offset
I added a `pci_s_dev_is_wake_source` function for the simple device model (used in SMM) for elog, wo […]
Done
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Justin TerAvest, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47226
to look at the new patch set (#3).
Change subject: soc/intel/jasperlake: Log PM event from an internal device ......................................................................
soc/intel/jasperlake: Log PM event from an internal device
Add support to check for the Power Management (PM) Status bit for various internal devices like USB, CNVi etc. and log them into the event log for debugging purposes.
BUG=b:172279037 TEST=Build and boot to OS in Drawlat. Ensure that the wake up event is logged into the event log for one of the internal devices eg. USB bluetooth. 8 | 2020-11-05 15:04:16 | S0ix Enter 9 | 2020-11-05 15:04:29 | S0ix Exit 10 | 2020-11-05 15:04:29 | Wake Source | PME - XHCI (USB 2.0 port) | 8 11 | 2020-11-05 15:04:29 | Wake Source | GPE # | 109 12 | 2020-11-05 15:05:08 | S0ix Enter 13 | 2020-11-05 15:05:14 | S0ix Exit 14 | 2020-11-05 15:05:14 | Wake Source | PME - XHCI (USB 2.0 port) | 8 15 | 2020-11-05 15:05:14 | Wake Source | GPE # | 109
Change-Id: I9f43675b698bf310f6b98b5e775d1259607abbcd Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/Makefile.inc M src/soc/intel/jasperlake/elog.c A src/soc/intel/jasperlake/xhci.c 4 files changed, 84 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/47226/3
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47226 )
Change subject: soc/intel/jasperlake: Log PM event from an internal device ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47226 )
Change subject: soc/intel/jasperlake: Log PM event from an internal device ......................................................................
soc/intel/jasperlake: Log PM event from an internal device
Add support to check for the Power Management (PM) Status bit for various internal devices like USB, CNVi etc. and log them into the event log for debugging purposes.
BUG=b:172279037 TEST=Build and boot to OS in Drawlat. Ensure that the wake up event is logged into the event log for one of the internal devices eg. USB bluetooth. 8 | 2020-11-05 15:04:16 | S0ix Enter 9 | 2020-11-05 15:04:29 | S0ix Exit 10 | 2020-11-05 15:04:29 | Wake Source | PME - XHCI (USB 2.0 port) | 8 11 | 2020-11-05 15:04:29 | Wake Source | GPE # | 109 12 | 2020-11-05 15:05:08 | S0ix Enter 13 | 2020-11-05 15:05:14 | S0ix Exit 14 | 2020-11-05 15:05:14 | Wake Source | PME - XHCI (USB 2.0 port) | 8 15 | 2020-11-05 15:05:14 | Wake Source | GPE # | 109
Change-Id: I9f43675b698bf310f6b98b5e775d1259607abbcd Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/47226 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/jasperlake/Kconfig M src/soc/intel/jasperlake/Makefile.inc M src/soc/intel/jasperlake/elog.c A src/soc/intel/jasperlake/xhci.c 4 files changed, 84 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/jasperlake/Kconfig b/src/soc/intel/jasperlake/Kconfig index e84add0..294f19f8 100644 --- a/src/soc/intel/jasperlake/Kconfig +++ b/src/soc/intel/jasperlake/Kconfig @@ -52,6 +52,7 @@ select SOC_INTEL_COMMON_BLOCK_SMM select SOC_INTEL_COMMON_BLOCK_POWER_LIMIT select SOC_INTEL_COMMON_BLOCK_SMM_IO_TRAP + select SOC_INTEL_COMMON_BLOCK_XHCI_ELOG select SOC_INTEL_COMMON_FSP_RESET select SOC_INTEL_COMMON_PCH_BASE select SOC_INTEL_COMMON_RESET diff --git a/src/soc/intel/jasperlake/Makefile.inc b/src/soc/intel/jasperlake/Makefile.inc index 209d8a2..677b5f3 100644 --- a/src/soc/intel/jasperlake/Makefile.inc +++ b/src/soc/intel/jasperlake/Makefile.inc @@ -44,6 +44,7 @@ ramstage-y += systemagent.c ramstage-y += sd.c ramstage-y += me.c +ramstage-y += xhci.c
smm-y += gpio.c smm-y += p2sb.c @@ -52,6 +53,7 @@ smm-y += smihandler.c smm-y += uart.c smm-y += elog.c +smm-y += xhci.c
verstage-y += gpio.c
diff --git a/src/soc/intel/jasperlake/elog.c b/src/soc/intel/jasperlake/elog.c index 0546bb0..5d3c69e 100644 --- a/src/soc/intel/jasperlake/elog.c +++ b/src/soc/intel/jasperlake/elog.c @@ -5,11 +5,17 @@ #include <device/pci_ops.h> #include <elog.h> #include <intelblocks/pmclib.h> +#include <intelblocks/xhci.h> #include <soc/pci_devs.h> #include <soc/pm.h> #include <stdint.h> #include <types.h>
+struct pme_map { + pci_devfn_t devfn; + unsigned int wake_source; +}; + static void pch_log_gpio_gpe(u32 gpe0_sts, u32 gpe0_en, int start) { int i; @@ -25,10 +31,6 @@ static void pch_log_rp_wake_source(void) { size_t i; - struct pme_map { - pci_devfn_t devfn; - unsigned int wake_source; - };
const struct pme_map pme_map[] = { { PCH_DEVFN_PCIE1, ELOG_WAKE_SOURCE_PME_PCIE1 }, @@ -55,6 +57,59 @@ } }
+static void pch_log_add_elog_event(const struct pme_map *ipme_map) +{ + /* + * If wake source is XHCI, check for detailed wake source events on + * USB2/3 ports. + */ + if ((ipme_map->devfn == PCH_DEVFN_XHCI) && + pch_xhci_update_wake_event(soc_get_xhci_usb_info())) + return; + + elog_add_event_wake(ipme_map->wake_source, 0); +} + +static void pch_log_pme_internal_wake_source(void) +{ + size_t i; + 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 }, + }; + + for (i = 0; i < ARRAY_SIZE(ipme_map); i++) { + const struct device *dev = pcidev_path_on_root(ipme_map[i].devfn); + if (!dev) + continue; + + if (pci_dev_is_wake_source(dev)) { + pch_log_add_elog_event(&ipme_map[i]); + dev_found = true; + } + } + + /* + * If device is still not found, but the wake source is internal PME, + * try probing XHCI ports to see if any of the USB2/3 ports indicate + * that it was the wake source. This path would be taken in case of GSMI + * logging with S0ix where the pci_pm_resume_noirq runs and clears the + * PME_STS_BIT in controller register. + */ + if (!dev_found) + dev_found = pch_xhci_update_wake_event(soc_get_xhci_usb_info()); + + if (!dev_found) + elog_add_event_wake(ELOG_WAKE_SOURCE_PME_INTERNAL, 0); +} + static void pch_log_wake_source(struct chipset_power_state *ps) { /* Power Button */ @@ -73,9 +128,9 @@ if (ps->gpe0_sts[GPE_STD] & PME_STS) elog_add_event_wake(ELOG_WAKE_SOURCE_PME, 0);
- /* Internal PME (TODO: determine wake device) */ + /* Internal PME */ if (ps->gpe0_sts[GPE_STD] & PME_B0_STS) - elog_add_event_wake(ELOG_WAKE_SOURCE_PME_INTERNAL, 0); + pch_log_pme_internal_wake_source();
/* SMBUS Wake */ if (ps->gpe0_sts[GPE_STD] & SMB_WAK_STS) diff --git a/src/soc/intel/jasperlake/xhci.c b/src/soc/intel/jasperlake/xhci.c new file mode 100644 index 0000000..424751e --- /dev/null +++ b/src/soc/intel/jasperlake/xhci.c @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <intelblocks/xhci.h> + +#define XHCI_USB2_PORT_STATUS_REG 0x480 +#define XHCI_USB3_PORT_STATUS_REG 0x500 +#define XHCI_USB2_PORT_NUM 8 +#define XHCI_USB3_PORT_NUM 6 + +static const struct xhci_usb_info usb_info = { + .usb2_port_status_reg = XHCI_USB2_PORT_STATUS_REG, + .num_usb2_ports = XHCI_USB2_PORT_NUM, + .usb3_port_status_reg = XHCI_USB3_PORT_STATUS_REG, + .num_usb3_ports = XHCI_USB3_PORT_NUM, +}; + +const struct xhci_usb_info *soc_get_xhci_usb_info(void) +{ + return &usb_info; +}
-
Furquan Shaikh (Code Review) -
Justin TerAvest (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Patrick Georgi (Code Review)
-
Tim Wawrzynczak (Code Review)