Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41899 )
Change subject: device/xhci: Add an xhci driver to enumerate capabilities ......................................................................
Patch Set 1: Code-Review+1
(4 comments)
https://review.coreboot.org/c/coreboot/+/41899/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/41899/1//COMMIT_MSG@10 PS1, Line 10: generating the ACPI device nodes. I always wonder, if coreboot has code to generate the ACPI code, why the OS/payload can’t do it itself.
https://review.coreboot.org/c/coreboot/+/41899/1/src/device/xhci.c File src/device/xhci.c:
https://review.coreboot.org/c/coreboot/+/41899/1/src/device/xhci.c@42 PS1, Line 42: ext_caps_offset = read16(bar + XHCI_HCCPARAMS1_XECP); Should a warning be printed, if that reads back 0?
https://review.coreboot.org/c/coreboot/+/41899/1/src/device/xhci.c@66 PS1, Line 66: ext_caps_offset = 0; So this basically terminates the while loop? Why not use `break`?
https://review.coreboot.org/c/coreboot/+/41899/1/src/device/xhci.c@68 PS1, Line 68: Should some debug prints be added to print the capabilities for debugging? The Linux driver allows that, I believe.