Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40474 )
Change subject: device: Add a helper to find device behind a PCI bridge device ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/40474/4/src/device/device_const.c File src/device/device_const.c:
https://review.coreboot.org/c/coreboot/+/40474/4/src/device/device_const.c@2... PS4, Line 230: bridge->path.type != DEVICE_PATH_PCI
Isn't it enough if the found device is PCI? I know with our current […]
I am not sure I understand the use case here. Are you saying that a PCI device can live under a bridge which is not really a PCI device?
https://review.coreboot.org/c/coreboot/+/40474/4/src/device/device_const.c@2... PS4, Line 230: bridge == NULL
maybe: !bridge
Sure, I can update that.
https://review.coreboot.org/c/coreboot/+/40474/4/src/device/device_const.c@2... PS4, Line 233: link_list
`link_list` is a list ;) calling it like this only scans the first entry. Should we walk it? […]
The whole concept of multiple links for a PCI bridge is weird. I don't understand how that would really work. Also, I don't see any PCI bridges supporting multiple links(buses) under them. It is currently used only for some cpu cluster device for AMD SoCs.
In my opinion, let's not add more logic for checking multiple links here unless it is a valid scenario. (Probably, the right thing to do would be for some one to evaluate whether the whole multiple links/buses is something that needs to be redone differently. That is for a different change though.).
For now, I think we can just go with the head of the linked list of links and search for device under that. I can add a comment if you think it is helpful.