Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31900 )
Change subject: device: Rename scan_static_bus() -> enable_static_devices() ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31900/1/src/soc/intel/common/block/hda/hda.c File src/soc/intel/common/block/hda/hda.c:
https://review.coreboot.org/#/c/31900/1/src/soc/intel/common/block/hda/hda.c... PS1, Line 72: .scan_bus = enable_static_devices,
We are dealing with static bus with at least one child? So we can evaluate the path type from that and pick scan function accordingly?
For "static", i.e. dt pre-enumerated, buses, yes. But it's not that our static scan functions would differ much. I see three cases:
1. enable_static_devices() * enables all downstream devices
2. scan_generic_bus() * numbers downstream links/buses * roughly same impl. as enable_static_devices()
3. root_dev_scan_bus() etc. * call enable_static_devices() * scan downstream buses
With the exception of potentially changing bus numbers, we could just squash them into one implementation. If my assessment is correct, the only troublemaker is dev_find_ slot_on_smbus(). It's not used much. Maybe I'll check the devicetrees if they are susceptible to bus number changes.
If necessary, we could then optimize based on the path type, e.g. skip scan_bridges() call for types where we don't expect further bridges.
Which brings us to another question; do we ever actually define an override on .scan_bus, or do we currently only use it to determine a scan function to match the path type?
Yes, we do. A grep shows me at least three different imple- mentations for path type PCI (assuming HT is reusing that type, too). Doesn't make you wrong; could also be that our enumeration of path types is incomplete?