Hi Rob, On Mon, 4 Dec 2023 07:59:09 -0600 Rob Herring <robh@xxxxxxxxxx> wrote: [...] > > > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > > > index 9c2137dae429..46b252bbe500 100644 > > > --- a/drivers/pci/bus.c > > > +++ b/drivers/pci/bus.c > > > @@ -342,8 +342,6 @@ void pci_bus_add_device(struct pci_dev *dev) > > > */ > > > pcibios_bus_add_device(dev); > > > pci_fixup_device(pci_fixup_final, dev); > > > - if (pci_is_bridge(dev)) > > > - of_pci_make_dev_node(dev); > > > pci_create_sysfs_dev_files(dev); > > > pci_proc_attach_device(dev); > > > pci_bridge_d3_update(dev); > > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c > > > index 51e3dd0ea5ab..e15eaf0127fc 100644 > > > --- a/drivers/pci/of.c > > > +++ b/drivers/pci/of.c > > > @@ -31,6 +31,8 @@ int pci_set_of_node(struct pci_dev *dev) > > > return 0; > > > > > > node = of_pci_find_child_device(dev->bus->dev.of_node, dev->devfn); > > > + if (!node && pci_is_bridge(dev)) > > > + of_pci_make_dev_node(dev); > > > if (!node) > > > return 0; > > > > Maybe it is too early. > > of_pci_make_dev_node() creates a node and fills some properties based on > > some already set values available in the PCI device such as its struct resource > > values. > > We need to have some values set by the PCI infra in order to create our DT node > > with correct values. > > Indeed, that's probably the issue I'm having. In that case, > DECLARE_PCI_FIXUP_HEADER should work. That's later, but still before > device_add(). > > I think modifying sysfs after device_add() is going to race with > userspace. Userspace is notified of a new device, and then the of_node > link may or may not be there when it reads sysfs. Also, not sure if > we'll need DT modaliases with PCI devices, but they won't work if the > DT node is not set before device_add(). Ok, we can try using DECLARE_PCI_FIXUP_HEADER. On your side, is moving from DECLARE_PCI_FIXUP_EARLY to DECLARE_PCI_FIXUP_HEADER fix your QEMU unittest ? We have to note that between the pci_fixup_device(pci_fixup_header, dev) call and the device_add() call, the call to pci_set_msi_domain() is present. MSIs are not supported currently but in the future ... Related to DT modaliases, I don't think they are needed. All drivers related to PCI device should be declared as pci_driver. Correct me if I am wrong but I think that the core PCI will load the correct module without any DT modalias. Best regards, Hervé -- Hervé Codina, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
