Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31950 )
Change subject: device/pciexp_device: Add set_subsystem() for pciexp device ......................................................................
Patch Set 5:
Patch Set 5:
Subrata, apologies, I may have done a bad review here, wasn't staring at the specs from the right angle.
So, PCI standard has register 0x0e, PCI header type. Based on the bottom 7 bits of that register, the layout of _standard_ PCI registers change! The subsystem ID we discuss here will have a _standard_ location of either 0x2c or 0x94.
Based on this information, I would now suggest a function named pci_bridge_set_subsystem() to be located in device/pci.c. It's not about PCIe, it's about the function exposing its configuration layout with PCI bridge layout.
You may want to wait for second opinions here...
Isn't the 0x94 address a very Intel implementation specific thing? i.e. according to PCI spec, there can be three header types: 0 - Normal Non-bridge devices 1 - PCI-PCI bridge 2 - PCI-Cardbus bridge
For type 0, standard location is 0x2c. For type 2, standard location is 0x40. For type 1, it is decided by the Capabilities pointer. Based on the offset of capability ID 0xD (bridge subsystem vendor/device ID capability), vendor id is located at offset 0x4 within that capability. Depending upon the implementation, capability 0xD can be at 0x90 or some other offset. I couldn't find the text where it states that the vendor id for bridge has to be at 0x94.