sparc: make driver/of/pdt no longer sparc-specific

Clean up pdt.c:
 - make build dependent upon config OF_PROMTREE
 - #ifdef out the sparc-specific stuff
 - create pdt-specific header
 - create a pdt_ops struct that pdt uses to call arch-specific prom routines

Signed-off-by Andres Salomon <[email protected]>

---
 arch/sparc/Kconfig              |    1 + arch/sparc/include/asm/prom.h   |    5 +- arch/sparc/kernel/prom.h        |    6 -- arch/sparc/kernel/prom_common.c |   57 ++++++++++++++++++++++- drivers/of/Kconfig              |    4 ++
 drivers/of/Makefile             |    1 +
 drivers/of/pdt.c                |   98 +++++++++++++++++++++++++------------- include/linux/of_pdt.h          |   42 +++++++++++++++++

 8 files changed, 171 insertions(+), 43 deletions(-)

 create mode 100644 include/linux/of_pdt.h

arch/sparc/Kconfig +0 -0

index 13a9f2f..ed3f009 100644
@@ -24,6 +24,7 @@ config SPARC

       select HAVE_ARCH_KGDB if !SMP || SPARC64
       select HAVE_ARCH_TRACEHOOK
       select ARCH_WANT_OPTIONAL_GPIOLIB
+       select OF_PROMTREE

       select RTC_CLASS
       select RTC_DRV_M48T59
       select HAVE_PERF_EVENTS

arch/sparc/include/asm/prom.h +3 -2

index f845828..329a976 100644
@@ -18,6 +18,7 @@
  * 2 of the License, or (at your option) any later version.
  */
  #include <linux/types.h>
+#include <linux/of_pdt.h>
  #include <linux/proc_fs.h>
  #include <linux/mutex.h>
  #include <asm/atomic.h>

@@ -65,8 +66,8 @@ extern struct device_node *of_console_device;
  extern char *of_console_path;
  extern char *of_console_options;
 
-extern void (*prom_build_more)(struct device_node *dp, struct device_node ***nextp);
-extern char *build_full_name(struct device_node *dp);
+extern void irq_trans_init(struct device_node *dp);
+extern char *build_path_component(struct device_node *dp);
 
  #endif /* __KERNEL__ */
  #endif /* _SPARC_PROM_H */

arch/sparc/kernel/prom.h +0 -6

index eeb04a7..cf5fe1c 100644
@@ -4,12 +4,6 @@
  #include <linux/spinlock.h>
  #include <asm/prom.h>
 
-extern void * prom_early_alloc(unsigned long size);
-extern void irq_trans_init(struct device_node *dp);
-
-extern unsigned int prom_unique_id;
-
-extern char *build_path_component(struct device_node *dp);
  extern void of_console_init(void);
 
  extern unsigned int prom_early_allocated;

arch/sparc/kernel/prom_common.c +26 -0

index 7b454f6..4c5f67f 100644
@@ -20,6 +20,7 @@
  #include <linux/mutex.h>
  #include <linux/slab.h>
  #include <linux/of.h>
+#include <linux/of_pdt.h>
  #include <asm/prom.h>
  #include <asm/oplib.h>
  #include <asm/leon.h>

@@ -117,6 +118,60 @@ int of_find_in_proplist(const char *list, const char *match, int len)
  }
  EXPORT_SYMBOL(of_find_in_proplist);
 
+/*
+ * SPARC32 and SPARC64's prom_firstprop/prom_nextprop do things differently
+ * here, despite sharing the same interface.  SPARC32 doesn't fill in 'buf',
+ * returning NULL on an error.  SPARC64 fills in 'buf', but sets it to an
+ * empty string upon error.
+ */
+static int __init handle_prop_quirks(char *buf, const char *name)
+{
+       if (!name || strlen(name) == 0)
+               return -1;
+
+#ifdef CONFIG_SPARC32
+       strcpy(buf, name);
+#endif
+       return 0;
+}
+
+static int __init prom_common_firstprop(phandle node, char *buf)
+{
+       const char *name;
+
+       buf[0] = '\0';
+       name = prom_firstprop(node, buf);
+       return handle_prop_quirks(buf, name);
+}

 config OF_DYNAMIC
       def_bool y
       depends on OF && PPC_OF

drivers/of/Makefile +0 -0

index f232cc9..54e8517 100644
@@ -1,5 +1,6 @@

 obj-y = base.o
 obj-$(CONFIG_OF_FLATTREE) += fdt.o
+obj-$(CONFIG_OF_PROMTREE) += pdt.o
 obj-$(CONFIG_OF_DEVICE) += device.o platform.o
 obj-$(CONFIG_OF_GPIO)   += gpio.o
 obj-$(CONFIG_OF_I2C)   += of_i2c.o

