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

2020-02-27 Thread 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 
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Hari Bathini 
Cc: Aneesh Kumar K.V 
Cc: Oliver O'Halloran 
Cc: Dan Williams 
Cc: ke...@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 000..e56c832
--- /dev/null
+++ b/arch/powerpc/kernel/of_property.c
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include 
+#include 
+#include 
+#include 
+
+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(c

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

2020-02-27 Thread Pingfan Liu
On Fri, Feb 28, 2020 at 2:03 PM Andrew Donnellan  wrote:
>
> On 28/2/20 4:53 pm, Pingfan Liu wrote:
> > 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.
> >
> > Signed-off-by: Pingfan Liu 
> > To: linuxppc-dev@lists.ozlabs.org
> > Cc: Benjamin Herrenschmidt 
> > Cc: Paul Mackerras 
> > Cc: Michael Ellerman 
> > Cc: Hari Bathini 
> > Cc: Aneesh Kumar K.V 
> > Cc: Oliver O'Halloran 
> > Cc: Dan Williams 
> > Cc: ke...@lists.infradead.org
>
> Which tree does this apply to? I don't see a new_property() in mm/drmem.c...
Sorry, there is mud in my git tree, I check, either linux git or
powerpc git tree does not have this function.

Nack this series, and I will send out V2 for patch 3/3.

Thanks,
Pingfan
>
> --
> Andrew Donnellan  OzLabs, ADL Canberra
> a...@linux.ibm.com IBM Australia Limited
>


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

2020-02-27 Thread Andrew Donnellan

On 28/2/20 4:53 pm, Pingfan Liu wrote:

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.

Signed-off-by: Pingfan Liu 
To: linuxppc-dev@lists.ozlabs.org
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: Hari Bathini 
Cc: Aneesh Kumar K.V 
Cc: Oliver O'Halloran 
Cc: Dan Williams 
Cc: ke...@lists.infradead.org


Which tree does this apply to? I don't see a new_property() in mm/drmem.c...

--
Andrew Donnellan  OzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited