Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41384 )
Change subject: device/pci_device: Add notion of "hidden" PCI devices ......................................................................
Patch Set 8:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41384/7/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/41384/7/src/device/pci_device.c@795 PS7, Line 795: hidden_pci_dev_ops
Is this needed? Usually, we check all the pointers for NULL.
Hmm, maybe not. With the NULL checks, I suppose it is up to the person using 'hidden' to provide an appropriate ops_pci if desired. That would also imply pci_bus_ops_pci on line 773 isn't necessary either.
https://review.coreboot.org/c/coreboot/+/41384/7/src/device/pci_device.c@122... PS7, Line 1223: * ID as if there were no device there (0xffffffff).
This is probably only true for a single device. IIRC, Intel has at least […]
Fair point. In this case, I meant the semantics of using the 'hidden' keyword in the devicetree. I'll try to make that more clear.
https://review.coreboot.org/c/coreboot/+/41384/7/src/device/pci_device.c@125... PS7, Line 1251: }
If it resembles what another function would do, why not put it into a […]
Good call, will do.