On Thu, Feb 01, 2024 at 04:55:29PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx> > > In order to introduce PCI power-sequencing, Please provide a proper problem description. > we need to create platform And properly express why this is a "need". > devices for child nodes of the port node. They will get matched against > the pwrseq drivers That's not what happens in your code, the child nodes of the bridge node in DeviceTree will match against arbitrary platform_drivers. I also would like this commit message to express that the job of the matched device is to: 1) power up said device, followed by triggering a scan on the parent PCI bus during it's probe function. 2) power down said device, during its remove function. > (if one exists) and then the actual PCI device will > reuse the node once it's detected on the bus. I think the "reuse" deserves to be clarified as there will be both a pci and a platform device associated with the same of_node. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx> > --- > drivers/pci/bus.c | 9 ++++++++- > drivers/pci/remove.c | 2 ++ > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > index 826b5016a101..17ab41094c4e 100644 > --- a/drivers/pci/bus.c > +++ b/drivers/pci/bus.c > @@ -12,6 +12,7 @@ > #include <linux/errno.h> > #include <linux/ioport.h> > #include <linux/of.h> > +#include <linux/of_platform.h> > #include <linux/proc_fs.h> > #include <linux/slab.h> > > @@ -342,8 +343,14 @@ 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)) > + if (pci_is_bridge(dev)) { > of_pci_make_dev_node(dev); > + retval = of_platform_populate(dev->dev.of_node, NULL, NULL, > + &dev->dev); I'm not familiar enough with the ins and outs of the PCI code. Can you confirm that there are no problems with this (possibly) calling pci_rescan_bus() before the bridge device is fully initialized below? Regards, Bjorn > + if (retval) > + pci_err(dev, "failed to populate child OF nodes (%d)\n", > + retval); > + } > pci_create_sysfs_dev_files(dev); > pci_proc_attach_device(dev); > pci_bridge_d3_update(dev); > diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c > index d749ea8250d6..fc9db2805888 100644 > --- a/drivers/pci/remove.c > +++ b/drivers/pci/remove.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0 > #include <linux/pci.h> > #include <linux/module.h> > +#include <linux/of_platform.h> > #include "pci.h" > > static void pci_free_resources(struct pci_dev *dev) > @@ -22,6 +23,7 @@ static void pci_stop_dev(struct pci_dev *dev) > device_release_driver(&dev->dev); > pci_proc_detach_device(dev); > pci_remove_sysfs_dev_files(dev); > + of_platform_depopulate(&dev->dev); > of_pci_remove_node(dev); > > pci_dev_assign_added(dev, false); > -- > 2.40.1 >