Nico Huber 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 11:
(6 comments)
https://review.coreboot.org/c/coreboot/+/35985/10/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/pcie_rp.h:
https://review.coreboot.org/c/coreboot/+/35985/10/src/soc/intel/common/block... PS10, Line 21: unsigned int offset;
It's not clear what these fields represent.
Done
https://review.coreboot.org/c/coreboot/+/35985/10/src/soc/intel/common/block... PS10, Line 25: /* Zero-terminated list */
Zero in what field? Looks like it's 'slot'. I think we should be more explicit in the comments.
Done
https://review.coreboot.org/c/coreboot/+/35985/10/src/soc/intel/common/block... PS10, Line 34:
Could you please add a description?
Done
https://review.coreboot.org/c/coreboot/+/35985/10/src/soc/intel/common/block... File src/soc/intel/common/block/pcie/pcie_rp.c:
https://review.coreboot.org/c/coreboot/+/35985/10/src/soc/intel/common/block... PS10, Line 32: <
Ack
Thanks!
https://review.coreboot.org/c/coreboot/+/35985/10/src/soc/intel/common/block... PS10, Line 79: 24
there's a maximum of 24 ports depending on SKU
Done
https://review.coreboot.org/c/coreboot/+/35985/10/src/soc/intel/common/block... PS10, Line 88: group->offset + 8
Could you please explain this check?
I hope the new version explains itself. Let me know if not.