fdt, mpc5xxx: Adapt MPC5xxx to use libfdt in ft_cpu_setup()

Add necessary functions to allow MPC5XXX machines to use libfdt. Code was derived from analogous MPC83xx implementation. Also, add a small coding style fix while in the area.

Signed-off-by Grzegorz Bernacki <[email protected]>

---
I have sent a wrong patch with wrong comments before, please disregard my previous email on the same topic; and sorry for the noise.

The baseline used to generate the patch was the master (as of cc3023b9f95d7ac959a764471a65001062aecf41) with pulled u-boot-fdt#fdt branch (as of 01f771763ed822145b54819abb9c4516c8216d48).

The patch has been tested on two MPC5200B-based boards, one using CONFIG_OF_LIBFDT, the other using CONFIG_OF_FLAT_TREE.

cpu/mpc5xxx/cpu.c +41 -0

index 1eac2bb..2be5caa 100644
@@ -19,6 +19,8 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
  * MA 02111-1307 USA
+ *
+ * Libfdt related routines derived from the MPC83xx.
  */
 
 /*

@@ -33,6 +35,9 @@
 
 #if defined(CONFIG_OF_FLAT_TREE)
 #include <ft_build.h>
+#elif defined(CONFIG_OF_LIBFDT)
+#include <libfdt.h>
+#include <libfdt_env.h>
 #endif
 
 DECLARE_GLOBAL_DATA_PTR;

@@ -109,9 +114,125 @@ unsigned long get_tbclk (void)
        return (tbclk);
 }
 
+#if defined(CONFIG_OF_LIBFDT)
+/*
+ * "Setter" functions used to add/modify FDT entries.
+ */
+static int fdt_set_tbfreq(void *fdt, int nodeoffset, const char *name, bd_t *bd)
+{
+       u32 tmp;
+       /*
+        * Create or update the property.
+        */
+       tmp = cpu_to_be32(OF_TBCLK);
+       return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
+}
+
+static int fdt_set_busfreq(void *fdt, int nodeoffset, const char *name,
+                               bd_t *bd)
+{
+       u32 tmp;
+       /*
+        * Create or update the property.
+        */
+       tmp = cpu_to_be32(bd->bi_busfreq);
+       return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
+}
+
+static int fdt_set_clockfreq(void *fdt, int nodeoffset, const char *name,
+                               bd_t *bd)
+{
+       u32 tmp;
+       /*
+        * Create or update the property.
+        */
+       tmp = cpu_to_be32(bd->bi_intfreq);
+       return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
+}
+

cpu/mpc5xxx/cpu.c +74 -4

+
+static int fdt_set_macaddress(void *fdt, int nodeoffset, const char *name,
+                               bd_t *bd)
+{
+       /*
+        * Fix it up if it exists, don't create it if it doesn't exist.
+        */
+       if (fdt_get_property(fdt, nodeoffset, name, 0)) {
+               return fdt_setprop(fdt, nodeoffset, name, bd->bi_enetaddr, 6);
+       }
+       return 0;
+}
+
 /* ------------------------------------------------------------------------- */
+/*
+ * Fixups to the fdt.
+ */
+static const struct {
+       char *node;
+       char *prop;
+       int (*set_fn)(void *fdt, int nodeoffset, const char *name, bd_t *bd);
+} fixup_props[] = {
+       {       "/cpus/" OF_CPU,
+               "timebase-frequency",
+               fdt_set_tbfreq
+       },
+       {       "/cpus/" OF_CPU,
+               "bus-frequency",
+               fdt_set_busfreq
+       },
+       {       "/cpus/" OF_CPU,
+               "clock-frequency",
+               fdt_set_clockfreq
+       },
+       {       "/" OF_SOC,
+               "bus-frequency",
+               fdt_set_ipbusfreq
+       },
+       {       "/" OF_SOC "/ethernet at 3000",
+               "mac-address",
+               fdt_set_macaddress
+       },
+       {       "/" OF_SOC "/ethernet at 3000",
+               "local-mac-address",
+               fdt_set_macaddress
+       }
+};
 