drivers/of/pdt.c +0 -0

index 61d9477..22f46fb 100644

drivers/of/pdt.c +1 -0

  * Paul Mackerras      August 1996.
  * Copyright (C) 1996-2005 Paul Mackerras.
  *

@@ -7,6 +6,7 @@
  *    {engebret|bergner}@us.ibm.com
  *
  *  Adapted for sparc by David S. Miller [email protected]
+ *  Adapted for multiple architectures by Andres Salomon <[email protected]>
  *
  *      This program is free software; you can redistribute it and/or
  *      modify it under the terms of the GNU General Public License

@@ -20,13 +20,44 @@
  #include <linux/mutex.h>
  #include <linux/slab.h>
  #include <linux/of.h>

drivers/of/pdt.c +7 -0

+
+static inline const char *fetch_node_name(struct device_node *dp)
+{
+       return dp->path_component_name;
+}
+
+#else

drivers/of/pdt.c +2 -5

-unsigned int prom_unique_id;
+#endif /* !CONFIG_SPARC */
 
  static struct property * __init build_one_prop(phandle node, char *prev,
                                               char *special_name,

@@ -35,7 +66,6 @@ static struct property * __init build_one_prop(phandle node, char *prev,
  {
        static struct property *tmp = NULL;
        struct property *p;
-       const char *name;
 
        if (tmp) {
                p = tmp;

@@ -43,7 +73,7 @@ static struct property * __init build_one_prop(phandle node, char *prev,
                tmp = NULL;
        } else {
                p = prom_early_alloc(sizeof(struct property) + 32);
-               p->unique_id = prom_unique_id++;
+               inc_unique_id(p);
        }
 
        p->name = (char *) (p + 1);

@@ -53,27 +83,24 @@ static struct property * __init build_one_prop(phandle node, char *prev,
                p->value = prom_early_alloc(special_len);
                memcpy(p->value, special_val, special_len);
        } else {
-               if (prev == NULL) {
-                       name = prom_firstprop(node, p->name);

drivers/of/pdt.c +16 -16

+               if (err) {
                        tmp = p;
                        return NULL;
                }
-#ifdef CONFIG_SPARC32
-               strcpy(p->name, name);
-#endif
-               p->length = prom_getproplen(node, p->name);
+               p->length = prom_ops.getproplen(node, p->name);
                if (p->length <= 0) {
                        p->length = 0;
                } else {
                        int len;
 
                        p->value = prom_early_alloc(p->length + 1);
-                       len = prom_getproperty(node, p->name, p->value,
+                       len = prom_ops.getproperty(node, p->name, p->value,
                                               p->length);
                        if (len <= 0)
                                p->length = 0;

@@ -106,10 +133,10 @@ static char * __init get_one_property(phandle node, const char *name)
        char *buf = "<NULL>";
        int len;
 
-       len = prom_getproplen(node, name);
+       len = prom_ops.getproplen(node, name);
        if (len > 0) {
                buf = prom_early_alloc(len);
-               len = prom_getproperty(node, name, buf, len);
+               len = prom_ops.getproperty(node, name, buf, len);
        }
 
        return buf;

@@ -124,7 +151,7 @@ static struct device_node * __init prom_create_node(phandle node,
                return NULL;
 
        dp = prom_early_alloc(sizeof(*dp));
-       dp->unique_id = prom_unique_id++;
+       inc_unique_id(dp);
        dp->parent = parent;
 
        kref_init(&dp->kref);

@@ -140,13 +167,13 @@ static struct device_node * __init prom_create_node(phandle node,
        return dp;
  }
 
-char * __init build_full_name(struct device_node *dp)
+static char * __init build_full_name(struct device_node *dp)
  {
        int len, ourlen, plen;
        char *n;
 
        plen = strlen(dp->parent->full_name);
-       ourlen = strlen(dp->path_component_name);
+       ourlen = strlen(fetch_node_name(dp));
        len = ourlen + plen + 2;
 
        n = prom_early_alloc(len);

@@ -155,7 +182,7 @@ char * __init build_full_name(struct device_node *dp)
                strcpy(n + plen, "/");
                plen++;
        }
-       strcpy(n + plen, dp->path_component_name);
+       strcpy(n + plen, fetch_node_name(dp));
 
        return n;
  }

@@ -182,36 +209,39 @@ static struct device_node * __init prom_build_tree(struct device_node *parent,
                *(*nextp) = dp;
                *nextp = &dp->allnext;
 
+#if defined(CONFIG_SPARC)
                dp->path_component_name = build_path_component(dp);
+#endif
                dp->full_name = build_full_name(dp);
 
-               dp->child = prom_build_tree(dp, prom_getchild(node), nextp);
+               dp->child = prom_build_tree(dp, prom_ops.getchild(node), nextp);
 
                if (prom_build_more)
                        prom_build_more(dp, nextp);
 
-               node = prom_getsibling(node);
+               node = prom_ops.getsibling(node);
        }
 
        return ret;
  }
 
-void __init prom_build_devicetree(void)
+void __init of_pdt_build_devicetree(int root_node)
  {
        struct device_node **nextp;
 
-       allnodes = prom_create_node(prom_root_node, NULL);
+       allnodes = prom_create_node(root_node, NULL);
        allnodes->path_component_name = "";
        allnodes->full_name = "/";
 
        nextp = &allnodes->allnext;
        allnodes->child = prom_build_tree(allnodes,
-                       prom_getchild(allnodes->phandle),
+                       prom_ops.getchild(allnodes->phandle),
                        &nextp);

drivers/of/pdt.c +0 -1

  }
-

include/linux/of_pdt.h new file +42 -1

new file mode 100644
index 0000000..1324ba5
@@ -0,0 +1,42 @@
+/*
+ * Definitions for building a device tree by calling into the
+ * Open Firmware PROM.
+ *
+ * Copyright (C) 2010  Andres Salomon <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#ifndef _LINUX_OF_PDT_H
+#define _LINUX_OF_PDT_H
+
+extern void *prom_early_alloc(unsigned long size);
+
+/* overridable operations for calling into the PROM */
+struct of_pdt_ops {
+       /* buffers passed should be 32 bytes; return 0 on success */
+       int (*firstprop)(phandle node, char *buf);
+       int (*nextprop)(phandle node, const char *prev, char *buf);
+
+       /* for both functions, return proplen on success; -1 on error */
+       int (*getproplen)(phandle node, const char *prop);
+       int (*getproperty)(phandle node, const char *prop, char *buf,
+                       int bufsize);
+
+       /* phandles are 0 if no child or sibling exists */
+       phandle (*getchild)(phandle parent);
+       phandle (*getsibling)(phandle node);
+};
+
+extern void of_pdt_set_ops(struct of_pdt_ops *ops);
+
+/* for building the device tree */
+extern void of_pdt_build_devicetree(int root_node);
+
+extern void (*prom_build_more)(struct device_node *dp,
+               struct device_node ***nextp);
+
+#endif /* _LINUX_OF_PDT_H */
--

1.5.6.5

Hi Andres, thanks for the patch. Comments below.

g.

On Sun, Aug 8, 2010 at 9:11 PM, Andres Salomon <[email protected]> wrote:

Quoted 8 lines

+
 unsigned int prom_early_allocated __initdata;

arch/sparc/kernel/prom_common.c +4 -1

-#include "../../../drivers/of/pdt.c"
+static struct of_pdt_ops prom_sparc_ops __initdata = {
+       .firstprop = prom_common_firstprop,
+       .nextprop = prom_common_nextprop,
+       .getproplen = (int (*)(phandle, const char

Quoted 5 lines

*))prom_getproplen,
+       .getproperty = (int (*)(phandle, const char *, char *, int))prom_getproperty,
+       .getchild = (phandle (*)(phandle))prom_getchild,
+       .getsibling = (phandle (*)(phandle))prom_getsibling,

If you have to explicitly cast these function pointers, then you're doing it wrong. :-) Listen to and fix the compiler complaint here.

Quoted 9 lines

(quoted diff) +1 -0

+}

drivers/of/Kconfig +0 -0

index 1678dbc..c8a4b7c 100644
@@ -5,6 +5,10 @@ config OF_FLATTREE

       bool
       depends on OF

drivers/of/Kconfig config OF_FLATTREE · line 11

Quoted 4 lines

drivers/of/Kconfig +4 -0

+config OF_PROMTREE
+       bool
+       depends on OF
+

I can tell from the context here you're working from an older tree. Please rebase onto Linus' current top-of-tree. :-) A bunch of OF related patches have been merged for 2.6.36 that will conflict with this patch.

drivers/of/Kconfig

Quoted 8 lines

drivers/of/Kconfig +7 -0

+
+#if defined(CONFIG_SPARC)
+static unsigned int prom_unique_id __initdata;
+
+#define inc_unique_id(p) do { \
+       (p)->unique_id = prom_unique_id++; \
+} while (0)

Use a static inline. C code is preferred over preprocessor code. Also preserver the namespace and use the of_pdt_ prefix (that goes for all the new functions here in this file).

Quoted 5 lines

-       of_console_init();

drivers/of/Kconfig +3 -0

+void __init of_pdt_set_ops(struct of_pdt_ops *ops)
+{
+       BUG_ON(!ops);

Quoted 3 lines

-       printk("PROM: Built device tree with %u bytes of memory.\n",
-              prom_early_allocated);
+       prom_ops = *ops;

As mentioned above, why is the structure copied instead of just storing the pointer.

Quoted 3 lines · On Sun, 8 Aug 2010 23:12:21 -0600 Grant Likely <[email protected]>…

If you have to explicitly cast these function pointers, then you're doing it wrong. :-) Listen to and fix the compiler complaint here.

Hm, can you please expand on that? The reason it's necessary to cast is because sparc's prom_* functions are using ints instead of phandles. I don't understand why casting is the wrong thing here.

I could write some 1-line wrapper functions that simply call prom_* rather than casting, I suppose.

From: Andres Salomon <[email protected]>
Date: Mon, 9 Aug 2010 01:32:45 -0400

Quoted 12 lines

Quoted 4 lines · On Sun, 8 Aug 2010 23:12:21 -0600 Grant Likely <[email protected]>…

If you have to explicitly cast these function pointers, then you're doing it wrong. :-) Listen to and fix the compiler complaint here.

Hm, can you please expand on that? The reason it's necessary to cast is because sparc's prom_* functions are using ints instead of phandles. I don't understand why casting is the wrong thing here.

Quoted 2 lines

I could write some 1-line wrapper functions that simply call prom_* rather than casting, I suppose.

There's no reason the Sparc interfaces can't take phandles since they are just ints, and that would make the call signatures of these functions compatible with the types used by the other architectures.

I'm tempted to break the phandle/device_node/property stuff out into a separate header (that gets exported), and include that from linux/of.h and asm/openprom.h.

Quoted 6 lines · On Sun, 8 Aug 2010 23:12:21 -0600 Grant Likely…

Hi Andres, thanks for the patch.  Comments below.

g.

On Sun, Aug 8, 2010 at 9:11 PM, Andres Salomon <[email protected]> wrote:

[...]

Quoted 17 lines

Quoted 8 lines

+
 unsigned int prom_early_allocated __initdata;

arch/sparc/kernel/prom_common.c +4 -1

-#include "../../../drivers/of/pdt.c"
+static struct of_pdt_ops prom_sparc_ops __initdata = {
+       .firstprop = prom_common_firstprop,
+       .nextprop = prom_common_nextprop,
+       .getproplen = (int (*)(phandle, const char

Quoted 5 lines

*))prom_getproplen,
+       .getproperty = (int (*)(phandle, const char *, char *, int))prom_getproperty,
+       .getchild = (phandle (*)(phandle))prom_getchild,
+       .getsibling = (phandle (*)(phandle))prom_getsibling,

If you have to explicitly cast these function pointers, then you're doing it wrong.  :-)  Listen to and fix the compiler complaint here.

Hm, can you please expand on that?  The reason it's necessary to cast is because sparc's prom_* functions are using ints instead of phandles.  I don't understand why casting is the wrong thing here.

Quoted 5 lines

I can tell from the context here you're working from an older tree. Please rebase onto Linus' current top-of-tree.  :-)  A bunch of OF related patches have been merged for 2.6.36 that will conflict with this patch.

Sorry, will do.  Was just looking for more feedback (while testing) before sending final versions of this stuff.

Quoted 12 lines

drivers/of/pdt.c line 36

Quoted 8 lines

drivers/of/pdt.c +7 -0

+
+#if defined(CONFIG_SPARC)
+static unsigned int prom_unique_id __initdata;
+
+#define inc_unique_id(p) do { \
+       (p)->unique_id = prom_unique_id++; \
+} while (0)

Use a static inline.  C code is preferred over preprocessor code. Also preserver the namespace and use the of_pdt_ prefix (that goes for all the new functions here in this file).

This is processing multiple types, that's the reason for the macro.

'p' can be either a property struct, or device_node struct.

[...]

Quoted 5 lines

Quoted 5 lines

-       of_console_init();

drivers/of/pdt.c +3 -0

+void __init of_pdt_set_ops(struct of_pdt_ops *ops)
+{
+       BUG_ON(!ops);

Quoted 7 lines

Quoted 3 lines

-       printk("PROM: Built device tree with %u bytes of memory.\n",
-              prom_early_allocated);
+       prom_ops = *ops;

As mentioned above, why is the structure copied instead of just storing the pointer.

Er, right, because originally the struct was handled differently.  No reason for it to be copied anymore.

Thanks for the feedback!