On 9/4/07, Bartlomiej Sieka tur@semihalf.com wrote:
Grant Likely wrote:
From: Grant Likely grant.likely@secretlab.ca
Affects boards: icecube (lite5200), jupiter, motionpro, tqm5200
Tested on: lite5200b
Signed-off-by: Grant Likely grant.likely@secretlab.ca
Hi Grant,
Thanks for including motionpro changes with this patch; see my comments below.
[...]
diff --git a/cpu/mpc5xxx/cpu.c b/cpu/mpc5xxx/cpu.c index 1eac2bb..e8a928a 100644 --- a/cpu/mpc5xxx/cpu.c +++ b/cpu/mpc5xxx/cpu.c @@ -25,14 +25,18 @@
- CPU specific code for the MPC5xxx CPUs
*/
+#undef DEBUG
Is this needed?
#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?
#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
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.
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.