[U-Boot-Users] [PATCH 2/3] Fix fdt_chosen() to call ft_board_setup(), clean up long lines.
The fdt_chosen() function was adding/seting some properties ad-hoc improperly and duplicated (poorly) what was done in ft_board_setup()
Clean up long lines (setting properties, printing errors).
Signed-off-by: Gerald Van Baren vanbaren@cideas.com --- Kim:
Can I have an ack/nack on this?
Thanks, gvb
common/fdt_support.c | 113 ++++++++++++++++++++++++++++++++----------------- 1 files changed, 74 insertions(+), 39 deletions(-)
diff --git a/common/fdt_support.c b/common/fdt_support.c index efa63f0..d12c751 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -32,6 +32,10 @@ #include <libfdt.h> #include <fdt_support.h>
+#ifdef CONFIG_OF_BOARD_SETUP +void ft_board_setup(void *blob, bd_t *bd); +#endif + /* * Global data (for the gd->bd) */ @@ -42,7 +46,6 @@ DECLARE_GLOBAL_DATA_PTR; */ struct fdt_header *fdt;
- /********************************************************************/
int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) @@ -50,9 +53,8 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) bd_t *bd = gd->bd; int nodeoffset; int err; - u32 tmp; /* used to set 32 bit integer properties */ - char *str; /* used to set string properties */ - ulong clock; + u32 tmp; /* used to set 32 bit integer properties */ + char *str; /* used to set string properties */
err = fdt_check_header(fdt); if (err < 0) { @@ -60,6 +62,17 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) return err; }
+#ifdef CONFIG_OF_BOARD_SETUP + /* + * ft_board_setup() sets various board-specific properties to + * the proper values. + * + * STRICTLY SPEAKING, this is out of place, but it isn't clear + * where a better place would be. + */ + ft_board_setup(fdt, bd); +#endif + if (initrd_start && initrd_end) { struct fdt_reserve_entry re; int used; @@ -72,7 +85,8 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) return err; } if (used >= total) { - printf("WARNING fdt_chosen: no room in the reserved map (%d of %d)\n", + printf("WARNING fdt_chosen: " + "no room in the reserved map (%d of %d)\n", used, total); return -1; } @@ -115,7 +129,10 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) */ nodeoffset = fdt_add_subnode(fdt, 0, "chosen"); if (nodeoffset < 0) { - printf("WARNING fdt_chosen: could not create the "/chosen node" (libfdt error %s).\n", fdt_strerror(nodeoffset)); + printf("WARNING fdt_chosen: " + "could not create the "/chosen node" " + "(libfdt error %s).\n", + fdt_strerror(nodeoffset)); return nodeoffset; } } @@ -125,42 +142,42 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) */ str = getenv("bootargs"); if (str != NULL) { - err = fdt_setprop(fdt, nodeoffset, "bootargs", str, strlen(str)+1); + err = fdt_setprop(fdt, nodeoffset, + "bootargs", str, strlen(str)+1); if (err < 0) - printf("WARNING fdt_chosen: could not set "bootargs" (libfdt error %s).\n", fdt_strerror(err)); + printf("WARNING fdt_chosen: " + "could not set "bootargs" " + "(libfdt error %s).\n", + fdt_strerror(err)); } if (initrd_start && initrd_end) { tmp = __cpu_to_be32(initrd_start); - err = fdt_setprop(fdt, nodeoffset, "linux,initrd-start", &tmp, sizeof(tmp)); + err = fdt_setprop(fdt, nodeoffset, + "linux,initrd-start", &tmp, sizeof(tmp)); if (err < 0) - printf("WARNING fdt_chosen: could not set "linux,initrd-start" (libfdt error %s).\n", fdt_strerror(err)); + printf("WARNING fdt_chosen: " + "could not set "linux,initrd-start" " + "(libfdt error %s).\n", + fdt_strerror(err)); tmp = __cpu_to_be32(initrd_end); - err = fdt_setprop(fdt, nodeoffset, "linux,initrd-end", &tmp, sizeof(tmp)); + err = fdt_setprop(fdt, nodeoffset, + "linux,initrd-end", &tmp, sizeof(tmp)); if (err < 0) - printf("WARNING fdt_chosen: could not set "linux,initrd-end" (libfdt error %s).\n", fdt_strerror(err)); + printf("WARNING fdt_chosen: " + "could not set "linux,initrd-end" " + "(libfdt error %s).\n", + fdt_strerror(err)); } #ifdef OF_STDOUT_PATH - err = fdt_setprop(fdt, nodeoffset, "linux,stdout-path", OF_STDOUT_PATH, strlen(OF_STDOUT_PATH)+1); + err = fdt_setprop(fdt, nodeoffset, + "linux,stdout-path", OF_STDOUT_PATH, strlen(OF_STDOUT_PATH)+1); if (err < 0) - printf("WARNING fdt_chosen: could not set "linux,stdout-path" (libfdt error %s).\n", fdt_strerror(err)); + printf("WARNING fdt_chosen: " + "could not set "linux,stdout-path" " + "(libfdt error %s).\n", + fdt_strerror(err)); #endif
- nodeoffset = fdt_find_node_by_path (fdt, "/cpus"); - if (nodeoffset >= 0) { - clock = cpu_to_be32(bd->bi_intfreq); - err = fdt_setprop(fdt, nodeoffset, "clock-frequency", &clock, 4); - if (err < 0) - printf("WARNING fdt_chosen: could not set "clock-frequency" (libfdt error %s).\n", fdt_strerror(err)); - } -#ifdef OF_TBCLK - nodeoffset = fdt_find_node_by_path (fdt, "/cpus/" OF_CPU "/timebase-frequency"); - if (nodeoffset >= 0) { - clock = cpu_to_be32(OF_TBCLK); - err = fdt_setprop(fdt, nodeoffset, "clock-frequency", &clock, 4); - if (err < 0) - printf("WARNING fdt_chosen: could not set "clock-frequency" (libfdt error %s).\n", fdt_strerror(err)); - } -#endif return err; }
@@ -203,7 +220,10 @@ int fdt_env(void *fdt) */ nodeoffset = fdt_add_subnode(fdt, 0, "u-boot-env"); if (nodeoffset < 0) { - printf("WARNING fdt_env: could not create the "/u-boot-env node" (libfdt error %s).\n", fdt_strerror(nodeoffset)); + printf("WARNING fdt_env: " + "could not create the "/u-boot-env node" " + "(libfdt error %s).\n", + fdt_strerror(nodeoffset)); return nodeoffset; }
@@ -231,7 +251,10 @@ int fdt_env(void *fdt) continue; err = fdt_setprop(fdt, nodeoffset, lval, rval, strlen(rval)+1); if (err < 0) { - printf("WARNING fdt_env: could not set "%s" (libfdt error %s).\n", lval, fdt_strerror(err)); + printf("WARNING fdt_env: " + "could not set "%s" " + "(libfdt error %s).\n", + lval, fdt_strerror(err)); return err; } } @@ -297,7 +320,7 @@ int fdt_bd_t(void *fdt) bd_t *bd = gd->bd; int nodeoffset; int err; - u32 tmp; /* used to set 32 bit integer properties */ + u32 tmp; /* used to set 32 bit integer properties */ int i;
err = fdt_check_header(fdt); @@ -323,7 +346,10 @@ int fdt_bd_t(void *fdt) */ nodeoffset = fdt_add_subnode(fdt, 0, "bd_t"); if (nodeoffset < 0) { - printf("WARNING fdt_bd_t: could not create the "/bd_t node" (libfdt error %s).\n", fdt_strerror(nodeoffset)); + printf("WARNING fdt_bd_t: " + "could not create the "/bd_t node" " + "(libfdt error %s).\n", + fdt_strerror(nodeoffset)); printf("libfdt: %s\n", fdt_strerror(nodeoffset)); return nodeoffset; } @@ -332,20 +358,29 @@ int fdt_bd_t(void *fdt) */ for (i = 0; i < sizeof(bd_map)/sizeof(bd_map[0]); i++) { tmp = cpu_to_be32(getenv("bootargs")); - err = fdt_setprop(fdt, nodeoffset, bd_map[i].name, &tmp, sizeof(tmp)); + err = fdt_setprop(fdt, nodeoffset, + bd_map[i].name, &tmp, sizeof(tmp)); if (err < 0) - printf("WARNING fdt_bd_t: could not set "%s" (libfdt error %s).\n", bd_map[i].name, fdt_strerror(err)); + printf("WARNING fdt_bd_t: " + "could not set "%s" " + "(libfdt error %s).\n", + bd_map[i].name, fdt_strerror(err)); } /* * Add a couple of oddball entries... */ err = fdt_setprop(fdt, nodeoffset, "enetaddr", &bd->bi_enetaddr, 6); if (err < 0) - printf("WARNING fdt_bd_t: could not set "enetaddr" (libfdt error %s).\n", fdt_strerror(err)); + printf("WARNING fdt_bd_t: " + "could not set "enetaddr" " + "(libfdt error %s).\n", + fdt_strerror(err)); err = fdt_setprop(fdt, nodeoffset, "ethspeed", &bd->bi_ethspeed, 4); if (err < 0) - printf("WARNING fdt_bd_t: could not set "ethspeed" (libfdt error %s).\n", fdt_strerror(err)); - + printf("WARNING fdt_bd_t: " + "could not set "ethspeed" " + "(libfdt error %s).\n", + fdt_strerror(err)); return 0; } #endif /* ifdef CONFIG_OF_HAS_BD_T */
On Sat, 26 May 2007 08:52:31 -0400 Jerry Van Baren gvb.uboot@gmail.com wrote:
The fdt_chosen() function was adding/seting some properties ad-hoc improperly and duplicated (poorly) what was done in ft_board_setup()
Clean up long lines (setting properties, printing errors).
Signed-off-by: Gerald Van Baren vanbaren@cideas.com
Kim:
Can I have an ack/nack on this?
Thanks, gvb
common/fdt_support.c | 113 ++++++++++++++++++++++++++++++++----------------- 1 files changed, 74 insertions(+), 39 deletions(-)
diff --git a/common/fdt_support.c b/common/fdt_support.c index efa63f0..d12c751 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -32,6 +32,10 @@ #include <libfdt.h> #include <fdt_support.h>
+#ifdef CONFIG_OF_BOARD_SETUP +void ft_board_setup(void *blob, bd_t *bd); +#endif
/*
- Global data (for the gd->bd)
*/ @@ -42,7 +46,6 @@ DECLARE_GLOBAL_DATA_PTR; */ struct fdt_header *fdt;
/********************************************************************/
int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) @@ -50,9 +53,8 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) bd_t *bd = gd->bd; int nodeoffset; int err;
- u32 tmp; /* used to set 32 bit integer properties */
- char *str; /* used to set string properties */
- ulong clock;
u32 tmp; /* used to set 32 bit integer properties */
char *str; /* used to set string properties */
err = fdt_check_header(fdt); if (err < 0) {
@@ -60,6 +62,17 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) return err; }
+#ifdef CONFIG_OF_BOARD_SETUP
- /*
* ft_board_setup() sets various board-specific properties to* the proper values.** STRICTLY SPEAKING, this is out of place, but it isn't clear* where a better place would be.*/- ft_board_setup(fdt, bd);
+#endif
I think it's fine here, btw. What are your reservations?
if (initrd_start && initrd_end) { struct fdt_reserve_entry re; int used; @@ -72,7 +85,8 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) return err; } if (used >= total) {
printf("WARNING fdt_chosen: no room in the reserved map (%d of %d)\n",
printf("WARNING fdt_chosen: ""no room in the reserved map (%d of %d)\n", used, total); return -1;@@ -115,7 +129,10 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) */ nodeoffset = fdt_add_subnode(fdt, 0, "chosen"); if (nodeoffset < 0) {
printf("WARNING fdt_chosen: could not create the \"/chosen node\" (libfdt error %s).\n", fdt_strerror(nodeoffset));
printf("WARNING fdt_chosen: ""could not create the \"/chosen node\" ""(libfdt error %s).\n",fdt_strerror(nodeoffset)); return nodeoffset;@@ -125,42 +142,42 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) */ str = getenv("bootargs"); if (str != NULL) {
err = fdt_setprop(fdt, nodeoffset, "bootargs", str, strlen(str)+1);
err = fdt_setprop(fdt, nodeoffset,"bootargs", str, strlen(str)+1);
printf("WARNING fdt_chosen: could not set \"bootargs\" (libfdt error %s).\n", fdt_strerror(err));
printf("WARNING fdt_chosen: ""could not set \"bootargs\" ""(libfdt error %s).\n",fdt_strerror(err));
err = fdt_setprop(fdt, nodeoffset, "linux,initrd-start", &tmp, sizeof(tmp));
err = fdt_setprop(fdt, nodeoffset,"linux,initrd-start", &tmp, sizeof(tmp));
printf("WARNING fdt_chosen: could not set \"linux,initrd-start\" (libfdt error %s).\n", fdt_strerror(err));
printf("WARNING fdt_chosen: ""could not set \"linux,initrd-start\" ""(libfdt error %s).\n",fdt_strerror(err));
err = fdt_setprop(fdt, nodeoffset, "linux,initrd-end", &tmp, sizeof(tmp));
err = fdt_setprop(fdt, nodeoffset,"linux,initrd-end", &tmp, sizeof(tmp));
printf("WARNING fdt_chosen: could not set \"linux,initrd-end\" (libfdt error %s).\n", fdt_strerror(err));
printf("WARNING fdt_chosen: ""could not set \"linux,initrd-end\" ""(libfdt error %s).\n",fdt_strerror(err));#ifdef OF_STDOUT_PATH
- err = fdt_setprop(fdt, nodeoffset, "linux,stdout-path", OF_STDOUT_PATH, strlen(OF_STDOUT_PATH)+1);
- err = fdt_setprop(fdt, nodeoffset,
"linux,stdout-path", OF_STDOUT_PATH, strlen(OF_STDOUT_PATH)+1);
printf("WARNING fdt_chosen: could not set \"linux,stdout-path\" (libfdt error %s).\n", fdt_strerror(err));
printf("WARNING fdt_chosen: ""could not set \"linux,stdout-path\" ""(libfdt error %s).\n",fdt_strerror(err));#endif
- nodeoffset = fdt_find_node_by_path (fdt, "/cpus");
- if (nodeoffset >= 0) {
clock = cpu_to_be32(bd->bi_intfreq);err = fdt_setprop(fdt, nodeoffset, "clock-frequency", &clock, 4);if (err < 0)printf("WARNING fdt_chosen: could not set \"clock-frequency\" (libfdt error %s).\n", fdt_strerror(err));- }
-#ifdef OF_TBCLK
- nodeoffset = fdt_find_node_by_path (fdt, "/cpus/" OF_CPU "/timebase-frequency");
- if (nodeoffset >= 0) {
clock = cpu_to_be32(OF_TBCLK);err = fdt_setprop(fdt, nodeoffset, "clock-frequency", &clock, 4);if (err < 0)printf("WARNING fdt_chosen: could not set \"clock-frequency\" (libfdt error %s).\n", fdt_strerror(err));- }
-#endif return err; }
@@ -203,7 +220,10 @@ int fdt_env(void *fdt) */ nodeoffset = fdt_add_subnode(fdt, 0, "u-boot-env"); if (nodeoffset < 0) {
printf("WARNING fdt_env: could not create the \"/u-boot-env node\" (libfdt error %s).\n", fdt_strerror(nodeoffset));
printf("WARNING fdt_env: ""could not create the \"/u-boot-env node\" ""(libfdt error %s).\n",fdt_strerror(nodeoffset));@@ -231,7 +251,10 @@ int fdt_env(void *fdt) continue; err = fdt_setprop(fdt, nodeoffset, lval, rval, strlen(rval)+1); if (err < 0) {
printf("WARNING fdt_env: could not set \"%s\" (libfdt error %s).\n", lval, fdt_strerror(err));
printf("WARNING fdt_env: ""could not set \"%s\" ""(libfdt error %s).\n",lval, fdt_strerror(err)); return err;@@ -297,7 +320,7 @@ int fdt_bd_t(void *fdt) bd_t *bd = gd->bd; int nodeoffset; int err;
- u32 tmp; /* used to set 32 bit integer properties */
u32 tmp; /* used to set 32 bit integer properties */ int i;
err = fdt_check_header(fdt);
@@ -323,7 +346,10 @@ int fdt_bd_t(void *fdt) */ nodeoffset = fdt_add_subnode(fdt, 0, "bd_t"); if (nodeoffset < 0) {
printf("WARNING fdt_bd_t: could not create the \"/bd_t node\" (libfdt error %s).\n", fdt_strerror(nodeoffset));
printf("WARNING fdt_bd_t: ""could not create the \"/bd_t node\" ""(libfdt error %s).\n",fdt_strerror(nodeoffset));
just in case the user didn't get it the first time, eh? ;)
return nodeoffset;} @@ -332,20 +358,29 @@ int fdt_bd_t(void *fdt) */ for (i = 0; i < sizeof(bd_map)/sizeof(bd_map[0]); i++) { tmp = cpu_to_be32(getenv("bootargs"));
err = fdt_setprop(fdt, nodeoffset, bd_map[i].name, &tmp, sizeof(tmp));
err = fdt_setprop(fdt, nodeoffset,bd_map[i].name, &tmp, sizeof(tmp));
printf("WARNING fdt_bd_t: could not set \"%s\" (libfdt error %s).\n", bd_map[i].name, fdt_strerror(err));
printf("WARNING fdt_bd_t: ""could not set \"%s\" ""(libfdt error %s).\n",bd_map[i].name, fdt_strerror(err));*/ err = fdt_setprop(fdt, nodeoffset, "enetaddr", &bd->bi_enetaddr, 6); if (err < 0)
- Add a couple of oddball entries...
printf("WARNING fdt_bd_t: could not set \"enetaddr\" (libfdt error %s).\n", fdt_strerror(err));
printf("WARNING fdt_bd_t: ""could not set \"enetaddr\" ""(libfdt error %s).\n",fdt_strerror(err));
printf("WARNING fdt_bd_t: could not set \"ethspeed\" (libfdt error %s).\n", fdt_strerror(err));
printf("WARNING fdt_bd_t: ""could not set \"ethspeed\" "
[these comments apply to all the similar hunks above:]
putting escaped quotes in user messages just makes it harder for the user to grep for error text in the code, IMO. you can achieve the same level of "message correctness" by postpending the word property.
"(libfdt error %s).\n",
s/libfdt error //g
fdt_strerror(err));
Kim
Kim Phillips wrote:
On Sat, 26 May 2007 08:52:31 -0400 Jerry Van Baren gvb.uboot@gmail.com wrote:
The fdt_chosen() function was adding/seting some properties ad-hoc improperly and duplicated (poorly) what was done in ft_board_setup()
Clean up long lines (setting properties, printing errors).
Signed-off-by: Gerald Van Baren vanbaren@cideas.com
Kim:
Can I have an ack/nack on this?
Thanks, gvb
common/fdt_support.c | 113 ++++++++++++++++++++++++++++++++----------------- 1 files changed, 74 insertions(+), 39 deletions(-)
diff --git a/common/fdt_support.c b/common/fdt_support.c index efa63f0..d12c751 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -32,6 +32,10 @@ #include <libfdt.h> #include <fdt_support.h>
+#ifdef CONFIG_OF_BOARD_SETUP +void ft_board_setup(void *blob, bd_t *bd); +#endif
/*
- Global data (for the gd->bd)
*/ @@ -42,7 +46,6 @@ DECLARE_GLOBAL_DATA_PTR; */ struct fdt_header *fdt;
/********************************************************************/
int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) @@ -50,9 +53,8 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) bd_t *bd = gd->bd; int nodeoffset; int err;
- u32 tmp; /* used to set 32 bit integer properties */
- char *str; /* used to set string properties */
- ulong clock;
u32 tmp; /* used to set 32 bit integer properties */
char *str; /* used to set string properties */
err = fdt_check_header(fdt); if (err < 0) {
@@ -60,6 +62,17 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) return err; }
+#ifdef CONFIG_OF_BOARD_SETUP
- /*
* ft_board_setup() sets various board-specific properties to* the proper values.** STRICTLY SPEAKING, this is out of place, but it isn't clear* where a better place would be.*/- ft_board_setup(fdt, bd);
+#endif
I think it's fine here, btw. What are your reservations?
Not really. ft_board_setup() is not related at all to creating the /chosen node. I've been thinking about this and believe the best approach is to create another "fdt" command "fdt boardsetup" that calls ft_board_setup().
The user can then run each part of the fdt setup separately and watch what is done to the blob:
fdt boardsetup - updates the board-specific fdt values fdt chosen - creates or updates the /chosen node fdt env - creates the /u-boot-env node fdt bd_t - creates the /bd_t node
The cmd_bootm.c will then call all four. Currently it calls the last 3 and fdt_chosen() calls fdt_board_setup() inappropriately IMHO. That is my reservation.
if (initrd_start && initrd_end) { struct fdt_reserve_entry re; int used; @@ -72,7 +85,8 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) return err; } if (used >= total) {
printf("WARNING fdt_chosen: no room in the reserved map (%d of %d)\n",
printf("WARNING fdt_chosen: ""no room in the reserved map (%d of %d)\n", used, total); return -1;@@ -115,7 +129,10 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) */ nodeoffset = fdt_add_subnode(fdt, 0, "chosen"); if (nodeoffset < 0) {
printf("WARNING fdt_chosen: could not create the \"/chosen node\" (libfdt error %s).\n", fdt_strerror(nodeoffset));
printf("WARNING fdt_chosen: ""could not create the \"/chosen node\" ""(libfdt error %s).\n",fdt_strerror(nodeoffset)); return nodeoffset;@@ -125,42 +142,42 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force) */ str = getenv("bootargs"); if (str != NULL) {
err = fdt_setprop(fdt, nodeoffset, "bootargs", str, strlen(str)+1);
err = fdt_setprop(fdt, nodeoffset,"bootargs", str, strlen(str)+1);
printf("WARNING fdt_chosen: could not set \"bootargs\" (libfdt error %s).\n", fdt_strerror(err));
printf("WARNING fdt_chosen: ""could not set \"bootargs\" ""(libfdt error %s).\n",fdt_strerror(err));
err = fdt_setprop(fdt, nodeoffset, "linux,initrd-start", &tmp, sizeof(tmp));
err = fdt_setprop(fdt, nodeoffset,"linux,initrd-start", &tmp, sizeof(tmp));
printf("WARNING fdt_chosen: could not set \"linux,initrd-start\" (libfdt error %s).\n", fdt_strerror(err));
printf("WARNING fdt_chosen: ""could not set \"linux,initrd-start\" ""(libfdt error %s).\n",fdt_strerror(err));
err = fdt_setprop(fdt, nodeoffset, "linux,initrd-end", &tmp, sizeof(tmp));
err = fdt_setprop(fdt, nodeoffset,"linux,initrd-end", &tmp, sizeof(tmp));
printf("WARNING fdt_chosen: could not set \"linux,initrd-end\" (libfdt error %s).\n", fdt_strerror(err));
printf("WARNING fdt_chosen: ""could not set \"linux,initrd-end\" ""(libfdt error %s).\n",fdt_strerror(err));#ifdef OF_STDOUT_PATH
- err = fdt_setprop(fdt, nodeoffset, "linux,stdout-path", OF_STDOUT_PATH, strlen(OF_STDOUT_PATH)+1);
- err = fdt_setprop(fdt, nodeoffset,
"linux,stdout-path", OF_STDOUT_PATH, strlen(OF_STDOUT_PATH)+1);
printf("WARNING fdt_chosen: could not set \"linux,stdout-path\" (libfdt error %s).\n", fdt_strerror(err));
printf("WARNING fdt_chosen: ""could not set \"linux,stdout-path\" ""(libfdt error %s).\n",fdt_strerror(err));#endif
- nodeoffset = fdt_find_node_by_path (fdt, "/cpus");
- if (nodeoffset >= 0) {
clock = cpu_to_be32(bd->bi_intfreq);err = fdt_setprop(fdt, nodeoffset, "clock-frequency", &clock, 4);if (err < 0)printf("WARNING fdt_chosen: could not set \"clock-frequency\" (libfdt error %s).\n", fdt_strerror(err));- }
-#ifdef OF_TBCLK
- nodeoffset = fdt_find_node_by_path (fdt, "/cpus/" OF_CPU "/timebase-frequency");
- if (nodeoffset >= 0) {
clock = cpu_to_be32(OF_TBCLK);err = fdt_setprop(fdt, nodeoffset, "clock-frequency", &clock, 4);if (err < 0)printf("WARNING fdt_chosen: could not set \"clock-frequency\" (libfdt error %s).\n", fdt_strerror(err));- }
-#endif return err; }
@@ -203,7 +220,10 @@ int fdt_env(void *fdt) */ nodeoffset = fdt_add_subnode(fdt, 0, "u-boot-env"); if (nodeoffset < 0) {
printf("WARNING fdt_env: could not create the \"/u-boot-env node\" (libfdt error %s).\n", fdt_strerror(nodeoffset));
printf("WARNING fdt_env: ""could not create the \"/u-boot-env node\" ""(libfdt error %s).\n",fdt_strerror(nodeoffset));@@ -231,7 +251,10 @@ int fdt_env(void *fdt) continue; err = fdt_setprop(fdt, nodeoffset, lval, rval, strlen(rval)+1); if (err < 0) {
printf("WARNING fdt_env: could not set \"%s\" (libfdt error %s).\n", lval, fdt_strerror(err));
printf("WARNING fdt_env: ""could not set \"%s\" ""(libfdt error %s).\n",lval, fdt_strerror(err)); return err;@@ -297,7 +320,7 @@ int fdt_bd_t(void *fdt) bd_t *bd = gd->bd; int nodeoffset; int err;
- u32 tmp; /* used to set 32 bit integer properties */
u32 tmp; /* used to set 32 bit integer properties */ int i;
err = fdt_check_header(fdt);
@@ -323,7 +346,10 @@ int fdt_bd_t(void *fdt) */ nodeoffset = fdt_add_subnode(fdt, 0, "bd_t"); if (nodeoffset < 0) {
printf("WARNING fdt_bd_t: could not create the \"/bd_t node\" (libfdt error %s).\n", fdt_strerror(nodeoffset));
printf("WARNING fdt_bd_t: ""could not create the \"/bd_t node\" ""(libfdt error %s).\n",fdt_strerror(nodeoffset));just in case the user didn't get it the first time, eh? ;)
No, the three routines are called separately by fdt * commands, so each could fill the blob's available space separately.
The bootm commands call the three sequentially and aborts on the first one that fails. The chosen node could be created successfully but the u-boot-env node could run out of space.
Did that cover your ;) or did I miss the point?
return nodeoffset;} @@ -332,20 +358,29 @@ int fdt_bd_t(void *fdt) */ for (i = 0; i < sizeof(bd_map)/sizeof(bd_map[0]); i++) { tmp = cpu_to_be32(getenv("bootargs"));
err = fdt_setprop(fdt, nodeoffset, bd_map[i].name, &tmp, sizeof(tmp));
err = fdt_setprop(fdt, nodeoffset,bd_map[i].name, &tmp, sizeof(tmp));
printf("WARNING fdt_bd_t: could not set \"%s\" (libfdt error %s).\n", bd_map[i].name, fdt_strerror(err));
printf("WARNING fdt_bd_t: ""could not set \"%s\" ""(libfdt error %s).\n",bd_map[i].name, fdt_strerror(err));*/ err = fdt_setprop(fdt, nodeoffset, "enetaddr", &bd->bi_enetaddr, 6); if (err < 0)
- Add a couple of oddball entries...
printf("WARNING fdt_bd_t: could not set \"enetaddr\" (libfdt error %s).\n", fdt_strerror(err));
printf("WARNING fdt_bd_t: ""could not set \"enetaddr\" ""(libfdt error %s).\n",fdt_strerror(err));
printf("WARNING fdt_bd_t: could not set \"ethspeed\" (libfdt error %s).\n", fdt_strerror(err));
printf("WARNING fdt_bd_t: ""could not set \"ethspeed\" "[these comments apply to all the similar hunks above:]
putting escaped quotes in user messages just makes it harder for the user to grep for error text in the code, IMO. you can achieve the same level of "message correctness" by postpending the word property.
"(libfdt error %s).\n",s/libfdt error //g
fdt_strerror(err));
Yup, more creeping ASCII.
Kim
Thanks, gvb
On Tue, 29 May 2007 22:00:25 -0400 Jerry Van Baren gvb.uboot@gmail.com wrote:
Kim Phillips wrote:
On Sat, 26 May 2007 08:52:31 -0400 Jerry Van Baren gvb.uboot@gmail.com wrote:
<snip>
+#ifdef CONFIG_OF_BOARD_SETUP
- /*
* ft_board_setup() sets various board-specific properties to* the proper values.** STRICTLY SPEAKING, this is out of place, but it isn't clear* where a better place would be.*/- ft_board_setup(fdt, bd);
+#endif
I think it's fine here, btw. What are your reservations?
Not really. ft_board_setup() is not related at all to creating the /chosen node. I've been thinking about this and believe the best approach is to create another "fdt" command "fdt boardsetup" that calls ft_board_setup().
The user can then run each part of the fdt setup separately and watch what is done to the blob:
fdt boardsetup - updates the board-specific fdt values fdt chosen - creates or updates the /chosen node fdt env - creates the /u-boot-env node fdt bd_t - creates the /bd_t node
The cmd_bootm.c will then call all four. Currently it calls the last 3 and fdt_chosen() calls fdt_board_setup() inappropriately IMHO. That is my reservation.
sounds like you've found the better place.
<snip>
@@ -323,7 +346,10 @@ int fdt_bd_t(void *fdt) */ nodeoffset = fdt_add_subnode(fdt, 0, "bd_t"); if (nodeoffset < 0) {
printf("WARNING fdt_bd_t: could not create the \"/bd_t node\" (libfdt error %s).\n", fdt_strerror(nodeoffset));
printf("WARNING fdt_bd_t: ""could not create the \"/bd_t node\" ""(libfdt error %s).\n",fdt_strerror(nodeoffset));just in case the user didn't get it the first time, eh? ;)
No, the three routines are called separately by fdt * commands, so each could fill the blob's available space separately.
The bootm commands call the three sequentially and aborts on the first one that fails. The chosen node could be created successfully but the u-boot-env node could run out of space.
Did that cover your ;) or did I miss the point?
You're printf'ing the same thing twice.
Kim
participants (2)
-
Jerry Van Baren -
Kim Phillips