Re: [PATCH] powerpc/pci: Remove useless null check before call of_node_put()

[Tyrel Datwyler <tyreld@linux.ibm.com>]

On 4/20/22 19:52, Haowen Bai wrote:
> No need to add null check before call of_node_put(), since the
> implementation of of_node_put() has done it.
> 
> Signed-off-by: Haowen Bai <baihaowen@meizu.com>
> ---
>  arch/powerpc/kernel/pci_dn.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> index 61571ae23953..ba3bbc9bec2d 100644
> --- a/arch/powerpc/kernel/pci_dn.c
> +++ b/arch/powerpc/kernel/pci_dn.c
> @@ -357,8 +357,8 @@ void pci_remove_device_node_info(struct device_node *dn)
> 
>  	/* Drop the parent pci_dn's ref to our backing dt node */
>  	parent = of_get_parent(dn);
> -	if (parent)
> -		of_node_put(parent);
> +
> +	of_node_put(parent);

This whole block of code looks useless, or suspect. Examining the rest of the
code for this function this is the only place that parent is referenced. The
of_get_parent() call returns the parent with its refcount incremented, and then
we turn around and call of_node_put() which drops that reference we just took.
The comment doesn't do what it says it does. If we really need to drop a
previous reference to the parent device node this code block would need to call
of_node_put() twice on parent to accomplish that.

A closer examination is required to determine if what the comment says we need
to do is required. If it is then the code as it exists today is leaking that
reference AFAICS.

-Tyrel

> 
>  	/*
>  	 * At this point we *might* still have a pci_dev that was