[M] Change in coreboot[master]: soc/amd/common/xhci: Add support for logging XHCI wake events
Attention is currently required from: Robert Zieba, Jason Glenesk, Raul Rangel, Matt DeVillier, Martin Roth, Fred Reitberger, Karthik Ramasubramanian, Felix Held.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67936 )
Change subject: soc/amd/common/xhci: Add support for logging XHCI wake events ......................................................................
Patch Set 10:
(7 comments)
File src/soc/amd/common/block/include/amdblocks/xhci.h:
https://review.coreboot.org/c/coreboot/+/67936/comment/bd680bf3_a36f2003 PS10, Line 11: <stdint.h> Isn't `size_t` in `<stddef.h>`? You can simply include `<types.h>` for convenience.
https://review.coreboot.org/c/coreboot/+/67936/comment/833b4243_05fa334e PS10, Line 13: #define XHCI_GEVENT GEVENT_31 Hmmm, what does this indirection achieve?
File src/soc/amd/common/block/xhci/elog.c:
https://review.coreboot.org/c/coreboot/+/67936/comment/4af5829c_e8932855 PS10, Line 20: base Instead of incrementing the value of `base`, would it be clearer to write this as `base + i * PORTSC_STRIDE`? You could also make a parametrized macro:
#define PORTSC_ADDR(port) (PORTSC_OFFSET + (port) * PORTSC_STRIDE)
https://review.coreboot.org/c/coreboot/+/67936/comment/8cac42ae_09e2aa29 PS10, Line 66: %d nit: %u for unsigned types
https://review.coreboot.org/c/coreboot/+/67936/comment/e88b76e4_969be4d9 PS10, Line 67: context->controller reuse `controller` local variable?
https://review.coreboot.org/c/coreboot/+/67936/comment/3d10f471_497c7003 PS10, Line 93: %d nit: %u for unsigned types
https://review.coreboot.org/c/coreboot/+/67936/comment/78c0a430_7cf55a42 PS10, Line 93: %#lx Hmmm, one would use `PRIxPTR` to display `uintptr_t` values
-
Angel Pons (Code Review)