Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44915 )
Change subject: drivers/intel/usb4: Add driver for USB4 PCIe root port ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44915/2/src/drivers/intel/usb4/pcie... File src/drivers/intel/usb4/pcie/chip.h:
https://review.coreboot.org/c/coreboot/+/44915/2/src/drivers/intel/usb4/pcie... PS2, Line 9: USB port number
Is this the port number as assigned in hardware? Or is this just a unique number assigned for each p […]
as far as I can tell this is defined in the soc at a platform level but not in any place that I've seen that we can read it from. this was in the static asl before and this driver tries to move it to the chipset.cb.
maybe a comment would be useful to indicate it is expected to be set by the soc and not the mainboard.
https://review.coreboot.org/c/coreboot/+/44915/2/src/drivers/intel/usb4/pcie... File src/drivers/intel/usb4/pcie/pcie.c:
https://review.coreboot.org/c/coreboot/+/44915/2/src/drivers/intel/usb4/pcie... PS2, Line 23: if (!config->usb4_port)
Should there be an error printed in case usb4_port device is not found?
I didn't initially because these should be set at the soc level and not be an issue at the mainboard, but it doesn't hurt to have more output for the next soc implementation that forgets.