Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36221 )
Change subject: Add configurable ramstage support for minimal PCI scanning ......................................................................
Patch Set 8: Code-Review-1
(2 comments)
Please answer and address previous questions about ENV_PAYLOAD_LOADER. Intel and PCI specs and devicetree are already convoluted enough with stale implementations and ideas, further complicated by poor HyperTransport integration.
Also, you state (PCI) minimal scanning is a frequently requested feature. Please forward such communications early-on to the mailing list so others can see the design requirements.
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c@119... PS8, Line 1191: dev = pcidev_path_behind(bus, devfn); I did not see this on first review, but this probably evaluates to same 'dev' as pci_scan_get_dev() call below.
This MINIMAL_PCI_SCANNING should just directly iterate the list of bus->children instead of attempting to search for each dev.fn separately.
https://review.coreboot.org/c/coreboot/+/36221/8/src/device/pci_device.c@122... PS8, Line 1229: *prev = dev->sibling; It is pci_probe_dev() that filled vendor field. So you are effectively blocking PCI configuration access to all but PCI functions listed as mandatory? Intentional or not?