[U-Boot-Users] [PATCH][RFC] Add common memory fixup function

Guys,
Here's my cut at a more generic version of Martin's patch that respects #ac and #sc.
Let me know what you think and if you have any comments. I'll add the board bits in a complete patch if this looks ok.
- k
diff --git a/common/fdt_support.c b/common/fdt_support.c index c67bb3d..e6de10f 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -441,6 +441,66 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat, do_fixup_by_compat(fdt, compat, prop, &val, 4, create); }
+int fdt_fixup_memory(void *blob, u64 start, u64 size) +{ + int i, err, nodeoffset, len = 0; + u8 tmp[16]; + const u32 *addrcell, *sizecell; + + err = fdt_check_header(blob); + if (err < 0) { + printf("%s: %s\n", __FUNCTION__, fdt_strerror(err)); + return err; + } + + /* update, or add and update /memory node */ + nodeoffset = fdt_path_offset(blob, "/memory"); + if (nodeoffset < 0) { + nodeoffset = fdt_add_subnode(blob, 0, "memory"); + if (nodeoffset < 0) + printf("WARNING: could not create /memory: %s.\n", + fdt_strerror(nodeoffset)); + return nodeoffset; + } + err = fdt_setprop(blob, nodeoffset, "device_type", "memory", + sizeof("memory")); + if (err < 0) { + printf("WARNING: could not set %s %s.\n", "device_type", + fdt_strerror(err)); + return err; + } + + addrcell = fdt_getprop(blob, 0, "#address-cells", NULL); + if ((addrcell) && (*addrcell == 2)) { + for (i = 0; i <= 7; i++) + tmp[i] = (start >> ((7 - i) * 8)) & 0xff; + len = 8; + } else { + for (i = 0; i <= 3; i++) + tmp[i] = (start >> ((3 - i) * 8)) & 0xff; + len = 4; + } + + sizecell = fdt_getprop(blob, 0, "#size-cells", NULL); + if ((sizecell) && (*sizecell == 2)) { + for (i = 0; i <= 7; i++) + tmp[i+len] = (size >> ((7 - i) * 8)) & 0xff; + len += 8; + } else { + for (i = 0; i <= 3; i++) + tmp[i+len] = (size >> ((3 - i) * 8)) & 0xff; + len += 4; + } + + err = fdt_setprop(fdt, nodeoffset, "reg", tmp, len); + if (err < 0) { + printf("WARNING: could not set %s %s.\n", + "reg", fdt_strerror(err)); + return err; + } + return 0; +} + void fdt_fixup_ethernet(void *fdt, bd_t *bd) { int node; diff --git a/include/fdt_support.h b/include/fdt_support.h index 8f781d4..3d6c1a8 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -44,6 +44,7 @@ void do_fixup_by_compat(void *fdt, const char *compat, const char *prop, const void *val, int len, int create); void do_fixup_by_compat_u32(void *fdt, const char *compat, const char *prop, u32 val, int create); +int fdt_fixup_memory(void *blob, u64 start, u64 size); void fdt_fixup_ethernet(void *fdt, bd_t *bd);
#ifdef CONFIG_OF_HAS_UBOOT_ENV

On Mon, 26 Nov 2007 14:55:46 -0600 (CST) Kumar Gala wrote:
Guys,
Here's my cut at a more generic version of Martin's patch that respects #ac and #sc.
Let me know what you think and if you have any comments. I'll add the board bits in a complete patch if this looks ok.
funny, I did the same with slightly different approach. below goes my version...
I think we are different only in #ac/#sc accounting, so let the people consider the best approach :)
Or... am I wrong in something? ----
fdt: add common memory fixup function
From: Martin Krause martin.krause@tqs.de
Add the function fdt_memory() to fixup the /memory node of the fdt with the memory values detected by U-Boot (taken from bd->bi_memstart and bd->bi_memsize).
The new function is called for all boards which define CONFIG_OF_LIBFDT.
This patch removes already existing board specific memory fixup routines for boards wich have CONFIG_OF_LIBFDT defined and switches them to the new routine. Boards wich use the CONIFG_OF_FLAT_TREE method are not touched.
Signed-off-by: Martin Krause martin.krause@tqs.de Signed-off-by: Vitaly Bordug vitb@kernel.crashing.org ---
README | 9 +++++ board/cds/common/ft_board.c | 9 ----- board/cm5200/cm5200.c | 23 ++----------- common/cmd_bootm.c | 12 +++++++ common/fdt_support.c | 75 +++++++++++++++++++++++++++++++++++++++++++ cpu/mpc83xx/cpu.c | 20 ++--------- include/fdt_support.h | 1 + 7 files changed, 103 insertions(+), 46 deletions(-)
diff --git a/README b/README index 3dad5fc..461f6b1 100644 --- a/README +++ b/README @@ -335,6 +335,15 @@ The following options need to be configured: passed using flattened device trees (based on open firmware concepts).
+ CONFIG_OF_MEMORY_FIXUP + + While using CONFIG_OF_LIBFDT above, each board needs special + /memory node set up in fdt with proper values. This config + option makes it auto-updated during bootm command. In some + cases, when HW requires "specific" crafting of /memory node, + or it is desired to have this step processed from BSP part + explicitly, this option should be disabled. + CONFIG_OF_LIBFDT * New libfdt-based support * Adds the "fdt" command diff --git a/board/cds/common/ft_board.c b/board/cds/common/ft_board.c index 3eda100..0c3c0f3 100644 --- a/board/cds/common/ft_board.c +++ b/board/cds/common/ft_board.c @@ -63,20 +63,11 @@ static void cds_pci_fixup(void *blob) void ft_board_setup(void *blob, bd_t *bd) { - u32 *p; - int len; - #ifdef CONFIG_PCI ft_pci_setup(blob, bd); #endif ft_cpu_setup(blob, bd);
- p = ft_get_prop(blob, "/memory/reg", &len); - if (p != NULL) { - *p++ = cpu_to_be32(bd->bi_memstart); - *p = cpu_to_be32(bd->bi_memsize); - } - cds_pci_fixup(blob); } #endif diff --git a/board/cm5200/cm5200.c b/board/cm5200/cm5200.c index e2ab5b8..13150f4 100644 --- a/board/cm5200/cm5200.c +++ b/board/cm5200/cm5200.c @@ -256,14 +256,13 @@ static void compose_hostname(hw_id_t hw_id, char *buf)
#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_OF_LIBFDT) /* - * Update 'model' and 'memory' properties in the blob according to the module - * that we are running on. + * Update 'model' property in the blob according to the module that we are + * running on. */ static void ft_blob_update(void *blob, bd_t *bd) { int len, ret, nodeoffset = 0; char module_name[MODULE_NAME_MAXLEN] = {0}; - ulong memory_data[2] = {0};
compose_module_name(hw_id, module_name); len = strlen(module_name) + 1; @@ -272,23 +271,6 @@ static void ft_blob_update(void *blob, bd_t *bd) if (ret < 0) printf("ft_blob_update(): cannot set /model property err:%s\n", fdt_strerror(ret)); - - memory_data[0] = cpu_to_be32(bd->bi_memstart); - memory_data[1] = cpu_to_be32(bd->bi_memsize); - - nodeoffset = fdt_find_node_by_path (blob, "/memory"); - if (nodeoffset >= 0) { - ret = fdt_setprop(blob, nodeoffset, "reg", memory_data, - sizeof(memory_data)); - if (ret < 0) - printf("ft_blob_update): cannot set /memory/reg " - "property err:%s\n", fdt_strerror(ret)); - } - else { - /* memory node is required in dts */ - printf("ft_blob_update(): cannot find /memory node " - "err:%s\n", fdt_strerror(nodeoffset)); - } } #endif /* defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_OF_LIBFDT) */
@@ -422,5 +404,6 @@ void ft_board_setup(void *blob, bd_t *bd) { ft_cpu_setup(blob, bd); ft_blob_update(blob, bd); + fdt_memory(blob); } #endif /* defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) */ diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index d816349..41547c6 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -992,6 +992,18 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag, do_reset (cmdtp, flag, argc, argv); } #endif +#ifdef CONFIG_OF_MEMORY_FIXUP + /* + * Add the "/memory" node if it does not exist, and do a fixup + * of the "reg" property with values detected by U-Boot + * (taken from bd->bi_memstart and bd->bi_memsize). + */ + if (fdt_memory(of_flat_tree) < 0) { + puts ("ERROR: /memory node create failed - " + "must RESET the board to recover.\n"); + do_reset (cmdtp, flag, argc, argv); + } +#endif #ifdef CONFIG_OF_BOARD_SETUP /* Call the board-specific fixup routine */ ft_board_setup(of_flat_tree, gd->bd); diff --git a/common/fdt_support.c b/common/fdt_support.c index 175d59e..f1f906c 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -348,4 +348,79 @@ int fdt_bd_t(void *fdt) } #endif /* ifdef CONFIG_OF_HAS_BD_T */
+/********************************************************************/ + +static int ft_get_property_int(void *blob, char *path, const char *prop) +{ + int offset, len; + void *ptr; + u32 dst; + + offset = fdt_find_node_by_path(blob, path); + ptr = fdt_getprop(blob, offset, prop, &len); + debug("%s(): got %d size %d\n", __FUNCTION__, ptr, len); + if ((ptr) && (len == sizeof(int))) { + memcpy(&dst, ptr, len); + debug("dest %d\n", dst); + return dst; + } + return -1; +} + +int fdt_memory(void *fdt) +{ + int nodeoffset; + int err; + u32 tmp[4]; + bd_t *bd = gd->bd; + + err = fdt_check_header(fdt); + if (err < 0) { + printf("fdt_memory: %s\n", fdt_strerror(err)); + return err; + } + + memset(tmp, 0, sizeof(tmp)); + /* Now, figure out toplevel #ac and #sc - we'll need them later */ + offset = fdt_find_node_by_path(fdt, "/"); + ac = ft_get_property_int(fdt, "/", "#address-cells"); + sc = ft_get_property_int(fdt, "/", "#size-cells"); + debug("#address-cells=%d size-cells=%d\n", ac, sc); + + /* update, or add and update /memory node */ + nodeoffset = fdt_find_node_by_path(fdt, "/memory"); + if (nodeoffset < 0) { + nodeoffset = fdt_add_subnode(fdt, 0, "memory"); + if (nodeoffset < 0) + printf("WARNING could not create /memory: %s.\n", + fdt_strerror(nodeoffset)); + return nodeoffset; + } + err = fdt_setprop(fdt, nodeoffset, "device_type", "memory", + sizeof("memory")); + if (err < 0) { + printf("WARNING: could not set %s %s.\n", + "device_type", fdt_strerror(err)); + return err; + } + + /* this is a little rashly, but as long as u-boot is keeping + * memstart and memsize as ulong, should be safe. + */ + if ((ac >= 1) || (sc >= 1)) + debug("ulong memory params while #ac=%d and #sc=%d\n", + ac, sc); + tmp[ac-1] = cpu_to_fdt32(bd->bi_memstart); + tmp[ac+sc-1] = cpu_to_fdt32(bd->bi_memsize); + + err = fdt_setprop(blob, offset, "reg", + tmp, (ac+sc)*sizeof(tmp[0]))) + if (err < 0) { + printf("WARNING: could not set %s %s.\n", + "reg", fdt_strerror(err)); + return err; + } + return 0; +} + #endif /* CONFIG_OF_LIBFDT */ diff --git a/cpu/mpc83xx/cpu.c b/cpu/mpc83xx/cpu.c index e634f0a..99b10ee 100644 --- a/cpu/mpc83xx/cpu.c +++ b/cpu/mpc83xx/cpu.c @@ -526,7 +526,6 @@ ft_cpu_setup(void *blob, bd_t *bd) int nodeoffset; int err; int j; - int tmp[2];
for (j = 0; j < (sizeof(fixup_props) / sizeof(fixup_props[0])); j++) { nodeoffset = fdt_find_node_by_path(blob, fixup_props[j].node); @@ -542,22 +541,6 @@ ft_cpu_setup(void *blob, bd_t *bd) fixup_props[j].node, fdt_strerror(nodeoffset)); } } - - /* update, or add and update /memory node */ - nodeoffset = fdt_find_node_by_path(blob, "/memory"); - if (nodeoffset < 0) { - nodeoffset = fdt_add_subnode(blob, 0, "memory"); - if (nodeoffset < 0) - debug("failed to add /memory node: %s\n", - fdt_strerror(nodeoffset)); - } - if (nodeoffset >= 0) { - fdt_setprop(blob, nodeoffset, "device_type", - "memory", sizeof("memory")); - tmp[0] = cpu_to_be32(bd->bi_memstart); - tmp[1] = cpu_to_be32(bd->bi_memsize); - fdt_setprop(blob, nodeoffset, "reg", tmp, sizeof(tmp)); - } } #elif defined(CONFIG_OF_FLAT_TREE) void @@ -567,6 +550,9 @@ ft_cpu_setup(void *blob, bd_t *bd) int len; ulong clock;
+ /* fixup/create /memory node */ + fdt_memory(blob); + clock = bd->bi_busfreq; p = ft_get_prop(blob, "/cpus/" OF_CPU "/bus-frequency", &len); if (p != NULL) diff --git a/include/fdt_support.h b/include/fdt_support.h index 60fa423..eca2186 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -29,6 +29,7 @@ #include <fdt.h>
int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force); +int fdt_memory(void *fdt);
#ifdef CONFIG_OF_HAS_UBOOT_ENV int fdt_env(void *fdt);

