Tim Wawrzynczak 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 requir […]
That works.
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.
yes you're right
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
I think it's a little more clear if each line doesn't make assumptions about entries that have preceded it, so I would rather keep separate elog codes for `TCSS XHCI (USB 2.0 port) | 2` for example.
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},
As long as they are represented under the same PM bit, then yeah it'll work. I would also add in the usb2/usb3 port elog entries too as I mentioned above.
and this driver can loop over each entry:
- Get SoC USB2/3 port info (BTW are there separate bits for USB4 ports?)
I did look and I couldn't find any mention of power status for usb4 ports
- Check if any of the ports are wake sources. Log Information about the controller and the port.
What do you think?
SG