Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48293 )
Change subject: mb/prodrive/hermes: Wrap UART driver by PCI device ......................................................................
Patch Set 4: Code-Review-1
(1 comment)
Patch Set 4:
I don't understand why this is necessary at all. Looks like you found a bug in sconfig you are trying to work around.
I don't know if it is a bug actually or if this is just not meant to be. When we started adding the chipset devicetrees for soc/skylake and soc/cannonlake, we encountered that problem on cannonlake.
If the PCI device on mb/hermes is wrapped by the UART driver and if also the patch for the chipset devicetree is applied, then the PCI device has two declarations in static.c.
DEVTREE_CONST struct device *DEVTREE_CONST __pci_0_19_2 = &_dev32; ... DEVTREE_CONST struct device *DEVTREE_CONST __pci_0_19_2 = &_dev70;
There was a similar problem with drivers/wifi/generic (CB:46865) as Michael mentioned. Furquan solved that by turning it around. So he moved the driver into the device. Here in this case, only the declaration for &_dev32 (from the example above) is left then, while &_dev72 is declared as a sub device.
This is a bug in sconfig as it's clearly the same PCI device (same dev function number, same parent).
https://review.coreboot.org/c/coreboot/+/48293/4/src/mainboard/prodrive/herm... File src/mainboard/prodrive/hermes/variants/baseboard/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/48293/4/src/mainboard/prodrive/herm... PS4, Line 177: device pci 19.2 on end
19.2->19.2 won't work; `device generic 0 on end` would be correct, see CB:46865 for example. […]
The device must be hidden and as it's not the 19.2 pci device the driver will not work.
-
Patrick Rudolph (Code Review)