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 DID ......................................................................
Patch Set 14:
(3 comments)
Patch Set 14:
Need to update the commit message, the implementation with LCAP works \o/
Good work! I really like this new implementation
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 27: It saves us a walk through the capabilites list per device.*/
'capabilites' may be misspelled - perhaps 'capabilities'?
^
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 I didn't look into the new implementation, yet, but if we can't drop this? non-existing ports should return 0xffff anyways, so the count may be unneeded. I'm not sure about the offset, though. Will take a deeper look tomorrow
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); : same as above; maybe we can throw all possible groups in one list... non-existent ports would be skipped anyways (needs proof :) )