Make sure to drop the references to the pwrctrl OF node and device taken
by of_pci_find_child_device() and of_find_device_by_node() respectively
when scanning the bus.

Fixes: 957f40d039a9 ("PCI/pwrctrl: Move creation of pwrctrl devices to pci_scan_device()")
Cc: stable@vger.kernel.org	# 6.15
Cc: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/pci/probe.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 4b8693ec9e4c..c5f59de790c7 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2515,9 +2515,15 @@ static struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus, in
 	struct device_node *np;
 
 	np = of_pci_find_child_device(dev_of_node(&bus->dev), devfn);
-	if (!np || of_find_device_by_node(np))
+	if (!np)
 		return NULL;
 
+	pdev = of_find_device_by_node(np);
+	if (pdev) {
+		put_device(&pdev->dev);
+		goto err_put_of_node;
+	}
+
 	/*
 	 * First check whether the pwrctrl device really needs to be created or
 	 * not. This is decided based on at least one of the power supplies
@@ -2525,17 +2531,24 @@ static struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus, in
 	 */
 	if (!of_pci_supply_present(np)) {
 		pr_debug("PCI/pwrctrl: Skipping OF node: %s\n", np->name);
-		return NULL;
+		goto err_put_of_node;
 	}
 
 	/* Now create the pwrctrl device */
 	pdev = of_platform_device_create(np, NULL, &host->dev);
 	if (!pdev) {
 		pr_err("PCI/pwrctrl: Failed to create pwrctrl device for node: %s\n", np->name);
-		return NULL;
+		goto err_put_of_node;
 	}
 
+	of_node_put(np);
+
 	return pdev;
+
+err_put_of_node:
+	of_node_put(np);
+
+	return NULL;
 }
 
 /*
-- 
2.49.1

On Mon, 21 Jul 2025 17:36:08 +0200
Johan Hovold <johan+linaro@kernel.org> wrote:

> Make sure to drop the references to the pwrctrl OF node and device taken
> by of_pci_find_child_device() and of_find_device_by_node() respectively
> when scanning the bus.
> 
> Fixes: 957f40d039a9 ("PCI/pwrctrl: Move creation of pwrctrl devices to pci_scan_device()")
> Cc: stable@vger.kernel.org	# 6.15
> Cc: Manivannan Sadhasivam <mani@kernel.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Similar to previous, I'd use a new DEFINE_FREE for the platform device and
the infrastructure is already in place for the for the of_node
(and used a lot in the core DT code).

One other comment inline.


> ---
>  drivers/pci/probe.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 4b8693ec9e4c..c5f59de790c7 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2515,9 +2515,15 @@ static struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus, in
>  	struct device_node *np;
>  
>  	np = of_pci_find_child_device(dev_of_node(&bus->dev), devfn);
> -	if (!np || of_find_device_by_node(np))
> +	if (!np)
>  		return NULL;
>  
> +	pdev = of_find_device_by_node(np);
Given we have two entirely different pdevs in here, I'd use an extra
local variable to indicate what this one is the pwctrl one created below.

> +	if (pdev) {
> +		put_device(&pdev->dev);
> +		goto err_put_of_node;
> +	}
> +
>  	/*
>  	 * First check whether the pwrctrl device really needs to be created or
>  	 * not. This is decided based on at least one of the power supplies
> @@ -2525,17 +2531,24 @@ static struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus, in
>  	 */
>  	if (!of_pci_supply_present(np)) {
>  		pr_debug("PCI/pwrctrl: Skipping OF node: %s\n", np->name);
> -		return NULL;
> +		goto err_put_of_node;
>  	}
>  
>  	/* Now create the pwrctrl device */
>  	pdev = of_platform_device_create(np, NULL, &host->dev);
>  	if (!pdev) {
>  		pr_err("PCI/pwrctrl: Failed to create pwrctrl device for node: %s\n", np->name);
> -		return NULL;
> +		goto err_put_of_node;
>  	}
>  
> +	of_node_put(np);
> +
>  	return pdev;
> +
> +err_put_of_node:
> +	of_node_put(np);
> +
> +	return NULL;
>  }
>  
>  /*

On Tue, Jul 22, 2025 at 11:08:33AM +0100, Jonathan Cameron wrote:
> On Mon, 21 Jul 2025 17:36:08 +0200
> Johan Hovold <johan+linaro@kernel.org> wrote:

> > @@ -2515,9 +2515,15 @@ static struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus, in
> >  	struct device_node *np;
> >  
> >  	np = of_pci_find_child_device(dev_of_node(&bus->dev), devfn);
> > -	if (!np || of_find_device_by_node(np))
> > +	if (!np)
> >  		return NULL;
> >  
> > +	pdev = of_find_device_by_node(np);

> Given we have two entirely different pdevs in here, I'd use an extra
> local variable to indicate what this one is the pwctrl one created below.

It's actually the same pwrctrl platform device (which may have been
created in an earlier call to the function) so using the same variable
should be fine.

Johan