+/ ********************************************************************/
+static int ft_get_property_int(void *blob, char *path, const char *prop) +{
- int offset, len;
- void *ptr;
- u32 dst;
- offset = fdt_find_node_by_path(blob, path);
- ptr = fdt_getprop(blob, offset, prop, &len);
- debug("%s(): got %d size %d\n", __FUNCTION__, ptr, len);
- if ((ptr) && (len == sizeof(int))) {
memcpy(&dst, ptr, len);
debug("dest %d\n", dst);
return dst;
- }
- return -1;
+}
+int fdt_memory(void *fdt)
I want it to be passed start and size rather than grabbing them directly. I have future applications in which using the same function will be useful.
+{
- int nodeoffset;
- int err;
- u32 tmp[4];
- bd_t *bd = gd->bd;
- err = fdt_check_header(fdt);
- if (err < 0) {
printf("fdt_memory: %s\n", fdt_strerror(err));
return err;
- }
- memset(tmp, 0, sizeof(tmp));
- /* Now, figure out toplevel #ac and #sc - we'll need them later */
- offset = fdt_find_node_by_path(fdt, "/");
- ac = ft_get_property_int(fdt, "/", "#address-cells");
- sc = ft_get_property_int(fdt, "/", "#size-cells");
- debug("#address-cells=%d size-cells=%d\n", ac, sc);
- /* update, or add and update /memory node */
- nodeoffset = fdt_find_node_by_path(fdt, "/memory");
- if (nodeoffset < 0) {
nodeoffset = fdt_add_subnode(fdt, 0, "memory");
if (nodeoffset < 0)
printf("WARNING could not create /memory: %s.\n",
fdt_strerror(nodeoffset));
return nodeoffset;
- }
- err = fdt_setprop(fdt, nodeoffset, "device_type", "memory",
sizeof("memory"));
- if (err < 0) {
printf("WARNING: could not set %s %s.\n",
"device_type", fdt_strerror(err));
return err;
- }
- /* this is a little rashly, but as long as u-boot is keeping
* memstart and memsize as ulong, should be safe.
*/
Hmm, I think this is a bad assumption. (We are looking at 36-bit phys support on e500 & e600).
- if ((ac >= 1) || (sc >= 1))
debug("ulong memory params while #ac=%d and #sc=%d\n",
ac, sc);
- tmp[ac-1] = cpu_to_fdt32(bd->bi_memstart);
- tmp[ac+sc-1] = cpu_to_fdt32(bd->bi_memsize);
- err = fdt_setprop(blob, offset, "reg",
tmp, (ac+sc)*sizeof(tmp[0])))
- if (err < 0) {
printf("WARNING: could not set %s %s.\n",
"reg", fdt_strerror(err));
return err;
- }
- return 0;
+}
#endif /* CONFIG_OF_LIBFDT */

