Nico Huber 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
-1 just because I saw a premature +2 along the lines.
About the implementation: If we need special semantics (if not, see alternatives below), I would prefer a clean, second implementation of pci_scan_bus() instead of making the current one even harder to follow.
About the devicetree notation: * `mandatory` seems orthogonal to the last addition `hidden`. How would I specify a device that is mandatory and should be used in (hidden) ACPI mode? * What is the point of specifying a device in the devicetree if it isn't mandatory? iow, do we need another flag at all? wouldn't the existing .on_mainboard suffice? If the point is to avoid scanning addresses that might return nothing, it should.
About alternatives: For most platforms, we know that bus 0 doesn't (shouldn't?) contain surprises. We had a little discussion about handling chipset embedded devices better lately. If we would specify bus 0 devices exactly, per platform, the mainboard devicetree would only have to mention details (e.g. disable that, add downstream devices there...). And for PCIe root ports, we already know that we only have to scan for downstream device 0 (and not 1..1f). I'm not very familiar with PCIe capabilities, but maybe there is even a hint in downstream bridges if we have to scan all 32 device numbers.
So how about, instead of adding a feature on top, for anonymous folks that frequently request minimal scanning, we look for a generic way first to implement it for everyone?