[U-Boot-Users] [PATCH 0/2] Move to LIBFDT for mpc5xxx platforms

Wolfgang, et al.
Here is hopefully the final version of migrating all the 5xxx boards to use LIBFDT. It has been tested on lite5200 and an earlier version of the patch was tested on motionpro. I'll wait a day to collect sign off and final comments, then I'll push it to the 5xxx tree.
Cheers, g.
-- Grant Likely, B.Sc. P.Eng. Secret Lab Technologies Ltd.

From: Grant Likely grant.likely@secretlab.ca
Given the path to a node, fdt_find_and_setprop() allows a property value to be set directly.
Signed-off-by: Grant Likely grant.likely@secretlab.ca ---
include/libfdt.h | 2 ++ libfdt/fdt_rw.c | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/include/libfdt.h b/include/libfdt.h index 340e89d..38c65a9 100644 --- a/include/libfdt.h +++ b/include/libfdt.h @@ -140,6 +140,8 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name, }) #define fdt_setprop_string(fdt, nodeoffset, name, str) \ fdt_setprop((fdt), (nodeoffset), (name), (str), strlen(str)+1) +int fdt_find_and_setprop(void *fdt, const char *node, const char *prop, + const void *val, int len, int create); int fdt_delprop(void *fdt, int nodeoffset, const char *name); int fdt_add_subnode_namelen(void *fdt, int parentoffset, const char *name, int namelen); diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c index 693bfe4..55fcc41 100644 --- a/libfdt/fdt_rw.c +++ b/libfdt/fdt_rw.c @@ -188,6 +188,32 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name, return 0; }
+/** + * fdt_find_and_setprop: Find a node and set it's property + * + * @fdt: ptr to device tree + * @node: path of node + * @prop: property name + * @val: ptr to new value + * @len: length of new property value + * @create: flag to create the property if it doesn't exist + * + * Convenience function to directly set a property given the path to the node. + */ +int fdt_find_and_setprop(void *fdt, const char *node, const char *prop, + const void *val, int len, int create) +{ + int nodeoff = fdt_find_node_by_path(fdt, node); + + if (nodeoff < 0) + return nodeoff; + + if ((!create) && (fdt_get_property(fdt, nodeoff, prop, 0) == NULL)) + return 0; /* create flag not set; so exit quietly */ + + return fdt_setprop(fdt, nodeoff, prop, val, len); +} + int fdt_delprop(void *fdt, int nodeoffset, const char *name) { struct fdt_property *prop;

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 ---
board/icecube/icecube.c | 7 +--- board/jupiter/jupiter.c | 8 +---- board/motionpro/motionpro.c | 8 ++--- board/tqm5200/tqm5200.c | 9 ++---- cpu/mpc5xxx/cpu.c | 69 +++++++++++++++++++++++++++---------------- include/configs/IceCube.h | 2 + include/configs/TQM5200.h | 2 + include/configs/jupiter.h | 2 + include/configs/motionpro.h | 2 + 9 files changed, 58 insertions(+), 51 deletions(-)
diff --git a/board/icecube/icecube.c b/board/icecube/icecube.c index c027f6f..07ba245 100644 --- a/board/icecube/icecube.c +++ b/board/icecube/icecube.c @@ -28,10 +28,7 @@ #include <mpc5xxx.h> #include <pci.h> #include <asm/processor.h> - -#if defined(CONFIG_OF_FLAT_TREE) -#include <ft_build.h> -#endif +#include <libfdt.h>
#if defined(CONFIG_LITE5200B) #include "mt46v32m16.h" @@ -386,7 +383,7 @@ void ide_set_reset (int idereset) } #endif
-#if defined(CONFIG_OF_FLAT_TREE) && defined(CONFIG_OF_BOARD_SETUP) +#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) void ft_board_setup(void *blob, bd_t *bd) { diff --git a/board/jupiter/jupiter.c b/board/jupiter/jupiter.c index b227487..efdc333 100644 --- a/board/jupiter/jupiter.c +++ b/board/jupiter/jupiter.c @@ -28,11 +28,7 @@ #include <mpc5xxx.h> #include <pci.h> #include <asm/processor.h> - -#if defined(CONFIG_OF_FLAT_TREE) -#include <ft_build.h> -#endif - +#include <libfdt.h>
#define SDRAM_DDR 0 #if 1 @@ -308,7 +304,7 @@ void ide_set_reset (int idereset) } #endif
-#if defined(CONFIG_OF_FLAT_TREE) && defined(CONFIG_OF_BOARD_SETUP) +#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) void ft_board_setup(void *blob, bd_t *bd) { diff --git a/board/motionpro/motionpro.c b/board/motionpro/motionpro.c index 6eb5fe9..f83998e 100644 --- a/board/motionpro/motionpro.c +++ b/board/motionpro/motionpro.c @@ -29,9 +29,7 @@ #include <common.h> #include <mpc5xxx.h> #include <miiphy.h> -#if defined(CONFIG_OF_FLAT_TREE) -#include <ft_build.h> -#endif +#include <libfdt.h>
#if defined(CONFIG_STATUS_LED) #include <status_led.h> @@ -196,12 +194,12 @@ int checkboard(void) }
-#if defined(CONFIG_OF_FLAT_TREE) && defined(CONFIG_OF_BOARD_SETUP) +#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) void ft_board_setup(void *blob, bd_t *bd) { ft_cpu_setup(blob, bd); } -#endif /* defined(CONFIG_OF_FLAT_TREE) && defined(CONFIG_OF_BOARD_SETUP) */ +#endif /* defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) */
#if defined(CONFIG_STATUS_LED) diff --git a/board/tqm5200/tqm5200.c b/board/tqm5200/tqm5200.c index 51f4aeb..21f67aa 100644 --- a/board/tqm5200/tqm5200.c +++ b/board/tqm5200/tqm5200.c @@ -31,10 +31,7 @@ #include <mpc5xxx.h> #include <pci.h> #include <asm/processor.h> - -#if defined(CONFIG_OF_FLAT_TREE) -#include <ft_build.h> -#endif +#include <libfdt.h>
#ifdef CONFIG_VIDEO_SM501 #include <sm501.h> @@ -780,9 +777,9 @@ int board_get_height (void)
#endif /* CONFIG_VIDEO_SM501 */
-#if defined(CONFIG_OF_FLAT_TREE) && defined(CONFIG_OF_BOARD_SETUP) +#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) void ft_board_setup(void *blob, bd_t *bd) { ft_cpu_setup(blob, bd); } -#endif /* defined(CONFIG_OF_FLAT_TREE) && defined(CONFIG_OF_BOARD_SETUP) */ +#endif /* defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) */ 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 + #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> #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); + do_fixup(fdt, node, prop, &val, sizeof(val), create); +} + +void ft_cpu_setup(void *blob, bd_t *bd) { - u32 *p; - int len; - - /* Core XLB bus frequency */ - p = ft_get_prop(blob, "/cpus/" OF_CPU "/bus-frequency", &len); - if (p != NULL) - *p = cpu_to_be32(bd->bi_busfreq); - - /* SOC peripherals use the IPB bus frequency */ - p = ft_get_prop(blob, "/" OF_SOC "/bus-frequency", &len); - if (p != NULL) - *p = cpu_to_be32(bd->bi_ipbfreq); - - p = ft_get_prop(blob, "/" OF_SOC "/ethernet@3000/mac-address", &len); - if (p != NULL) - memcpy(p, bd->bi_enetaddr, 6); - - p = ft_get_prop(blob, "/" OF_SOC "/ethernet@3000/local-mac-address", &len); - if (p != NULL) - memcpy(p, bd->bi_enetaddr, 6); + int div = in_8((void*)CFG_MBAR + 0x204) & 0x0020 ? 8 : 4; + char * cpu_path = "/cpus/" OF_CPU; + char * eth_path = "/" OF_SOC "/ethernet@3000"; + + do_fixup_u32(blob, cpu_path, "timebase-frequency", OF_TBCLK, 1); + do_fixup_u32(blob, cpu_path, "bus-frequency", bd->bi_busfreq, 1); + do_fixup_u32(blob, cpu_path, "clock-frequency", bd->bi_intfreq, 1); + do_fixup_u32(blob, "/" OF_SOC, "bus-frequency", bd->bi_ipbfreq, 1); + do_fixup_u32(blob, "/" OF_SOC, "system-frequency", + bd->bi_busfreq*div, 1); + do_fixup(blob, eth_path, "mac-address", bd->bi_enetaddr, 6, 0); + do_fixup(blob, eth_path, "local-mac-address", bd->bi_enetaddr, 6, 0); } #endif diff --git a/include/configs/IceCube.h b/include/configs/IceCube.h index bdd92ba..4c16d22 100644 --- a/include/configs/IceCube.h +++ b/include/configs/IceCube.h @@ -178,7 +178,7 @@ #endif /* CONFIG_MPC5200 */
/* pass open firmware flat tree */ -#define CONFIG_OF_FLAT_TREE 1 +#define CONFIG_OF_LIBFDT 1 #define CONFIG_OF_BOARD_SETUP 1
#define OF_CPU "PowerPC,5200@0" diff --git a/include/configs/TQM5200.h b/include/configs/TQM5200.h index c08173b..e0c9d81 100644 --- a/include/configs/TQM5200.h +++ b/include/configs/TQM5200.h @@ -701,7 +701,7 @@ * Open firmware flat tree support *----------------------------------------------------------------------- */ -#define CONFIG_OF_FLAT_TREE 1 +#define CONFIG_OF_LIBFDT 1 #define CONFIG_OF_BOARD_SETUP 1
#define OF_CPU "PowerPC,5200@0" diff --git a/include/configs/jupiter.h b/include/configs/jupiter.h index 4070ab9..b7100e9 100644 --- a/include/configs/jupiter.h +++ b/include/configs/jupiter.h @@ -145,7 +145,7 @@
#if 0 /* pass open firmware flat tree */ -#define CONFIG_OF_FLAT_TREE 1 +#define CONFIG_OF_LIBFDT 1 #define CONFIG_OF_BOARD_SETUP 1
#define OF_CPU "PowerPC,5200@0" diff --git a/include/configs/motionpro.h b/include/configs/motionpro.h index 82827c6..9a21632 100644 --- a/include/configs/motionpro.h +++ b/include/configs/motionpro.h @@ -417,7 +417,7 @@ extern void __led_set(led_id_t id, int state); #define CFG_RESET_ADDRESS 0xfff00100
/* pass open firmware flat tree */ -#define CONFIG_OF_FLAT_TREE 1 +#define CONFIG_OF_LIBFDT 1 #define CONFIG_OF_BOARD_SETUP 1
#define OF_CPU "PowerPC,5200@0"

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?
Regards, Bartlomiej

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.

On 9/4/07, Grant Likely grant.likely@secretlab.ca wrote:
On 9/4/07, Bartlomiej Sieka tur@semihalf.com wrote:
-#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?
... How about putting them in common/fdt_support.c?
Cheers, g.
Cheers, g.
-- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. grant.likely@secretlab.ca (403) 399-0195
participants (2)
-
Bartlomiej Sieka
-
Grant Likely