Overall LGTM. I think we only need to avoid the unconditional logging of wake source for controller.
Patch set 5:Code-Review +1
2 comments:
File src/soc/intel/common/block/include/intelblocks/xhci.h:
and a usb_info
* struct.
Leftover comment? Or were you intending to add usb_info pointer to the above structure as well?
File src/soc/intel/common/block/xhci/elog.c:
Patch Set #5, Line 111: elog_add_event_wake(wake_info[i].elog_wake_type_host, 0);
Isn't this being logged unconditionally now?
The problem we run into here is: we cannot just rely on the XHCI controller PMCS since that gets cleared in case of S0ix. So, the host controller wake event needs to be logged only if any of the USB2 or USB3 ports are wake sources. That makes it a little icky.
xhci_stash_host_wake(wake_info[i].elog_wake_type_host);
and within xhci_port_wake_check(...), elog_add_event_wake() will have to be changed to:
xhci_port_log_wake() which calls xhci_pop_host_wake() and if it is not -1 (or some undefined value), then log host wake first and then the port.
To view, visit change 47411. To unsubscribe, or for help writing mail filters, visit settings.