-#ifdef CONFIG_OF_FLAT_TREE
+void
+ft_cpu_setup(void *blob, bd_t *bd)
+{
+       int nodeoffset;
+       int err;
+       int j;
+
+       for (j = 0; j < (sizeof(fixup_props) / sizeof(fixup_props[0])); j++) {
+               nodeoffset = fdt_find_node_by_path(blob, fixup_props[j].node);
+               if (nodeoffset >= 0) {
+                       err = fixup_props[j].set_fn(blob, nodeoffset,
+                                               fixup_props[j].prop, bd);
+                       if (err < 0)
+                               debug("Problem setting %s = %s: %s\n",
+                                       fixup_props[j].node,
+                                       fixup_props[j].prop,
+                                       fdt_strerror(err));
+               } else {
+                       debug("Couldn't find %s: %s\n",
+                               fixup_props[j].node,
+                               fdt_strerror(nodeoffset));
+               }
+       }
+}
+#elif defined(CONFIG_OF_FLAT_TREE)
 void
 ft_cpu_setup(void *blob, bd_t *bd)
 {

@@ -132,8 +253,9 @@ ft_cpu_setup(void *blob, bd_t *bd)
        if (p != NULL)
                memcpy(p, bd->bi_enetaddr, 6);
 
-       p = ft_get_prop(blob, "/" OF_SOC "/ethernet at 3000/local-mac-address", &len);
+       p = ft_get_prop(blob, "/" OF_SOC "/ethernet at 3000/local-mac-address",
+                        &len);
        if (p != NULL)
                memcpy(p, bd->bi_enetaddr, 6);
 }
-#endif
+#endif /* defined(CONFIG_OF_FLAT_TREE) */
 
-------------------------------------------------------------------------

This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
U-Boot-Users mailing list
U-Boot-Users at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users

Quoted 40 lines

(quoted diff) +40 -0

+static int fdt_set_tbfreq(void *fdt, int nodeoffset, const char *name, bd_t *bd)
+{
+       u32 tmp;
+       /*
+        * Create or update the property.
+        */
+       tmp = cpu_to_be32(OF_TBCLK);
+       return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
+}
+
+static int fdt_set_busfreq(void *fdt, int nodeoffset, const char *name,
+                               bd_t *bd)
+{
+       u32 tmp;
+       /*
+        * Create or update the property.
+        */
+       tmp = cpu_to_be32(bd->bi_busfreq);
+       return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
+}
+
+static int fdt_set_clockfreq(void *fdt, int nodeoffset, const char *name,
+                               bd_t *bd)
+{
+       u32 tmp;
+       /*
+        * Create or update the property.
+        */
+       tmp = cpu_to_be32(bd->bi_intfreq);
+       return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
+}
+
+static int fdt_set_ipbusfreq(void *fdt, int nodeoffset, const char *name,
+                               bd_t *bd)
+{
+       u32 tmp;
+       /*
+        * Create or update the property.
+        */
+       tmp = cpu_to_be32(bd->bi_ipbfreq);

Review

Quoted 2 lines

(quoted diff) +2 -0

+       return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
+}

These 4 functions are pretty close to identical (except for the parameter to cpu_to_be32()). Surely there is a more compact way to do this. In addition, these function don't really contain anything that screams out "5xxx only!". Can this be common support code usable by all boards?

On Fri, Aug 03, 2007 at 08:25:56AM -0600, Grant Likely wrote:
[...]

Quoted 10 lines

Quoted 10 lines

(quoted diff) +10 -0

+static int fdt_set_busfreq(void *fdt, int nodeoffset, const char *name,
+                               bd_t *bd)
+{
+       u32 tmp;
+       /*
+        * Create or update the property.
+        */
+       tmp = cpu_to_be32(bd->bi_busfreq);
+       return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
+}

Please see the patch below for a more generic approach to property fixups. The patch updates mpc5xxx-related code, but I've looked at mpc83xx fixups and it seems like they can be adapted to this approach as well.

