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:
(1 comment)
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 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.
I think in general it is okay as long as we can correctly associate the port with the controller i.e. print controller information in this driver where the port is being printed. Ultimately, it is useful for building the timeline, so as long as the semantics are consistent and clear, it should work fine.
The reason I mentioned it is if in a future Intel platform there is another XHCI controller, it will save us from having to allocate 2 more wake source types for its usb2/3 ports. I haven't seen multiple XHCI controllers in PCH on Intel platforms, but zork had 2 XHCI controllers (though it never enabled elog logging for USB ports).
I did look and I couldn't find any mention of power status for usb4 ports
Interesting. Probably something to check with Intel if it is just missing from the EDS.