Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47411 )
Change subject: soc/intel/common: Adapt XHCI elog driver for reuse ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47411/3/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/xhci.h:
https://review.coreboot.org/c/coreboot/+/47411/3/src/soc/intel/common/block/... PS3, Line 45: void If you pass in pci_devfn_t here, then a separate `soc_get_north_xhci_usb_info()` shouldn't be required. Basically, the driver doesn't have to care about where the controller lives as long as it can use xhci_devfn as the handle.
https://review.coreboot.org/c/coreboot/+/47411/3/src/soc/intel/common/block/... File src/soc/intel/common/block/xhci/elog.c:
https://review.coreboot.org/c/coreboot/+/47411/3/src/soc/intel/common/block/... PS3, Line 125: ELOG_WAKE_SOURCE_PME_XHCI_USB_3 I think this will have to be accepted as an input from the caller.
BTW, this makes me think - should we just reuse ELOG_WAKE_SOURCE_PME_XHCI_USB_3 and ELOG_WAKE_SOURCE_PME_XHCI_USB_2 and differentiate using XHCI controller?
i.e. eventlog would look something like this 8 | ... | S0ix Enter 9 | ... | S0ix Exit 10 | ... | Wake Source | PME - XHCI | 0 11 | ... | Wake Source | PME - XHCI (USB 2.0 port) | 8 12 | ... | S0ix Enter 13 | ... | S0ix Exit 14 | ... | Wake Source | PME - TCSS XHCI | 0 15 | ... | Wake Source | PME - XHCI (USB 2.0 port) | 2 16 | ... | Wake Source | PME - XHCI (USB 3.0 port) | 3
It will probably require passing in a table of XHCI controllers from SoC to this driver { { PCH_DEVFN_XHCI, ELOG_WAKE_SOURCE_PME_XHCI}, { PCH_DEVFN_TCSS_XHCI, ELOG_WAKE_SOURCE_PME_TCSS_XHCI}, }
and this driver can loop over each entry: 1. Get SoC USB2/3 port info (BTW are there separate bits for USB4 ports?) 2. Check if any of the ports are wake sources. Log Information about the controller and the port.
What do you think?