Arthur Heymans 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 18:
(1 comment)
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 83: if (CONFIG(SKYLAKE_SOC_PCH_H)) : pcie_rp_update_devicetree(pch_h_rp_groups); : else : pcie_rp_update_devicetree(pch_lp_rp_groups); :
We already do that for every port. If it wasn't set to root port, it would we invalid to read LCAP :)
\o/ ofc. I read the datasheet but didn't look at the code again before commenting :D thanks
Well, we know that LP doesn't have 1b.*, so it's safe. One version is more explicit, the other saves us a few lines. I have no strong preference.
Since we already have the checks I mentioned, I would prefer having just one list, but having two isn't bad, too. I don't know. @Arthur what do you say?
I don't think it matters much here, but if you ever want to replace FSP and do the port mapping yourself you probably need to know how much root ports you have. That's a big IF, so I think either ways are fine.