Move to LIBFDT for mpc5xxx platforms

From: Grant Likely <[email protected]>

Affects boards: icecube (lite5200), jupiter, motionpro, tqm5200

Tested on: lite5200b

Signed-off-by Grant Likely <[email protected]>

---

cpu/mpc5xxx/cpu.c +12 -3

 #endif
 
 DECLARE_GLOBAL_DATA_PTR;

@@ -109,31 +113,46 @@ unsigned long get_tbclk (void)
 	return (tbclk);
 }
 
+
 /* ------------------------------------------------------------------------- */
 
-#ifdef CONFIG_OF_FLAT_TREE
-void
-ft_cpu_setup(void *blob, bd_t *bd)
+#ifdef CONFIG_OF_LIBFDT
+static void do_fixup(void *fdt, const char *node, const char *prop,
+		     const void *val, int len, int create)
+{
+#if defined(DEBUG)
+	int i;
+	debug("Updating property '%s/%s' = ", node, prop);
+	for (i = 0; i < len; i++)
+		debug(" %.2x", *(u8*)(val+i));
+	debug("\n");
+#endif

Quoted 8 lines · Grant Likely

From: Grant Likely <[email protected]>

Affects boards: icecube (lite5200), jupiter, motionpro, tqm5200

Tested on: lite5200b

Signed-off-by Grant Likely <[email protected]>

---

Hi Grant,

Thanks for including motionpro changes with this patch; see my comments below.

[...]

cpu/mpc5xxx/cpu.c line 29

Quoted 10 lines

cpu/mpc5xxx/cpu.c +2 -0

index 1eac2bb..e8a928a 100644
@@ -25,14 +25,18 @@
  * CPU specific code for the MPC5xxx CPUs
  */
 
+#undef DEBUG
+

Is this needed?

cpu/mpc5xxx/cpu.c line 39

Quoted 12 lines

cpu/mpc5xxx/cpu.c +4 -2

 #include <common.h>
 #include <watchdog.h>
 #include <command.h>
 #include <mpc5xxx.h>
+#include <asm/io.h>
 #include <asm/processor.h>
 
-#if defined(CONFIG_OF_FLAT_TREE)
-#include <ft_build.h>
+#if defined(CONFIG_OF_LIBFDT)
+#include <libfdt.h>
+#include <libfdt_env.h>

Are we using this?

Quoted 24 lines

cpu/mpc5xxx/cpu.c +12 -3

 #endif
 
 DECLARE_GLOBAL_DATA_PTR;

@@ -109,31 +113,46 @@ unsigned long get_tbclk (void)
      return (tbclk);
 }
 
+
 /* ------------------------------------------------------------------------- */
 
-#ifdef CONFIG_OF_FLAT_TREE
-void
-ft_cpu_setup(void *blob, bd_t *bd)
+#ifdef CONFIG_OF_LIBFDT
+static void do_fixup(void *fdt, const char *node, const char *prop,
+                  const void *val, int len, int create)
+{
+#if defined(DEBUG)
+     int i;
+     debug("Updating property '%s/%s' = ", node, prop);
+     for (i = 0; i < len; i++)
+             debug(" %.2x", *(u8*)(val+i));
+     debug("\n");
+#endif

cpu/mpc5xxx/cpu.c unsigned long get_tbclk (void) · line 139

Quoted 10 lines

cpu/mpc5xxx/cpu.c +10 -0

+     int rc = fdt_find_and_setprop(fdt, node, prop, val, len, create);
+     if (rc)
+             printf("Unable to update property %s:%s, err=%s\n",
+                    node, prop, fdt_strerror(rc));
+}
+
+static void do_fixup_u32(void *fdt, const char *node, const char *prop,
+                      u32 val, int create)
+{
+     val = cpu_to_be32(val);

Shouldn't this be cpu_to_fdt32()? In such a case we need libfdt_env.h of course.

cpu/mpc5xxx/cpu.c unsigned long get_tbclk (void) · line 141

Quoted 2 lines

cpu/mpc5xxx/cpu.c +2 -0

+     do_fixup(fdt, node, prop, &val, sizeof(val), create);
+}

I don't think do_fixup() and do_fixup_u32() should be in cpu/mpc5xxx/cpu.c - they can and should be used by 83xx (and others) without modification. Is libfdt really out of the question because of printf() calls?

Quoted 28 lines · Bartlomiej Sieka

Quoted 14 lines

cpu/mpc5xxx/cpu.c +11 -3

-#ifdef CONFIG_OF_FLAT_TREE
-void
-ft_cpu_setup(void *blob, bd_t *bd)
+#ifdef CONFIG_OF_LIBFDT
+static void do_fixup(void *fdt, const char *node, const char *prop,
+                  const void *val, int len, int create)
+{
+#if defined(DEBUG)
+     int i;
+     debug("Updating property '%s/%s' = ", node, prop);
+     for (i = 0; i < len; i++)
+             debug(" %.2x", *(u8*)(val+i));
+     debug("\n");
+#endif

cpu/mpc5xxx/cpu.c unsigned long get_tbclk (void) · line 139

Quoted 10 lines

cpu/mpc5xxx/cpu.c +10 -0

+     int rc = fdt_find_and_setprop(fdt, node, prop, val, len, create);
+     if (rc)
+             printf("Unable to update property %s:%s, err=%s\n",
+                    node, prop, fdt_strerror(rc));
+}
+
+static void do_fixup_u32(void *fdt, const char *node, const char *prop,
+                      u32 val, int create)
+{
+     val = cpu_to_be32(val);

Shouldn't this be cpu_to_fdt32()? In such a case we need libfdt_env.h of course.

Quoted 7 lines

cpu/mpc5xxx/cpu.c unsigned long get_tbclk (void) · line 141

Quoted 2 lines

cpu/mpc5xxx/cpu.c +2 -0

+     do_fixup(fdt, node, prop, &val, sizeof(val), create);
+}

I don't think do_fixup() and do_fixup_u32() should be in cpu/mpc5xxx/cpu.c - they can and should be used by 83xx (and others) without modification. Is libfdt really out of the question because of printf() calls?

I think so. libfdt is *supposed* to be cross platform code that is used by u-boot, dtc, etc. Unfortunately there has already been drift in this regard, :-( I think libfdt functions should provide feedback via the return code, and let callers take care of formating output. The fixup functions in cpu.c is purely shorthand to keep the body of ft_setup_cpu tidy (so it doesn't need the same boilerplate for each function call to check the results), but I do agree that they are useful to more than just 5xxx. So where should they go? And as a secondary question, do we put them there now, or move them in the next merge window?

Cheers,
g.

--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely at secretlab.ca
(403) 399-0195