I am interested in the comments people might have before proceeding further (i.e., getting rid of OF_FLAT_TREE in mpc5xxx).

Regards,
Bartlomiej

cpu/mpc5xxx/cpu.c +13 -2

index 1eac2bb..1e661c2 100644
@@ -33,6 +33,9 @@
 
 #if defined(CONFIG_OF_FLAT_TREE)
 #include <ft_build.h>
+#elif defined(CONFIG_OF_LIBFDT)
+#include <libfdt.h>
+#include <libfdt_env.h>
 #endif
 
 DECLARE_GLOBAL_DATA_PTR;

@@ -109,9 +112,81 @@ unsigned long get_tbclk (void)
 	return (tbclk);
 }
 
+#if defined(CONFIG_OF_LIBFDT)
 /* ------------------------------------------------------------------------- */
-
-#ifdef CONFIG_OF_FLAT_TREE
+/*
+ * Fixups to the fdt.
+ */
+void
+ft_cpu_setup(void *blob, bd_t *bd)
+{
+	u32 tbfreq;
+	u32 busfreq;
+	u32 intfreq;

cpu/mpc5xxx/cpu.c +20 -0

+		{	"/" OF_SOC "/ethernet at 3000",
+			"mac-address",
+			bd->bi_enetaddr,
+			6,
+			0
+		},
+		{	"/" OF_SOC "/ethernet at 3000",
+			"local-mac-address",
+			bd->bi_enetaddr,
+			6,
+			0
+		}
+	};
+
+	/* manually set members that can't be initialized in the definition */
+	tbfreq = cpu_to_be32(OF_TBCLK);
+	fixup_props[0].val = &tbfreq;
+	fixup_props[0].len = sizeof(tbfreq);
+
+	busfreq = cpu_to_be32(bd->bi_busfreq);

cpu/mpc5xxx/cpu.c +4 -0

+	fdt_fixup_props(blob, fixup_props,
+			sizeof(fixup_props) / sizeof(fixup_props[0]));
+}
+#elif defined(CONFIG_OF_FLAT_TREE)
 void
 ft_cpu_setup(void *blob, bd_t *bd)
 {

include/libfdt.h +12 -0

index 340e89d..065c580 100644
@@ -23,6 +23,17 @@
 #include <fdt.h>
 #include <libfdt_env.h>
 
+
+/* Structure describing property to be fixed-up in cpu-specific code */
+typedef struct {
+        char *node;
+        char *prop;
+        void *val;
+        int len;        /* sizeof(val) */
+        char create;    /* whether to create non-existing properties */
+} fdt_fixup_props_t;
+
+
 #define FDT_FIRST_SUPPORTED_VERSION	0x10
 #define FDT_LAST_SUPPORTED_VERSION	0x11
 

@@ -145,6 +156,7 @@ int fdt_add_subnode_namelen(void *fdt, int parentoffset,
 			    const char *name, int namelen);
 int fdt_add_subnode(void *fdt, int parentoffset, const char *name);
 int fdt_del_node(void *fdt, int nodeoffset);
+void fdt_fixup_props(void *blob, fdt_fixup_props_t fixup_props[], int count);
 
 /* Extra functions */
 const char *fdt_strerror(int errval);

libfdt/fdt_rw.c +29 -0

index 693bfe4..572a4e6 100644
@@ -19,6 +19,8 @@
 #include "config.h"
 #if CONFIG_OF_LIBFDT
 
+#include <common.h>
+
 #include "libfdt_env.h"
 
 #include <fdt.h>

@@ -295,4 +297,40 @@ int fdt_pack(void *fdt)
 	return 0;
 }
 
