Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35985 )
Change subject: intel/skylake: Implement PCIe RP devicetree update based on DID ......................................................................
Patch Set 14:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35985/14/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/pcie_rp.c:
https://review.coreboot.org/c/coreboot/+/35985/14/src/soc/intel/common/block... PS14, Line 28: #define INTEL_RP_CLIST_XCAP 0x40 Individual PCIe capabilities aren't guaranteed to be at the same offset. Do we want to maintain this assumption?
https://review.coreboot.org/c/coreboot/+/35985/14/src/soc/intel/common/block... PS14, Line 44: /* Read 1-based absolute port number: */ Could you expand this comment indicating the original port number is in this field of the capabilities? We're inherently relying on that to hold true for this scheme to work. I think it could use an expanded comment to that effect.
https://review.coreboot.org/c/coreboot/+/35985/14/src/soc/intel/common/block... PS14, Line 47: port_num 1-based comment above means it starts at 1 and counts up? Wouldn't this check be invalid because offset is 0 based? e.g. the 2nd group would be offfset 8. But the first root port as read out of the capability field would be 9 ?
https://review.coreboot.org/c/coreboot/+/35985/14/src/soc/intel/common/block... PS14, Line 97: "Remapping PCIe Root Port #%u from %s to new function number %u.\n", Is it helpful to have the old function number in this message?