Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35985 )
Change subject: soc/intel: Implement PCIe RP devicetree update based on LCAP ......................................................................
Patch Set 19:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35985/19/src/soc/intel/skylake/chip... File src/soc/intel/skylake/chip.c:
https://review.coreboot.org/c/coreboot/+/35985/19/src/soc/intel/skylake/chip... PS19, Line 102: unsigned int inc = PCI_DEVFN(0, 1); I really think this is arithmetic on devfn is not good. We shouldn't be using the underlying encoding. We can fix it later, but these assumptions (along with all the pci.devfn directly accesses) leads to harder to change code.
https://review.coreboot.org/c/coreboot/+/35985/19/src/soc/intel/skylake/chip... PS19, Line 102: unsigned int inc Why isn't inc marked const since it's never manipulated again?
https://review.coreboot.org/c/coreboot/+/35985/19/src/soc/intel/skylake/chip... PS19, Line 133: func0->path.pci.devfn = dev->path.pci.devfn; Why is the swap necessary?
https://review.coreboot.org/c/coreboot/+/35985/19/src/soc/intel/skylake/chip... PS19, Line 140: static void pcie_override_devicetree_after_silicon_init(void) Is this file just showing an alternative approach to the common one?