+
+/*
+ * Function fixes up properties in passed 'blob'. It uses the array
+ * 'fixup_props' to take the description of properites to be fixed up, and
+ * processes 'count' elements from this array.
+ */
+void fdt_fixup_props(void *blob, fdt_fixup_props_t fixup_props[], int count)
+{
+	int nodeoffset;
+	int err;
+	int j;
+
+	for (j = 0; j < count; j++) {
+		nodeoffset = fdt_find_node_by_path(blob, fixup_props[j].node);
+		if (nodeoffset >= 0) {
+			if (fixup_props[j].create ||
+				fdt_get_property(blob, nodeoffset,
+					fixup_props[j].prop, 0))
+				err = fdt_setprop(blob, nodeoffset,
+							fixup_props[j].prop,
+							fixup_props[j].val,
+							fixup_props[j].len);
+			else
+				err = 0;
+			if (err < 0)
+				debug("Problem setting %s = %s: %s\n",
+					fixup_props[j].node,

IMO your proposal is not acceptable. Follow Grant's advice and the current cpu/mpc83xx/cpu.c methodology.

Quoted 2 lines · On Wed, 22 Aug 2007 09:18:20 -0400 Jerry Van Baren…

IMO your proposal is not acceptable. Follow Grant's advice and the current cpu/mpc83xx/cpu.c methodology.

? Bartlomiej's first patch followed the 83xx 'methodology', which Grant commented on, which is what Bartlomiej tried to fix here. Maybe I missed something, but anyway, now that the window has closed, we can arrange to fix libfdt support for all powerpc boards for the next release.

What about having a list of functions to call? It could be called something like fdt_update_sequence, and be similar in implementation to lib_ppc/board.c's init_sequence. It could also reside in lib_ppc, esp. since, e.g. all powerpc's have a timebase, and to help naturally enforce a certain level of consistency among powerpc board code. The updater/fixup/"setter" functions would only need be passed the pointer to the base of the blob to update.

The update functions need to be rewritten to be independent of the OF_CPU, OF_SOC, OF_QE, OF_STDOUT_PATH defines. Those should all be removed (OF_TBCLK might be a bit harder to get rid of though -- implement TBCLK_DIVISOR instead?). btw, Linux is changing the name of the soc node from, e.g. "soc8360", to simply "soc", and u-boot needs to handle transitions like these better. Instead of adding support for both name types, the update functions can handle these situations by using some of the higher level functions available in libfdt, such as fdt_find_node_by_type or even fdt_find_compatible_node.

Quoted 11 lines · Kim Phillips

Quoted 2 lines · On Wed, 22 Aug 2007 09:18:20 -0400 Jerry Van Baren…

IMO your proposal is not acceptable. Follow Grant's advice and the current cpu/mpc83xx/cpu.c methodology.

? Bartlomiej's first patch followed the 83xx 'methodology', which Grant commented on, which is what Bartlomiej tried to fix here. Maybe I missed something, but anyway, now that the window has closed, we can arrange to fix libfdt support for all powerpc boards for the next release.

Well, perhaps Grant can speak to the issue better than me, but I read Grant's reply as a request to coelesce the four "setter" functions into one, not to remove all setter functions and change it back to the original table mechanism. IMHO, using hardcoded [0] [1]... indexes is

Quoted 7 lines

What about having a list of functions to call? It could be called something like fdt_update_sequence, and be similar in implementation to lib_ppc/board.c's init_sequence. It could also reside in lib_ppc, esp. since, e.g. all powerpc's have a timebase, and to help naturally enforce a certain level of consistency among powerpc board code. The updater/fixup/"setter" functions would only need be passed the pointer to the base of the blob to update.

We _have_ "a list of functions to call". That is what is done with the 83xx which is what I advocated and what Bartlomiej had before rewriting it back into a table without functions.

Quoted 1 line · On Wed, 22 Aug 2007 19:16:16 -0400 Jerry Van Baren <[email protected]> wrote:

Kim Phillips wrote:

Quoted 7 lines

Quoted 7 lines

What about having a list of functions to call? It could be called something like fdt_update_sequence, and be similar in implementation to lib_ppc/board.c's init_sequence. It could also reside in lib_ppc, esp. since, e.g. all powerpc's have a timebase, and to help naturally enforce a certain level of consistency among powerpc board code. The updater/fixup/"setter" functions would only need be passed the pointer to the base of the blob to update.

Quoted 3 lines

We _have_ "a list of functions to call". That is what is done with the 83xx which is what I advocated and what Bartlomiej had before rewriting it back into a table without functions.

yeah, I meant /just/ a list of functions to call, without all the extra fluff in both the existing 83xx and Bartlomiej's implementations.

Kim

These 4 functions are pretty close to identical (except for the parameter to cpu_to_be32()). Surely there is a more compact way to do this.

In addition, these function don't really contain anything that screams out "5xxx only!". Can this be common support code usable by all boards?

The first half of Grant's comment was:

Quoted 3 lines

These 4 functions are pretty close to identical (except for the parameter to cpu_to_be32()). Surely there is a more compact way to do this.

My answer is "no, the whole reason there is one line different is because it *has to be*." As the French say about the "Y" chromosome "...and viva la difference." My first attempt, and Bartlomiej's proposed patch, tried (tries) to put the value in the table, but it is a *WORSE* solution because you have the problem with byteswapping or not and different sizes of values. The scorecard for putting the values in the tables and also having it maintainable is 0 for 2.

Quoted 5 lines · Jerry Van Baren

The first half of Grant's comment was:

Quoted 3 lines

These 4 functions are pretty close to identical (except for the parameter to cpu_to_be32()). Surely there is a more compact way to do this.

Quoted 7 lines

My answer is "no, the whole reason there is one line different is because it *has to be*." As the French say about the "Y" chromosome "...and viva la difference." My first attempt, and Bartlomiej's proposed patch, tried (tries) to put the value in the table, but it is a *WORSE* solution because you have the problem with byteswapping or not and different sizes of values. The scorecard for putting the values in the tables and also having it maintainable is 0 for 2.

Yes, this is the icky bit; but I still argue that there is a more compact way to do it. Table driven is nice when it works; but (as I also mentioned in my original comment) whie it doesn't its probably better to just fall back to something programatic

ie (totally untested):

static int fixup_prop(void *fdt, char *node, char *prop, void *val, int size)

{
        int nodeoffset;
        int rc;

        nodeoffset = fdt_find_node_by_path(fdt, node);
        if (nodeoffset < 0) {
                debug("Couldn't find %s: %s\n", node, fdt_strerror(nodeoffset));
                return nodeoffset;
        }

        rc = fdt_get_property(fdt, nodeoffset, prop, 0)
        if (rc)
                goto err;

        rc = fdt_setprop(fdt, nodeoffset, prop, val, size);
        if (rc)
                goto err;

err:

        debug("Problem setting %s = %s: %s\n", node, prop, fdt_strerror(rc));

        return rc;
}

static int fixup_int_prop(void *fdt, char *node, char *prop, u32 val)

{
        val = cpu_to_be32(val);
        return ft_fixup_int_prop(fdt, node, prop, &val, sizeof(val));
}

void
ft_cpu_setup(void *blob, bd_t *bd)

{
        fixup_int_prop(blob, "/cpus/" OF_CPU, "timebase-frequency", OF_TBCLK);
        fixup_int_prop(blob, "/cpus/" OF_CPU, "bus-frequency", bd->bi_busfreq);
        fixup_int_prop(blob, "/cpus/" OF_CPU, "clock-frequency",
bd->bi_intfreq);
        fixup_int_prop(blob, "/cpus/" OF_CPU, "bus-frequency", bd->ipbfreq);
        fixup_prop(blob, "/" OF_SOC "/ethernet at 3000", "mac-address",
                   bd->bi_enetaddr, 6);
        fixup_prop(blob, "/" OF_SOC "/ethernet at 3000", "local-mac-address",
                   bd->bi_enetaddr, 6);
}

This adds 2 helper functions instead of 5, and one of them is the same path for all of the fixups. Drops the table approach entirely due to the problems with extracting values out of bd. (Which is what I meant in the third part of my original comment)

The 2 new helpers could also be generalized for use by all boards.

Cheers,
g.

Quoted 13 lines · Grant Likely

Quoted 12 lines · Jerry Van Baren

The first half of Grant's comment was:

Quoted 3 lines

These 4 functions are pretty close to identical (except for the parameter to cpu_to_be32()). Surely there is a more compact way to do this.

My answer is "no, the whole reason there is one line different is because it *has to be*." As the French say about the "Y" chromosome "...and viva la difference." My first attempt, and Bartlomiej's proposed patch, tried (tries) to put the value in the table, but it is a *WORSE* solution because you have the problem with byteswapping or not and different sizes of values. The scorecard for putting the values in the tables and also having it maintainable is 0 for 2.

Quoted 4 lines

Yes, this is the icky bit; but I still argue that there is a more compact way to do it. Table driven is nice when it works; but (as I also mentioned in my original comment) whie it doesn't its probably better to just fall back to something programatic

Quoted 10 lines

void
ft_cpu_setup(void *blob, bd_t *bd)

{
        fixup_int_prop(blob, "/cpus/" OF_CPU, "timebase-frequency", OF_TBCLK);
        fixup_int_prop(blob, "/cpus/" OF_CPU, "bus-frequency", bd->bi_busfreq);
        fixup_int_prop(blob, "/cpus/" OF_CPU, "clock-frequency",
bd->bi_intfreq);
        fixup_int_prop(blob, "/cpus/" OF_CPU, "bus-frequency", bd->ipbfreq);
        fixup_prop(blob, "/" OF_SOC "/ethernet at 3000", "mac-address",
                   bd->bi_enetaddr, 6);

Quoted 2 lines

        fixup_prop(blob, "/" OF_SOC "/ethernet at 3000", "local-mac-address",
                   bd->bi_enetaddr, 6);

Quoted 6 lines

}

This adds 2 helper functions instead of 5, and one of them is the same path for all of the fixups. Drops the table approach entirely due to the problems with extracting values out of bd. (Which is what I meant in the third part of my original comment)

I think 2 is too optimistic, but it is still possible this approach would be better than the current 5. The above looks better, but I claim only because it is oversimplified - I contend the 2 helper functions will expand to 5 functions that basically are the current "setter" functions. On the other hand, maybe not. It is worth trying.

Quoted 1 line

The 2 new helpers could also be generalized for use by all boards.

s/2/n/ (where n is probably 5) and I would agree 100%. :-/

Quoted 2 lines

Cheers,
g.

Quoted 1 line · Jerry Van Baren

Grant Likely wrote:

Quoted 9 lines

Quoted 9 lines

ft_cpu_setup(void *blob, bd_t *bd)

{
        fixup_int_prop(blob, "/cpus/" OF_CPU, "timebase-frequency", OF_TBCLK);
        fixup_int_prop(blob, "/cpus/" OF_CPU, "bus-frequency", bd->bi_busfreq);
        fixup_int_prop(blob, "/cpus/" OF_CPU, "clock-frequency",
bd->bi_intfreq);
        fixup_int_prop(blob, "/cpus/" OF_CPU, "bus-frequency", bd->ipbfreq);
        fixup_prop(blob, "/" OF_SOC "/ethernet at 3000", "mac-address",
                   bd->bi_enetaddr, 6);

	fixup_mac_prop(blob, "/" OF_SOC "/ethernet at 3000", "mac-address",
		bd->bi_enetaddr, 1);

Quoted 2 lines

Quoted 2 lines

        fixup_prop(blob, "/" OF_SOC "/ethernet at 3000", "local-mac-address",
                   bd->bi_enetaddr, 6);

	fixup_mac_prop(blob, "/" OF_SOC "/ethernet at 3000",
		"local-mac-address", bd->bi_enetaddr, 0);

Quoted 11 lines

Quoted 6 lines

}

This adds 2 helper functions instead of 5, and one of them is the same path for all of the fixups. Drops the table approach entirely due to the problems with extracting values out of bd. (Which is what I meant in the third part of my original comment)

I think 2 is too optimistic, but it is still possible this approach would be better than the current 5. The above looks better, but I claim only because it is oversimplified - I contend the 2 helper functions will expand to 5 functions that basically are the current "setter" functions. On the other hand, maybe not. It is worth trying.

[more snippage]

Quoted 5 lines

Quoted 1 line

The 2 new helpers could also be generalized for use by all boards.

s/2/n/ (where n is probably 5) and I would agree 100%. :-/

Quoted 2 lines

Cheers,
g.

Thinking about it and a quick glance (I'm running remotely), I think we could do the job with three "setter" functions based on Grant's proposal, which would be a Good Thing[tm].

static int fixup_int_prop(void *fdt, char *node, char *prop, u32 val)
- Like Grant proposes, note it does a cpu_to_be32()

static int fixup_mac_prop(void *fdt, char *node, char *prop, void *val, int force)
- Note I replaced "size" with "force". We know the size (6). The "force" flag (better name, anyone?) would be whether the MAC addr should be created if it _doesn't_ exist.

Quoted 2 lines · Jerry Van Baren

IMO your proposal is not acceptable. Follow Grant's advice and the current cpu/mpc83xx/cpu.c methodology.

? Bartlomiej's first patch followed the 83xx 'methodology', which Grant commented on, which is what Bartlomiej tried to fix here. Maybe I missed something, but anyway, now that the window has closed, we can arrange to fix libfdt support for all powerpc boards for the next release.

Quoted 11 lines · In message <[email protected]> you wrote:

Quoted 2 lines · Jerry Van Baren

IMO your proposal is not acceptable. Follow Grant's advice and the current cpu/mpc83xx/cpu.c methodology.

? Bartlomiej's first patch followed the 83xx 'methodology', which Grant commented on, which is what Bartlomiej tried to fix here. Maybe I missed something, but anyway, now that the window has closed, we can arrange to fix libfdt support for all powerpc boards for the next release.

We still accept patches that fix bugs and solve urgent problems, and if I understand correctly, Bartek's patch attempts to fix a real problem.

Quoted 5 lines · On Thu, 23 Aug 2007 01:30:29 +0200 Wolfgang Denk <[email protected]> wrote:

Quoted 4 lines · In message <[email protected]> you wrote:

Quoted 2 lines · Jerry Van Baren

IMO your proposal is not acceptable. Follow Grant's advice and the current cpu/mpc83xx/cpu.c methodology.

Quoted 8 lines

Quoted 5 lines

? Bartlomiej's first patch followed the 83xx 'methodology', which Grant commented on, which is what Bartlomiej tried to fix here. Maybe I missed something, but anyway, now that the window has closed, we can arrange to fix libfdt support for all powerpc boards for the next release.

We still accept patches that fix bugs and solve urgent problems, and if I understand correctly, Bartek's patch attempts to fix a real problem.

maybe I misunderstood then. I just don't want to see code being duplicated among two subplatforms (83xx and 52xx) without some stuff being refactored into common powerpc areas.

Kim

maybe I misunderstood then. I just don't want to see code being duplicated among two subplatforms (83xx and 52xx) without some stuff being refactored into common powerpc areas.

Quoted 5 lines · Bartlomiej Sieka

Add necessary functions to allow MPC5XXX machines to use libfdt. Code was derived from analogous MPC83xx implementation. Also, add a small coding style fix while in the area.

Signed-off-by Grzegorz Bernacki <[email protected]>

Comments below

From the following discussion [see thread "RFC: generic property fixup mechanism for LIBFDT"] I got the impression that an improved implementation is not available yet, so it will have to wait for the next release.

Do you agree to include this patch (which fixes a few build problems) into the current release and clean it up in the next one?

Signed-off-by Grzegorz Bernacki <[email protected]>

---
Not sure where this should be merged, it was applied to u-boot-fdt#fdt (as of 01f771763ed822145b54819abb9c4516c8216d48) merged with the master (as of cc3023b9f95d7ac959a764471a65001062aecf41). Tested on two MPC5200B-based boards, one using CONFIG_OF_LIBFDT, the other using CONFIG_OF_FLAT_TREE.