On Wed, Oct 23, 2024 at 12:23:57PM +0200, Bartosz Golaszewski wrote: > On Tue, 22 Oct 2024 at 12:28, Manivannan Sadhasivam via B4 Relay > <devnull+manivannan.sadhasivam.linaro.org@xxxxxxxxxx> wrote: > > > > From: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> > > > > This makes the pci_bus_add_device() API easier to maintain. Also add more > > comments to the helper to describe how the devices are created. > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> > > --- > > drivers/pci/bus.c | 59 ++++++++++++++++++++++++++++++++++++++----------------- > > 1 file changed, 41 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > > index 351af581904f..c4cae1775c9e 100644 > > --- a/drivers/pci/bus.c > > +++ b/drivers/pci/bus.c > > @@ -321,6 +321,46 @@ void __weak pcibios_resource_survey_bus(struct pci_bus *bus) { } > > > > void __weak pcibios_bus_add_device(struct pci_dev *pdev) { } > > > > +/* > > + * Create pwrctl devices (if required) for the PCI devices to handle the power > > + * state. > > + */ > > +static void pci_pwrctl_create_devices(struct pci_dev *dev) > > +{ > > + struct device_node *np = dev_of_node(&dev->dev); > > + struct device *parent = &dev->dev; > > + struct platform_device *pdev; > > + > > + /* > > + * First ensure that we are starting from a PCI bridge and it has a > > + * corresponding devicetree node. > > + */ > > + if (np && pci_is_bridge(dev)) { > > + /* > > + * Now look for the child PCI device nodes and create pwrctl > > + * devices for them. The pwrctl device drivers will manage the > > + * power state of the devices. > > + */ > > + for_each_child_of_node_scoped(np, child) { > > + /* > > + * First check whether the pwrctl device really needs to > > + * be created or not. This is decided based on at least > > + * one of the power supplies being defined in the > > + * devicetree node of the device. > > + */ > > + if (!of_pci_is_supply_present(child)) { > > + pci_dbg(dev, "skipping OF node: %s\n", child->name); > > + return; > > + } > > + > > + /* Now create the pwrctl device */ > > + pdev = of_platform_device_create(child, NULL, parent); > > + if (!pdev) > > + pci_err(dev, "failed to create OF node: %s\n", child->name); > > + } > > + } > > +} > > + > > /** > > * pci_bus_add_device - start driver for a single device > > * @dev: device to add > > @@ -345,24 +385,7 @@ void pci_bus_add_device(struct pci_dev *dev) > > pci_proc_attach_device(dev); > > pci_bridge_d3_update(dev); > > > > - if (dev_of_node(&dev->dev) && pci_is_bridge(dev)) { > > - for_each_child_of_node_scoped(dn, child) { > > - /* > > - * First check whether the pwrctl device needs to be > > - * created or not. This is decided based on at least > > - * one of the power supplies being defined in the > > - * devicetree node of the device. > > - */ > > - if (!of_pci_is_supply_present(child)) { > > - pci_dbg(dev, "skipping OF node: %s\n", child->name); > > - continue; > > - } > > - > > - pdev = of_platform_device_create(child, NULL, &dev->dev); > > - if (!pdev) > > - pci_err(dev, "failed to create OF node: %s\n", child->name); > > - } > > - } > > + pci_pwrctl_create_devices(dev); > > > > /* > > * Create a device link between the PCI device and pwrctl device (if > > > > -- > > 2.25.1 > > > > > > Would it be possible to move this into drivers/pwrctl/ and provide a > header stub for when PCI_PWRCTL is disabled in Kconfig? > Unfortunately, no. Because the pwrctl drivers are modular and PCI core is built-in. So if we use the pwrctl APIs in PCI core, then it will require pwrctl to always be built-in, which we do not want. - Mani -- மணிவண்ணன் சதாசிவம்