On Mon, 26 Nov 2007 16:23:10 -0600 Kumar Gala wrote:
- /* this is a little rashly, but as long as u-boot is
keeping
* memstart and memsize as ulong, should be safe.
*/
Hmm, I think this is a bad assumption. (We are looking at 36-bit phys support on e500 & e600).
I'll look at it once again, from u64 POW, as was discussed....

Hi Vitaly,
Vitaly Bordug wrote on Monday, November 26, 2007 11:12 PM:
On Mon, 26 Nov 2007 14:55:46 -0600 (CST) Kumar Gala wrote:
Guys,
Here's my cut at a more generic version of Martin's patch that respects #ac and #sc.
Let me know what you think and if you have any comments. I'll add the board bits in a complete patch if this looks ok.
funny, I did the same with slightly different approach. below goes my version...
I think we are different only in #ac/#sc accounting, so let the people consider the best approach :)
Or... am I wrong in something?
fdt: add common memory fixup function
From: Martin Krause martin.krause@tqs.de
Add the function fdt_memory() to fixup the /memory node of the fdt with the memory values detected by U-Boot (taken from bd->bi_memstart and bd->bi_memsize).
The new function is called for all boards which define CONFIG_OF_LIBFDT.
This patch removes already existing board specific memory fixup routines for boards wich have CONFIG_OF_LIBFDT defined and switches them to the new routine. Boards wich use the CONIFG_OF_FLAT_TREE method are not touched.
Signed-off-by: Martin Krause martin.krause@tqs.de Signed-off-by: Vitaly Bordug vitb@kernel.crashing.org
README | 9 +++++ board/cds/common/ft_board.c | 9 ----- board/cm5200/cm5200.c | 23 ++----------- common/cmd_bootm.c | 12 +++++++ common/fdt_support.c | 75 +++++++++++++++++++++++++++++++++++++++++++ cpu/mpc83xx/cpu.c | 20 ++--------- include/fdt_support.h | 1 + 7 files changed, 103 insertions(+), 46 deletions(-)
[...]
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index d816349..41547c6 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -992,6 +992,18 @@ do_bootm_linux (cmd_tbl_t *cmdtp, int flag, do_reset (cmdtp, flag, argc, argv); } #endif +#ifdef CONFIG_OF_MEMORY_FIXUP
/*
* Add the "/memory" node if it does not exist, and do a fixup
* of the "reg" property with values detected by U-Boot
* (taken from bd->bi_memstart and bd->bi_memsize).
*/
if (fdt_memory(of_flat_tree) < 0) {
puts ("ERROR: /memory node create failed - "
"must RESET the board to recover.\n");
do_reset (cmdtp, flag, argc, argv);
}
+#endif
Ah, you placed a CONFIG_OF_MEMORY_FIXUP around the fdt_memory() call. That's OK with me, but then please set this define for all boards, which did a memory fixup already. I can't remeber, which boards this were exactly, but I know that the TQM5200 was one of it :-).
Best Regards, Martin Krause

