Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35985 )
Change subject: intel/skylake: Implement PCIe RP devicetree update based on LCAP ......................................................................
Patch Set 17:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35985/14/src/soc/intel/skylake/chip... File src/soc/intel/skylake/chip.c:
https://review.coreboot.org/c/coreboot/+/35985/14/src/soc/intel/skylake/chip... PS14, Line 49: offset = 8, .count = 4
Dropped `offset`
curious, do you know if 8 ports is spec or intel? what do you think about assuming it's always 8?
https://review.coreboot.org/c/coreboot/+/35985/14/src/soc/intel/skylake/chip... PS14, Line 83: if (CONFIG(SKYLAKE_SOC_PCH_H)) : pcie_rp_update_devicetree(pch_h_rp_groups); : else : pcie_rp_update_devicetree(pch_lp_rp_groups); :
Hmmm, it's really just 5 additional PCI-config reads. But we'd always […]
yeah, I've been thinking about that assumption, too. Since the list is at SoC level I would guess we won't ever have a problem here. At least for current SoCs the port IDs do not overlap / have another meaning irt H vs. LP.
For PCH-H you already have that set the count from 4 to 8. So when this is done for PCH-LP, too, we only have one difference: the additional group. So it would be easy to have exactly one list (actually that would be the PCH-H one).