[PATCH 1/3] powerpc/of: split out new_property() for reusing

[Christophe Leroy]

Le 28/02/2020 à 06:53, Pingfan Liu a écrit :
> Since new_property() is used in several calling sites, splitting it out for
> reusing.
> 
> To ease the review, although the split out part has coding style issue,
> keeping it untouched and fixed in next patch.

The moved function fits in one screen. I don't think it is worth 
splitting out for coding style that applies on the new lines only. You 
can squash the two patches, it will still be easy to review.

> 
> Signed-off-by: Pingfan Liu <kernelfans at gmail.com>
> To: linuxppc-dev at lists.ozlabs.org
> Cc: Benjamin Herrenschmidt <benh at kernel.crashing.org>
> Cc: Paul Mackerras <paulus at samba.org>
> Cc: Michael Ellerman <mpe at ellerman.id.au>
> Cc: Hari Bathini <hbathini at linux.ibm.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar at linux.ibm.com>
> Cc: Oliver O'Halloran <oohall at gmail.com>
> Cc: Dan Williams <dan.j.williams at intel.com>
> Cc: kexec at lists.infradead.org
> ---
>   arch/powerpc/include/asm/prom.h           |  2 ++
>   arch/powerpc/kernel/Makefile              |  2 +-
>   arch/powerpc/kernel/of_property.c         | 32 +++++++++++++++++++++++++++++++
>   arch/powerpc/mm/drmem.c                   | 26 -------------------------
>   arch/powerpc/platforms/pseries/reconfig.c | 26 -------------------------
>   5 files changed, 35 insertions(+), 53 deletions(-)
>   create mode 100644 arch/powerpc/kernel/of_property.c
> 
> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> index 94e3fd5..02f7b1b 100644
> --- a/arch/powerpc/include/asm/prom.h
> +++ b/arch/powerpc/include/asm/prom.h
> @@ -90,6 +90,8 @@ struct of_drc_info {
>   extern int of_read_drc_info_cell(struct property **prop,
>   			const __be32 **curval, struct of_drc_info *data);
>   
> +extern struct property *new_property(const char *name, const int length,
> +		const unsigned char *value, struct property *last);

Don't add useless 'extern' keyword.

>   
>   /*
>    * There are two methods for telling firmware what our capabilities are.
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 157b014..23375fd 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -47,7 +47,7 @@ obj-y				:= cputable.o ptrace.o syscalls.o \
>   				   signal.o sysfs.o cacheinfo.o time.o \
>   				   prom.o traps.o setup-common.o \
>   				   udbg.o misc.o io.o misc_$(BITS).o \
> -				   of_platform.o prom_parse.o
> +				   of_platform.o prom_parse.o of_property.o
>   obj-$(CONFIG_PPC64)		+= setup_64.o sys_ppc32.o \
>   				   signal_64.o ptrace32.o \
>   				   paca.o nvram_64.o firmware.o note.o
> diff --git a/arch/powerpc/kernel/of_property.c b/arch/powerpc/kernel/of_property.c
> new file mode 100644
> index 0000000..e56c832
> --- /dev/null
> +++ b/arch/powerpc/kernel/of_property.c
> @@ -0,0 +1,32 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/of.h>
> +#include <linux/string.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +
> +struct property *new_property(const char *name, const int length,
> +				     const unsigned char *value, struct property *last)
> +{
> +	struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
> +
> +	if (!new)
> +		return NULL;
> +
> +	if (!(new->name = kstrdup(name, GFP_KERNEL)))
> +		goto cleanup;
> +	if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
> +		goto cleanup;
> +
> +	memcpy(new->value, value, length);
> +	*(((char *)new->value) + length) = 0;
> +	new->length = length;
> +	new->next = last;
> +	return new;
> +
> +cleanup:
> +	kfree(new->name);
> +	kfree(new->value);
> +	kfree(new);
> +	return NULL;
> +}
> +
> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
> index 85b088a..888227e 100644
> --- a/arch/powerpc/mm/drmem.c
> +++ b/arch/powerpc/mm/drmem.c
> @@ -99,32 +99,6 @@ static void init_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>   
>   extern int test_hotplug;
>   
> -static struct property *new_property(const char *name, const int length,
> -				     const unsigned char *value, struct property *last)
> -{
> -	struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
> -
> -	if (!new)
> -		return NULL;
> -
> -	if (!(new->name = kstrdup(name, GFP_KERNEL)))
> -		goto cleanup;
> -	if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
> -		goto cleanup;
> -
> -	memcpy(new->value, value, length);
> -	*(((char *)new->value) + length) = 0;
> -	new->length = length;
> -	new->next = last;
> -	return new;
> -
> -cleanup:
> -	kfree(new->name);
> -	kfree(new->value);
> -	kfree(new);
> -	return NULL;
> -}
> -
>   static int drmem_update_dt_v2(struct device_node *memory,
>   			      struct property *prop)
>   {
> diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
> index 7f7369f..8e5a2ba 100644
> --- a/arch/powerpc/platforms/pseries/reconfig.c
> +++ b/arch/powerpc/platforms/pseries/reconfig.c
> @@ -165,32 +165,6 @@ static char * parse_next_property(char *buf, char *end, char **name, int *length
>   	return tmp;
>   }
>   
> -static struct property *new_property(const char *name, const int length,
> -				     const unsigned char *value, struct property *last)
> -{
> -	struct property *new = kzalloc(sizeof(*new), GFP_KERNEL);
> -
> -	if (!new)
> -		return NULL;
> -
> -	if (!(new->name = kstrdup(name, GFP_KERNEL)))
> -		goto cleanup;
> -	if (!(new->value = kmalloc(length + 1, GFP_KERNEL)))
> -		goto cleanup;
> -
> -	memcpy(new->value, value, length);
> -	*(((char *)new->value) + length) = 0;
> -	new->length = length;
> -	new->next = last;
> -	return new;
> -
> -cleanup:
> -	kfree(new->name);
> -	kfree(new->value);
> -	kfree(new);
> -	return NULL;
> -}
> -
>   static int do_add_node(char *buf, size_t bufsize)
>   {
>   	char *path, *end, *name;
> 

Christophe