Kumar Gala wrote on Monday, November 26, 2007 9:56 PM:
diff --git a/common/fdt_support.c b/common/fdt_support.c index c67bb3d..e6de10f 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -441,6 +441,66 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat, do_fixup_by_compat(fdt, compat, prop, &val, 4, create); }
+int fdt_fixup_memory(void *blob, u64 start, u64 size) +{
- int i, err, nodeoffset, len = 0;
- u8 tmp[16];
- const u32 *addrcell, *sizecell;
- err = fdt_check_header(blob);
- if (err < 0) {
printf("%s: %s\n", __FUNCTION__, fdt_strerror(err));
return err;
- }
- /* update, or add and update /memory node */
- nodeoffset = fdt_path_offset(blob, "/memory");
- if (nodeoffset < 0) {
nodeoffset = fdt_add_subnode(blob, 0, "memory");
if (nodeoffset < 0)
printf("WARNING: could not create /memory: %s.\n",
fdt_strerror(nodeoffset));
return nodeoffset;
- }
- err = fdt_setprop(blob, nodeoffset, "device_type", "memory",
sizeof("memory"));
- if (err < 0) {
printf("WARNING: could not set %s %s.\n", "device_type",
fdt_strerror(err));
return err;
- }
- addrcell = fdt_getprop(blob, 0, "#address-cells", NULL);
- if ((addrcell) && (*addrcell == 2)) {
for (i = 0; i <= 7; i++)
tmp[i] = (start >> ((7 - i) * 8)) & 0xff;
len = 8;
- } else {
for (i = 0; i <= 3; i++)
tmp[i] = (start >> ((3 - i) * 8)) & 0xff;
len = 4;
- }
Could this perhaps lead to endianess issues under some special circumstances?
Best Regards, Martin Krause

On Tue, Nov 27, 2007 at 09:13:06AM +0100, Martin Krause wrote:
Kumar Gala wrote on Monday, November 26, 2007 9:56 PM:
- addrcell = fdt_getprop(blob, 0, "#address-cells", NULL);
- if ((addrcell) && (*addrcell == 2)) {
for (i = 0; i <= 7; i++)
tmp[i] = (start >> ((7 - i) * 8)) & 0xff;
len = 8;
- } else {
for (i = 0; i <= 3; i++)
tmp[i] = (start >> ((3 - i) * 8)) & 0xff;
len = 4;
- }
Could this perhaps lead to endianess issues under some special circumstances?
I don't think so -- the device tree is defined as always being big-endian, regardless of the host endianness (multi-cell values would get a bit weird otherwise).
-Scott
participants (4)
-
Kumar Gala
-
Martin Krause
-
Scott Wood
-
Vitaly Bordug