[PATCH] PCI: kirin: fix memory leak in kirin_pcie_parse_port()

[PATCH] PCI: kirin: fix memory leak in kirin_pcie_parse_port()

[Javier Carrasco]

The conversion of this file to use the agnostic GPIO API has introduced
a new early return where the refcounts of two device nodes (parent and
child) are not decremented.

Given that the device nodes are not required outside the loops where
they are used, and to avoid potential bugs every time a new error path
is introduced to the loop, the _scoped() versions of the macros have
been applied. The bug was introduced recently, and the fix is not
relevant for old stable kernels that might not support the scoped()
variant.

Fixes: 1d38f9d89f85 ("PCI: kirin: Convert to use agnostic GPIO API")
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
This bug was found while analyzing the code and I don't have hardware to
validate it beyond compilation and static analysis. Any test with real
hardware to make sure there are no regressions is always welcome.

The dev_err() messages have not been converted into dev_err_probe() to
keep the current format, but I am open to convert them if preferred.
---
 drivers/pci/controller/dwc/pcie-kirin.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index d1f54f188e71..0a29136491b8 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -403,11 +403,10 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
 				 struct device_node *node)
 {
 	struct device *dev = &pdev->dev;
-	struct device_node *parent, *child;
 	int ret, slot, i;
 
-	for_each_available_child_of_node(node, parent) {
-		for_each_available_child_of_node(parent, child) {
+	for_each_available_child_of_node_scoped(node, parent) {
+		for_each_available_child_of_node_scoped(parent, child) {
 			i = pcie->num_slots;
 
 			pcie->id_reset_gpio[i] = devm_fwnode_gpiod_get_index(dev,
@@ -424,14 +423,13 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
 			pcie->num_slots++;
 			if (pcie->num_slots > MAX_PCI_SLOTS) {
 				dev_err(dev, "Too many PCI slots!\n");
-				ret = -EINVAL;
-				goto put_node;
+				return -EINVAL;
 			}
 
 			ret = of_pci_get_devfn(child);
 			if (ret < 0) {
 				dev_err(dev, "failed to parse devfn: %d\n", ret);
-				goto put_node;
+				return ret;
 			}
 
 			slot = PCI_SLOT(ret);
@@ -439,10 +437,8 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
 			pcie->reset_names[i] = devm_kasprintf(dev, GFP_KERNEL,
 							      "pcie_perst_%d",
 							      slot);
-			if (!pcie->reset_names[i]) {
-				ret = -ENOMEM;
-				goto put_node;
-			}
+			if (!pcie->reset_names[i])
+				return -ENOMEM;
 
 			gpiod_set_consumer_name(pcie->id_reset_gpio[i],
 						pcie->reset_names[i]);
@@ -450,11 +446,6 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
 	}
 
 	return 0;
-
-put_node:
-	of_node_put(child);
-	of_node_put(parent);
-	return ret;
 }
 
 static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,

---
base-commit: d35b2284e966c0bef3e2182a5c5ea02177dd32e4
change-id: 20240609-pcie-kirin-memleak-18c83a31d111

Best regards,
-- 
Javier Carrasco <javier.carrasco.cruz@gmail.com>

Re: [PATCH] PCI: kirin: fix memory leak in kirin_pcie_parse_port()

[Andy Shevchenko]

On Sun, Jun 09, 2024 at 12:56:14PM +0200, Javier Carrasco wrote:
> The conversion of this file to use the agnostic GPIO API has introduced
> a new early return where the refcounts of two device nodes (parent and
> child) are not decremented.
> 
> Given that the device nodes are not required outside the loops where
> they are used, and to avoid potential bugs every time a new error path
> is introduced to the loop, the _scoped() versions of the macros have
> been applied. The bug was introduced recently, and the fix is not
> relevant for old stable kernels that might not support the scoped()
> variant.

Looks reasonable to me,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] PCI: kirin: fix memory leak in kirin_pcie_parse_port()

[Manivannan Sadhasivam]

On Sun, Jun 09, 2024 at 12:56:14PM +0200, Javier Carrasco wrote:
> The conversion of this file to use the agnostic GPIO API has introduced
> a new early return where the refcounts of two device nodes (parent and
> child) are not decremented.
> 
> Given that the device nodes are not required outside the loops where
> they are used, and to avoid potential bugs every time a new error path
> is introduced to the loop, the _scoped() versions of the macros have
> been applied. The bug was introduced recently, and the fix is not
> relevant for old stable kernels that might not support the scoped()
> variant.
> 
> Fixes: 1d38f9d89f85 ("PCI: kirin: Convert to use agnostic GPIO API")
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> ---
> This bug was found while analyzing the code and I don't have hardware to
> validate it beyond compilation and static analysis. Any test with real
> hardware to make sure there are no regressions is always welcome.
> 
> The dev_err() messages have not been converted into dev_err_probe() to
> keep the current format, but I am open to convert them if preferred.

Sure, please do it in a separate patch.

- Mani

> ---
>  drivers/pci/controller/dwc/pcie-kirin.c | 21 ++++++---------------
>  1 file changed, 6 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
> index d1f54f188e71..0a29136491b8 100644
> --- a/drivers/pci/controller/dwc/pcie-kirin.c
> +++ b/drivers/pci/controller/dwc/pcie-kirin.c
> @@ -403,11 +403,10 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  				 struct device_node *node)
>  {
>  	struct device *dev = &pdev->dev;
> -	struct device_node *parent, *child;
>  	int ret, slot, i;
>  
> -	for_each_available_child_of_node(node, parent) {
> -		for_each_available_child_of_node(parent, child) {
> +	for_each_available_child_of_node_scoped(node, parent) {
> +		for_each_available_child_of_node_scoped(parent, child) {
>  			i = pcie->num_slots;
>  
>  			pcie->id_reset_gpio[i] = devm_fwnode_gpiod_get_index(dev,
> @@ -424,14 +423,13 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  			pcie->num_slots++;
>  			if (pcie->num_slots > MAX_PCI_SLOTS) {
>  				dev_err(dev, "Too many PCI slots!\n");
> -				ret = -EINVAL;
> -				goto put_node;
> +				return -EINVAL;
>  			}
>  
>  			ret = of_pci_get_devfn(child);
>  			if (ret < 0) {
>  				dev_err(dev, "failed to parse devfn: %d\n", ret);
> -				goto put_node;
> +				return ret;
>  			}
>  
>  			slot = PCI_SLOT(ret);
> @@ -439,10 +437,8 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  			pcie->reset_names[i] = devm_kasprintf(dev, GFP_KERNEL,
>  							      "pcie_perst_%d",
>  							      slot);
> -			if (!pcie->reset_names[i]) {
> -				ret = -ENOMEM;
> -				goto put_node;
> -			}
> +			if (!pcie->reset_names[i])
> +				return -ENOMEM;
>  
>  			gpiod_set_consumer_name(pcie->id_reset_gpio[i],
>  						pcie->reset_names[i]);
> @@ -450,11 +446,6 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  	}
>  
>  	return 0;
> -
> -put_node:
> -	of_node_put(child);
> -	of_node_put(parent);
> -	return ret;
>  }
>  
>  static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
> 
> ---
> base-commit: d35b2284e966c0bef3e2182a5c5ea02177dd32e4
> change-id: 20240609-pcie-kirin-memleak-18c83a31d111
> 
> Best regards,
> -- 
> Javier Carrasco <javier.carrasco.cruz@gmail.com>
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH] PCI: kirin: fix memory leak in kirin_pcie_parse_port()

[Javier Carrasco]

On 09/06/2024 12:56, Javier Carrasco wrote:
> The conversion of this file to use the agnostic GPIO API has introduced
> a new early return where the refcounts of two device nodes (parent and
> child) are not decremented.
> 
> Given that the device nodes are not required outside the loops where
> they are used, and to avoid potential bugs every time a new error path
> is introduced to the loop, the _scoped() versions of the macros have
> been applied. The bug was introduced recently, and the fix is not
> relevant for old stable kernels that might not support the scoped()
> variant.
> 
> Fixes: 1d38f9d89f85 ("PCI: kirin: Convert to use agnostic GPIO API")
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
> This bug was found while analyzing the code and I don't have hardware to
> validate it beyond compilation and static analysis. Any test with real
> hardware to make sure there are no regressions is always welcome.
> 
> The dev_err() messages have not been converted into dev_err_probe() to
> keep the current format, but I am open to convert them if preferred.
> ---
>  drivers/pci/controller/dwc/pcie-kirin.c | 21 ++++++---------------
>  1 file changed, 6 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
> index d1f54f188e71..0a29136491b8 100644
> --- a/drivers/pci/controller/dwc/pcie-kirin.c
> +++ b/drivers/pci/controller/dwc/pcie-kirin.c
> @@ -403,11 +403,10 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  				 struct device_node *node)
>  {
>  	struct device *dev = &pdev->dev;
> -	struct device_node *parent, *child;
>  	int ret, slot, i;
>  
> -	for_each_available_child_of_node(node, parent) {
> -		for_each_available_child_of_node(parent, child) {
> +	for_each_available_child_of_node_scoped(node, parent) {
> +		for_each_available_child_of_node_scoped(parent, child) {
>  			i = pcie->num_slots;
>  
>  			pcie->id_reset_gpio[i] = devm_fwnode_gpiod_get_index(dev,
> @@ -424,14 +423,13 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  			pcie->num_slots++;
>  			if (pcie->num_slots > MAX_PCI_SLOTS) {
>  				dev_err(dev, "Too many PCI slots!\n");
> -				ret = -EINVAL;
> -				goto put_node;
> +				return -EINVAL;
>  			}
>  
>  			ret = of_pci_get_devfn(child);
>  			if (ret < 0) {
>  				dev_err(dev, "failed to parse devfn: %d\n", ret);
> -				goto put_node;
> +				return ret;
>  			}
>  
>  			slot = PCI_SLOT(ret);
> @@ -439,10 +437,8 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  			pcie->reset_names[i] = devm_kasprintf(dev, GFP_KERNEL,
>  							      "pcie_perst_%d",
>  							      slot);
> -			if (!pcie->reset_names[i]) {
> -				ret = -ENOMEM;
> -				goto put_node;
> -			}
> +			if (!pcie->reset_names[i])
> +				return -ENOMEM;
>  
>  			gpiod_set_consumer_name(pcie->id_reset_gpio[i],
>  						pcie->reset_names[i]);
> @@ -450,11 +446,6 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  	}
>  
>  	return 0;
> -
> -put_node:
> -	of_node_put(child);
> -	of_node_put(parent);
> -	return ret;
>  }
>  
>  static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
> 
> ---
> base-commit: d35b2284e966c0bef3e2182a5c5ea02177dd32e4
> change-id: 20240609-pcie-kirin-memleak-18c83a31d111
> 
> Best regards,


Hi, has this patch been applied anywhere? I couldn't find it in
linux-next and pci/next.

Best regards,
Javier Carrasco

Re: [PATCH] PCI: kirin: fix memory leak in kirin_pcie_parse_port()

[Krzysztof Wilczyński]

Hello,

[...]
> Hi, has this patch been applied anywhere? I couldn't find it in
> linux-next and pci/next.

No, not yet.  It's on my TODO list.  You can always have a peek at our
Patchwork to see how are things progressing.

Meanwhile, there are always patches to be reviewed, if you feel particularly
restless, so feel free.

Thank you!

	Krzysztof

Re: [PATCH] PCI: kirin: fix memory leak in kirin_pcie_parse_port()

[Jonathan Cameron]

On Sun, 09 Jun 2024 12:56:14 +0200
Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:

> The conversion of this file to use the agnostic GPIO API has introduced
> a new early return where the refcounts of two device nodes (parent and
> child) are not decremented.
> 
> Given that the device nodes are not required outside the loops where
> they are used, and to avoid potential bugs every time a new error path
> is introduced to the loop, the _scoped() versions of the macros have
> been applied. The bug was introduced recently, and the fix is not
> relevant for old stable kernels that might not support the scoped()
> variant.
> 
> Fixes: 1d38f9d89f85 ("PCI: kirin: Convert to use agnostic GPIO API")
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Diff on this on is irritating as it doesn't actually show the
buggy code...  Ah well.

Change is valid, but one suggestion inline.

Looks like it's queued now already, but if not.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>


> ---
> This bug was found while analyzing the code and I don't have hardware to
> validate it beyond compilation and static analysis. Any test with real
> hardware to make sure there are no regressions is always welcome.
> 
> The dev_err() messages have not been converted into dev_err_probe() to
> keep the current format, but I am open to convert them if preferred.
> ---
>  drivers/pci/controller/dwc/pcie-kirin.c | 21 ++++++---------------
>  1 file changed, 6 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
> index d1f54f188e71..0a29136491b8 100644
> --- a/drivers/pci/controller/dwc/pcie-kirin.c
> +++ b/drivers/pci/controller/dwc/pcie-kirin.c
> @@ -403,11 +403,10 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  				 struct device_node *node)
>  {
>  	struct device *dev = &pdev->dev;
> -	struct device_node *parent, *child;
>  	int ret, slot, i;
>  
> -	for_each_available_child_of_node(node, parent) {
> -		for_each_available_child_of_node(parent, child) {
> +	for_each_available_child_of_node_scoped(node, parent) {
> +		for_each_available_child_of_node_scoped(parent, child) {
>  			i = pcie->num_slots;
>  
>  			pcie->id_reset_gpio[i] = devm_fwnode_gpiod_get_index(dev,
> @@ -424,14 +423,13 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  			pcie->num_slots++;
>  			if (pcie->num_slots > MAX_PCI_SLOTS) {
>  				dev_err(dev, "Too many PCI slots!\n");
> -				ret = -EINVAL;
> -				goto put_node;
> +				return -EINVAL;
Perhaps a future change, but this would be nicer as
				return dev_err_probe(dev, -EINVAL,
						     "Too many PCI slots!\n");
Maybe as part of a general change to this driver to use
dev_err_probe() for all the error prints in paths only called
from probe().

>  			}
>  
>  			ret = of_pci_get_devfn(child);
>  			if (ret < 0) {
>  				dev_err(dev, "failed to parse devfn: %d\n", ret);
> -				goto put_node;
> +				return ret;
>  			}
>  
>  			slot = PCI_SLOT(ret);
> @@ -439,10 +437,8 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  			pcie->reset_names[i] = devm_kasprintf(dev, GFP_KERNEL,
>  							      "pcie_perst_%d",
>  							      slot);
> -			if (!pcie->reset_names[i]) {
> -				ret = -ENOMEM;
> -				goto put_node;
> -			}
> +			if (!pcie->reset_names[i])
> +				return -ENOMEM;
>  
>  			gpiod_set_consumer_name(pcie->id_reset_gpio[i],
>  						pcie->reset_names[i]);
> @@ -450,11 +446,6 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  	}
>  
>  	return 0;
> -
> -put_node:
> -	of_node_put(child);
> -	of_node_put(parent);
> -	return ret;
>  }
>  
>  static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
> 
> ---
> base-commit: d35b2284e966c0bef3e2182a5c5ea02177dd32e4
> change-id: 20240609-pcie-kirin-memleak-18c83a31d111
> 
> Best regards,

Re: [PATCH] PCI: kirin: fix memory leak in kirin_pcie_parse_port()

[Javier Carrasco]

On 05/07/2024 12:18, Jonathan Cameron wrote:
> On Sun, 09 Jun 2024 12:56:14 +0200
> Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
> 
>> The conversion of this file to use the agnostic GPIO API has introduced
>> a new early return where the refcounts of two device nodes (parent and
>> child) are not decremented.
>>
>> Given that the device nodes are not required outside the loops where
>> they are used, and to avoid potential bugs every time a new error path
>> is introduced to the loop, the _scoped() versions of the macros have
>> been applied. The bug was introduced recently, and the fix is not
>> relevant for old stable kernels that might not support the scoped()
>> variant.
>>
>> Fixes: 1d38f9d89f85 ("PCI: kirin: Convert to use agnostic GPIO API")
>> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> Diff on this on is irritating as it doesn't actually show the
> buggy code...  Ah well.
> 
> Change is valid, but one suggestion inline.
> 
> Looks like it's queued now already, but if not.
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> 
>> ---
>> This bug was found while analyzing the code and I don't have hardware to
>> validate it beyond compilation and static analysis. Any test with real
>> hardware to make sure there are no regressions is always welcome.
>>
>> The dev_err() messages have not been converted into dev_err_probe() to
>> keep the current format, but I am open to convert them if preferred.
>> ---
>>  drivers/pci/controller/dwc/pcie-kirin.c | 21 ++++++---------------
>>  1 file changed, 6 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
>> index d1f54f188e71..0a29136491b8 100644
>> --- a/drivers/pci/controller/dwc/pcie-kirin.c
>> +++ b/drivers/pci/controller/dwc/pcie-kirin.c
>> @@ -403,11 +403,10 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>>  				 struct device_node *node)
>>  {
>>  	struct device *dev = &pdev->dev;
>> -	struct device_node *parent, *child;
>>  	int ret, slot, i;
>>  
>> -	for_each_available_child_of_node(node, parent) {
>> -		for_each_available_child_of_node(parent, child) {
>> +	for_each_available_child_of_node_scoped(node, parent) {
>> +		for_each_available_child_of_node_scoped(parent, child) {
>>  			i = pcie->num_slots;
>>  
>>  			pcie->id_reset_gpio[i] = devm_fwnode_gpiod_get_index(dev,
>> @@ -424,14 +423,13 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>>  			pcie->num_slots++;
>>  			if (pcie->num_slots > MAX_PCI_SLOTS) {
>>  				dev_err(dev, "Too many PCI slots!\n");
>> -				ret = -EINVAL;
>> -				goto put_node;
>> +				return -EINVAL;
> Perhaps a future change, but this would be nicer as
> 				return dev_err_probe(dev, -EINVAL,
> 						     "Too many PCI slots!\n");
> Maybe as part of a general change to this driver to use
> dev_err_probe() for all the error prints in paths only called
> from probe().
> 

Yeah, it seems that other paths that have nothing to do with this fix
would require the same modification.

Best regards,
Javier Carrasco

Re: [PATCH] PCI: kirin: fix memory leak in kirin_pcie_parse_port()

[Krzysztof Wilczyński]

Hello,

[...]
> Looks like it's queued now already, but if not.
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

No worries.  Updated the commit to add your review.

Thank you!

	Krzysztof

Re: [PATCH] PCI: kirin: fix memory leak in kirin_pcie_parse_port()

[Krzysztof Wilczyński]

> The conversion of this file to use the agnostic GPIO API has introduced
> a new early return where the refcounts of two device nodes (parent and
> child) are not decremented.
> 
> Given that the device nodes are not required outside the loops where
> they are used, and to avoid potential bugs every time a new error path
> is introduced to the loop, the _scoped() versions of the macros have
> been applied. The bug was introduced recently, and the fix is not
> relevant for old stable kernels that might not support the scoped()
> variant.

Applied to controller/kirin, thank you!

[1/1] PCI: kirin: Fix memory leak in kirin_pcie_parse_port()
      https://git.kernel.org/pci/pci/c/1a164371b362

	Krzysztof

Re: [PATCH] PCI: kirin: fix memory leak in kirin_pcie_parse_port()

[Bjorn Helgaas]

On Sun, Jun 09, 2024 at 12:56:14PM +0200, Javier Carrasco wrote:
> The conversion of this file to use the agnostic GPIO API has introduced
> a new early return where the refcounts of two device nodes (parent and
> child) are not decremented.
> 
> Given that the device nodes are not required outside the loops where
> they are used, and to avoid potential bugs every time a new error path
> is introduced to the loop, the _scoped() versions of the macros have
> been applied. The bug was introduced recently, and the fix is not
> relevant for old stable kernels that might not support the scoped()
> variant.
> 
> Fixes: 1d38f9d89f85 ("PCI: kirin: Convert to use agnostic GPIO API")
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

FYI, I moved this to the pci/controller/gpio patch just before
1d38f9d89f85.  That way there's no bisection hole and we don't need to
complicate this commit log with details about the bug because the bug
never exists.

> ---
> This bug was found while analyzing the code and I don't have hardware to
> validate it beyond compilation and static analysis. Any test with real
> hardware to make sure there are no regressions is always welcome.
> 
> The dev_err() messages have not been converted into dev_err_probe() to
> keep the current format, but I am open to convert them if preferred.
> ---
>  drivers/pci/controller/dwc/pcie-kirin.c | 21 ++++++---------------
>  1 file changed, 6 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
> index d1f54f188e71..0a29136491b8 100644
> --- a/drivers/pci/controller/dwc/pcie-kirin.c
> +++ b/drivers/pci/controller/dwc/pcie-kirin.c
> @@ -403,11 +403,10 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  				 struct device_node *node)
>  {
>  	struct device *dev = &pdev->dev;
> -	struct device_node *parent, *child;
>  	int ret, slot, i;
>  
> -	for_each_available_child_of_node(node, parent) {
> -		for_each_available_child_of_node(parent, child) {
> +	for_each_available_child_of_node_scoped(node, parent) {
> +		for_each_available_child_of_node_scoped(parent, child) {
>  			i = pcie->num_slots;
>  
>  			pcie->id_reset_gpio[i] = devm_fwnode_gpiod_get_index(dev,
> @@ -424,14 +423,13 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  			pcie->num_slots++;
>  			if (pcie->num_slots > MAX_PCI_SLOTS) {
>  				dev_err(dev, "Too many PCI slots!\n");
> -				ret = -EINVAL;
> -				goto put_node;
> +				return -EINVAL;
>  			}
>  
>  			ret = of_pci_get_devfn(child);
>  			if (ret < 0) {
>  				dev_err(dev, "failed to parse devfn: %d\n", ret);
> -				goto put_node;
> +				return ret;
>  			}
>  
>  			slot = PCI_SLOT(ret);
> @@ -439,10 +437,8 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  			pcie->reset_names[i] = devm_kasprintf(dev, GFP_KERNEL,
>  							      "pcie_perst_%d",
>  							      slot);
> -			if (!pcie->reset_names[i]) {
> -				ret = -ENOMEM;
> -				goto put_node;
> -			}
> +			if (!pcie->reset_names[i])
> +				return -ENOMEM;
>  
>  			gpiod_set_consumer_name(pcie->id_reset_gpio[i],
>  						pcie->reset_names[i]);
> @@ -450,11 +446,6 @@ static int kirin_pcie_parse_port(struct kirin_pcie *pcie,
>  	}
>  
>  	return 0;
> -
> -put_node:
> -	of_node_put(child);
> -	of_node_put(parent);
> -	return ret;
>  }
>  
>  static long kirin_pcie_get_resource(struct kirin_pcie *kirin_pcie,
> 
> ---
> base-commit: d35b2284e966c0bef3e2182a5c5ea02177dd32e4
> change-id: 20240609-pcie-kirin-memleak-18c83a31d111
> 
> Best regards,
> -- 
> Javier Carrasco <javier.carrasco.cruz@gmail.com>
>