Attention is currently required from: Tim Crawford, Jeremy Soller, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49104 )
Change subject: soc/intel/cannonlake: Allow setting PCIe subsystem IDs after FSP SiliconInit ......................................................................
Patch Set 7:
(1 comment)
File src/soc/intel/cannonlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/49104/comment/a36a32df_5dc8cd62 PS7, Line 582: params->SiNumberOfSsidTableEntry = i;
Note that this approach will result in FSP programming the default SVID/SSID values when neither xHC […]
Meh, you are right.
It would be better to have only the lines that dereference the device node inside the if, e.g.
ssid_table[i].reg = PCI_SUBSYSTEM_VENDOR_ID; ssid_table[i].device = PCI_SLOT(PCH_DEVFN_XHCI); ssid_table[i].function = PCI_FUNC(PCH_DEVFN_XHCI); dev = pcidev_path_on_root(PCH_DEVFN_XHCI); if (dev) { ssid_table[i].svid = dev->subsystem_vendor; ssid_table[i].ssid = dev->subsystem_device; } ++i;
That would ensure that we always have the two entries, and if the devices are disabled, the entries shouldn't hurt.