Re: [PATCH v3 5/5] powerpc/pseries: use of_property_alloc/free() and of_node_alloc()

From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: "Clément Léger" <clement.leger@bootlin.com>,
	"Michael Ellerman" <mpe@ellerman.id.au>,
	"Benjamin Herrenschmidt" <benh@kernel.crashing.org>,
	"Paul Mackerras" <paulus@samba.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Frank Rowand" <frowand.list@gmail.com>,
	"Nathan Lynch" <nathanl@linux.ibm.com>,
	"Laurent Dufour" <ldufour@linux.ibm.com>,
	"Daniel Henrique Barboza" <danielhb413@gmail.com>,
	"David Gibson" <david@gibson.dropbear.id.au>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"David Hildenbrand" <david@redhat.com>,
	"Ohhoon Kwon" <ohoono.kwon@samsung.com>,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>,
	YueHaibing <yuehaibing@huawei.com>
Cc: "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Steen Hegelund <steen.hegelund@microchip.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Horatiu Vultur <horatiu.vultur@microchip.com>,
	Allan Nielsen <allan.nielsen@microchip.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Bjorn Helgaas <helgaas@kernel.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	Lizhi Hou <lizhi.hou@xilinx.com>
Subject: Re: [PATCH v3 5/5] powerpc/pseries: use of_property_alloc/free() and of_node_alloc()
Date: Tue, 28 Jun 2022 17:00:45 +0000	[thread overview]
Message-ID: <a6068e53-80ea-600b-0b54-0a1d0c784c54@csgroup.eu> (raw)
In-Reply-To: <20220620104123.341054-6-clement.leger@bootlin.com>

Le 20/06/2022 à 12:41, Clément Léger a écrit :
> Use of_property_alloc/free() and of_node_alloc() to create and free
> device-tree nodes and properties. In order to obtain something cleaner
> and allow using only of_node_put() instead of manual property deletion,
> a rework of the usage of node in reconfig.c has been done.
> 
> Signed-off-by: Clément Léger <clement.leger@bootlin.com>
> ---

> diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
> index cad7a0c93117..8704c541de3c 100644
> --- a/arch/powerpc/platforms/pseries/reconfig.c
> +++ b/arch/powerpc/platforms/pseries/reconfig.c
> @@ -19,46 +19,29 @@
>   
>   #include "of_helpers.h"
>   
> -static int pSeries_reconfig_add_node(const char *path, struct property *proplist)
> +static struct device_node *pSeries_reconfig_add_node(const char *path)
>   {
> -	struct device_node *np;
> -	int err = -ENOMEM;
> -
> -	np = kzalloc(sizeof(*np), GFP_KERNEL);
> -	if (!np)
> -		goto out_err;
> -
> -	np->full_name = kstrdup(kbasename(path), GFP_KERNEL);
> -	if (!np->full_name)
> -		goto out_err;
> -
> -	np->properties = proplist;
> -	of_node_set_flag(np, OF_DYNAMIC);
> -	of_node_init(np);
> +	struct device_node *np, *parent;
>   
> -	np->parent = pseries_of_derive_parent(path);
> -	if (IS_ERR(np->parent)) {
> -		err = PTR_ERR(np->parent);
> -		goto out_err;
> +	np = of_find_node_by_path(path)

Missing ;

Did you test build ?

> +	if (np) {
> +		of_node_put(np);
> +		return ERR_PTR(-EINVAL);
>   	}
>   
> -	err = of_attach_node(np);
> -	if (err) {
> -		printk(KERN_ERR "Failed to add device node %s\n", path);
> -		goto out_err;
> -	}
> +	parent = pseries_of_derive_parent(path);
> +	if (IS_ERR(parent))
> +		return parent;
>   
> -	of_node_put(np->parent);
> +	np = of_node_alloc(kbasename(path));
> +	if (!np) {
> +		of_node_put(parent);
> +		return ERR_PTR(-ENOMEM);
> +	}
>   
> -	return 0;
> +	np->parent = parent;
>   
> -out_err:
> -	if (np) {
> -		of_node_put(np->parent);
> -		kfree(np->full_name);
> -		kfree(np);
> -	}
> -	return err;
> +	return np;
>   }
>   
>   static int pSeries_reconfig_remove_node(struct device_node *np)