Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31999 )
Change subject: soc/intel/common: Move support to log XHCI wake events ......................................................................
Patch Set 1:
(7 comments)
https://review.coreboot.org/#/c/31999/1/src/soc/intel/common/Kconfig File src/soc/intel/common/Kconfig:
https://review.coreboot.org/#/c/31999/1/src/soc/intel/common/Kconfig@71 PS1, Line 71: SOC_INTEL_COMMON_ELOG_XHCI Move to soc/intel/common/block/xhci/Kconfig
https://review.coreboot.org/#/c/31999/1/src/soc/intel/common/elog_xhci.h File src/soc/intel/common/elog_xhci.h:
PS1: Changes in this file can be moved to soc/intel/common/block/include/intelblocks/xhci.h
https://review.coreboot.org/#/c/31999/1/src/soc/intel/common/elog_xhci.h@23 PS1, Line 23: usb2_port_status_reg Can you please add a comment indicating that this is the offset of the USB2 port status register within memory mapped address space for XHCI controller?
https://review.coreboot.org/#/c/31999/1/src/soc/intel/common/elog_xhci.h@30 PS1, Line 30: bool Comment indicating what the return value means?
https://review.coreboot.org/#/c/31999/1/src/soc/intel/common/elog_xhci.c File src/soc/intel/common/elog_xhci.c:
PS1: This file should be moved to soc/intel/common/block/xhci/elog.c
https://review.coreboot.org/#/c/31999/1/src/soc/intel/skylake/elog.c File src/soc/intel/skylake/elog.c:
https://review.coreboot.org/#/c/31999/1/src/soc/intel/skylake/elog.c@47 PS1, Line 47: struct const
https://review.coreboot.org/#/c/31999/1/src/soc/intel/skylake/elog.c@127 PS1, Line 127: PCH_DEV_XHCI I see other uses of PCH_DEV_* in soc/intel/common/block. So, it might be okay to just assume that PCH_DEV_XHCI is provided by every SoC using that driver and not pass it in?