Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31982 )
Change subject: {northbridge, soc, southbridge}/intel: Make use of pci_dev_set_subsystem() ......................................................................
Patch Set 1:
(2 comments)
Looked good, just needs a decision if PCI SSID 0:0 is ever allowed.
https://review.coreboot.org/#/c/31982/1/src/northbridge/intel/fsp_rangeley/n... File src/northbridge/intel/fsp_rangeley/northbridge.c:
https://review.coreboot.org/#/c/31982/1/src/northbridge/intel/fsp_rangeley/n... PS1, Line 184: We might want to copy this into pci_dev_set_subsystem() as I believe it is incorrect to set SSID to 0000:0000. Wait for second opinion on this one before changing.
https://review.coreboot.org/#/c/31982/1/src/soc/intel/baytrail/chip.c File src/soc/intel/baytrail/chip.c:
https://review.coreboot.org/#/c/31982/1/src/soc/intel/baytrail/chip.c@76 PS1, Line 76: .set_subsystem = &pci_dev_set_subsystem, You could drop that '&' as redundant, but don't bother fixing it here.
We might change the callsites such that when dev->ops or dev->ops->set_subsystem are NULL, we would call pci_dev_set_subsystem() nevertheless. I am undecided about that.