[U-Boot] [PATCH][for 1.3.5] Fix handling of mem reserves for ramdisk

[U-Boot] [PATCH][for 1.3.5] Fix handling of mem reserves for ramdisk

[Heiko Schocher]

Hello Kumar,

> When we call fdt_chosen in bootm we get a dummy mem reserve added for
> the ramdisk location before its relocated.  We need to delete that
> mem reserve before we call fdt_initrd() for the final fixup.
>
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> ---
>  lib_ppc/bootm.c |   16 +++++++++++++++-
>  1 files changed, 15 insertions(+), 1 deletions(-)
>
> diff --git a/lib_ppc/bootm.c b/lib_ppc/bootm.c
> index 348421f..c801021 100644
> --- a/lib_ppc/bootm.c
> +++ b/lib_ppc/bootm.c
> @@ -180,8 +180,22 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
>
>  #if defined(CONFIG_OF_LIBFDT)
>  	/* fixup the initrd now that we know where it should be */
> -	if ((of_flat_tree) && (initrd_start && initrd_end))
> +	if ((of_flat_tree) && (initrd_start && initrd_end)) {
> +                uint64_t addr, size;
> +                int  total = fdt_num_mem_rsv(of_flat_tree);
> +                int  j;
> +
> +                /* The call to fdt_chosen created a dummy mem rsv, delete it */
> +                for (j = 0; j < total; j++) {
> +                        fdt_get_mem_rsv(of_flat_tree, j, &addr, &size);
> +                        if (addr == images->rd_start) {
> +                                fdt_del_mem_rsv(of_flat_tree, j);
> +                                break;
> +                        }
> +                }
> +
>  		fdt_initrd(of_flat_tree, initrd_start, initrd_end, 1);
> +	}
>  #endif
>  	debug ("## Transferring control to Linux (at address %08lx) ...\n",
>  		(ulong)kernel);
> -- 1.5.5.1

Hmm... OK, now the code will do the same as before, but is this optimal?

Why must we do a

a) dummy mem reservation
b) search for it
c) delete it
d) make the right mem reservation?

Think the steps a) - c) are useless ...

Please have a look at the following patch, it deletes the steps a) - c)
and fixes the fdt chosen command ...

[PATCH] powerpc: Fix bootm to boot up again with a Ramdisk.

Patch
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=2a1a2cb6e2b87ee550e6f27b647d23331dfd5e1b#patch3

didnt remove the dummy mem reservation which was set up
in fdt_chosen, and this stopped Linux from booting with a
Ramdisk. This patch fixes this, by deleting the useless
dummy mem reservation.

Signed-off-by: Heiko Schocher <hs@denx.de>
---
 common/cmd_fdt.c      |    3 ++-
 common/fdt_support.c  |    4 +---
 include/fdt_support.h |    2 +-
 lib_ppc/bootm.c       |    5 +++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/common/cmd_fdt.c b/common/cmd_fdt.c
index 0593bad..288a5c4 100644
--- a/common/cmd_fdt.c
+++ b/common/cmd_fdt.c
@@ -450,7 +450,8 @@ int do_fdt (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 			initrd_end = simple_strtoul(argv[3], NULL, 16);
 		}

-		fdt_chosen(working_fdt, initrd_start, initrd_end, 1);
+		fdt_chosen(working_fdt, 1);
+		fdt_initrd(working_fdt, initrd_start, initrd_end, 1);
 	}
 	/* resize the fdt */
 	else if (strncmp(argv[1], "re", 2) == 0) {
diff --git a/common/fdt_support.c b/common/fdt_support.c
index a7773ab..8ceeb0f 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -165,7 +165,7 @@ int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end, int force)
 	return 0;
 }

-int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force)
+int fdt_chosen(void *fdt, int force)
 {
 	int   nodeoffset;
 	int   err;
@@ -215,8 +215,6 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force)
 		}
 	}

-	fdt_initrd(fdt, initrd_start, initrd_end, force);
-
 #ifdef CONFIG_OF_STDOUT_VIA_ALIAS
 	path = fdt_getprop(fdt, nodeoffset, "linux,stdout-path", NULL);
 	if ((path == NULL) || force)
diff --git a/include/fdt_support.h b/include/fdt_support.h
index 424c3c6..ceaadc2 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -28,7 +28,7 @@

 #include <fdt.h>

-int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force);
+int fdt_chosen(void *fdt, int force);
 int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end, int force);
 void do_fixup_by_path(void *fdt, const char *path, const char *prop,
 		      const void *val, int len, int create);
diff --git a/lib_ppc/bootm.c b/lib_ppc/bootm.c
index 348421f..d581493 100644
--- a/lib_ppc/bootm.c
+++ b/lib_ppc/bootm.c
@@ -145,8 +145,9 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
 	 * if the user wants it (the logic is in the subroutines).
 	 */
 	if (of_size) {
-		/* pass in dummy initrd info, we'll fix up later */
-		if (fdt_chosen(of_flat_tree, images->rd_start, images->rd_end, 0) < 0) {
+		/* we dont have to pass anymore the dummy initrd info!
+                   we'll add this later, immediately with the right values. */
+		if (fdt_chosen(of_flat_tree, 0) < 0) {
 			puts ("ERROR: ");
 			puts ("/chosen node create failed");
 			puts (" - must RESET the board to recover.\n");
-- 
1.5.4.1


bye
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH][for 1.3.5] Fix handling of mem reserves for ramdisk

[Kumar Gala]

> Hmm... OK, now the code will do the same as before, but is this  
> optimal?
>
> Why must we do a
>
> a) dummy mem reservation
> b) search for it
> c) delete it
> d) make the right mem reservation?
>
> Think the steps a) - c) are useless ...
>
> Please have a look at the following patch, it deletes the steps a) -  
> c)
> and fixes the fdt chosen command ...
>
> [PATCH] powerpc: Fix bootm to boot up again with a Ramdisk.
>
> Patch
> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=2a1a2cb6e2b87ee550e6f27b647d23331dfd5e1b#patch3
>
> didnt remove the dummy mem reservation which was set up
> in fdt_chosen, and this stopped Linux from booting with a
> Ramdisk. This patch fixes this, by deleting the useless
> dummy mem reservation.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> ---
> common/cmd_fdt.c      |    3 ++-
> common/fdt_support.c  |    4 +---
> include/fdt_support.h |    2 +-
> lib_ppc/bootm.c       |    5 +++--
> 4 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/common/cmd_fdt.c b/common/cmd_fdt.c
> index 0593bad..288a5c4 100644
> --- a/common/cmd_fdt.c
> +++ b/common/cmd_fdt.c
> @@ -450,7 +450,8 @@ int do_fdt (cmd_tbl_t * cmdtp, int flag, int  
> argc, char *argv[])
> 			initrd_end = simple_strtoul(argv[3], NULL, 16);
> 		}
>
> -		fdt_chosen(working_fdt, initrd_start, initrd_end, 1);
> +		fdt_chosen(working_fdt, 1);
> +		fdt_initrd(working_fdt, initrd_start, initrd_end, 1);

You are removing functionality, if you want to do this add a command  
'fdt initrd' that sets the initrd props and the mem reserve information.

>
> 	}
> 	/* resize the fdt */
> 	else if (strncmp(argv[1], "re", 2) == 0) {
> diff --git a/common/fdt_support.c b/common/fdt_support.c
> index a7773ab..8ceeb0f 100644
> --- a/common/fdt_support.c
> +++ b/common/fdt_support.c
> @@ -165,7 +165,7 @@ int fdt_initrd(void *fdt, ulong initrd_start,  
> ulong initrd_end, int force)
> 	return 0;
> }
>
> -int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int  
> force)
> +int fdt_chosen(void *fdt, int force)
> {
> 	int   nodeoffset;
> 	int   err;
> @@ -215,8 +215,6 @@ int fdt_chosen(void *fdt, ulong initrd_start,  
> ulong initrd_end, int force)
> 		}
> 	}
>
> -	fdt_initrd(fdt, initrd_start, initrd_end, force);
> -
> #ifdef CONFIG_OF_STDOUT_VIA_ALIAS
> 	path = fdt_getprop(fdt, nodeoffset, "linux,stdout-path", NULL);
> 	if ((path == NULL) || force)
> diff --git a/include/fdt_support.h b/include/fdt_support.h
> index 424c3c6..ceaadc2 100644
> --- a/include/fdt_support.h
> +++ b/include/fdt_support.h
> @@ -28,7 +28,7 @@
>
> #include <fdt.h>
>
> -int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int  
> force);
> +int fdt_chosen(void *fdt, int force);
> int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end, int  
> force);
> void do_fixup_by_path(void *fdt, const char *path, const char *prop,
> 		      const void *val, int len, int create);
> diff --git a/lib_ppc/bootm.c b/lib_ppc/bootm.c
> index 348421f..d581493 100644
> --- a/lib_ppc/bootm.c
> +++ b/lib_ppc/bootm.c
> @@ -145,8 +145,9 @@ int do_bootm_linux(int flag, int argc, char  
> *argv[], bootm_headers_t *images)
> 	 * if the user wants it (the logic is in the subroutines).
> 	 */
> 	if (of_size) {
> -		/* pass in dummy initrd info, we'll fix up later */
> -		if (fdt_chosen(of_flat_tree, images->rd_start, images->rd_end, 0)  
> < 0) {
> +		/* we dont have to pass anymore the dummy initrd info!
> +                   we'll add this later, immediately with the right  
> values. */
> +		if (fdt_chosen(of_flat_tree, 0) < 0) {
> 			puts ("ERROR: ");
> 			puts ("/chosen node create failed");
> 			puts (" - must RESET the board to recover.\n");


The reason we had the code before was to try and make sure the size of  
the fdt was as close to its final size as possible before we dealt  
with the ramdisk relocation (boot_ramdisk_high()) that included the  
properties and the memreserve in the fdt.  Step's a)-c) are there to  
make sure the size is correct.

- k

[U-Boot] [PATCH][for 1.3.5] Fix handling of mem reserves for ramdisk

[Wolfgang Denk]

Dear Kumar Gala,

In message <6E892604-9B2A-4338-8DF7-C58688FC5A94@kernel.crashing.org> you wrote:
>
> The reason we had the code before was to try and make sure the size of  
> the fdt was as close to its final size as possible before we dealt  
> with the ramdisk relocation (boot_ramdisk_high()) that included the  
> properties and the memreserve in the fdt.  Step's a)-c) are there to  
> make sure the size is correct.

Well, but the size needed for this should be a constant, so it should
be sufficioent to determine  it  once  and  then  use  a  hard  coded
constant; eventually add some padding.

Doing this in runtime is a waste of resources (flash memory, RAM, and
time - not to mention human time for maintaining the code).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Let the programmers be many and the managers few -- then all will  be
productive.               -- Geoffrey James, "The Tao of Programming"

[U-Boot] [PATCH][for 1.3.5] Fix handling of mem reserves for ramdisk

[Kumar Gala]

On Sep 8, 2008, at 9:10 AM, Wolfgang Denk wrote:

> Dear Kumar Gala,
>
> In message <6E892604-9B2A-4338-8DF7- 
> C58688FC5A94 at kernel.crashing.org> you wrote:
>>
>> The reason we had the code before was to try and make sure the size  
>> of
>> the fdt was as close to its final size as possible before we dealt
>> with the ramdisk relocation (boot_ramdisk_high()) that included the
>> properties and the memreserve in the fdt.  Step's a)-c) are there to
>> make sure the size is correct.
>
> Well, but the size needed for this should be a constant, so it should
> be sufficioent to determine  it  once  and  then  use  a  hard  coded
> constant; eventually add some padding.
>
> Doing this in runtime is a waste of resources (flash memory, RAM, and
> time - not to mention human time for maintaining the code).

I'm ok with determining the size bump and doing it that way.

- k

[U-Boot] [PATCH][for 1.3.5] Fix handling of mem reserves for ramdisk

[Heiko Schocher]

Hello Kumar,

Kumar Gala wrote:
> On Sep 8, 2008, at 9:10 AM, Wolfgang Denk wrote:
>> Dear Kumar Gala,
>>
>> In message <6E892604-9B2A-4338-8DF7-C58688FC5A94@kernel.crashing.org>
>> you wrote:
>>>
>>> The reason we had the code before was to try and make sure the size of
>>> the fdt was as close to its final size as possible before we dealt
>>> with the ramdisk relocation (boot_ramdisk_high()) that included the
>>> properties and the memreserve in the fdt.  Step's a)-c) are there to
>>> make sure the size is correct.
>>
>> Well, but the size needed for this should be a constant, so it should
>> be sufficioent to determine  it  once  and  then  use  a  hard  coded
>> constant; eventually add some padding.
>>
>> Doing this in runtime is a waste of resources (flash memory, RAM, and
>> time - not to mention human time for maintaining the code).
> 
> I'm ok with determining the size bump and doing it that way.

[PATCH] powerpc: Fix bootm to boot up again with a Ramdisk

Patch
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=2a1a2cb6e2b87ee550e6f27b647d23331dfd5e1b#patch3

didnt remove the dummy mem reservation in fdt_chosen, and
this stopped Linux from booting with a Ramdisk. This patch
fixes this, by deleting the useless dummy mem reservation.

When booting with a Ramdisk, a fix offset FDT_RAMDISK_OVERHEAD
is now added to of_size, so we dont need anymore a dummy
mem reservation.

Signed-off-by: Heiko Schocher <hs@denx.de>
---
 common/cmd_fdt.c      |    3 ++-
 common/fdt_support.c  |    4 +---
 include/fdt.h         |    2 ++
 include/fdt_support.h |    2 +-
 lib_ppc/bootm.c       |    8 ++++++--
 5 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/common/cmd_fdt.c b/common/cmd_fdt.c
index 0593bad..288a5c4 100644
--- a/common/cmd_fdt.c
+++ b/common/cmd_fdt.c
@@ -450,7 +450,8 @@ int do_fdt (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 			initrd_end = simple_strtoul(argv[3], NULL, 16);
 		}

-		fdt_chosen(working_fdt, initrd_start, initrd_end, 1);
+		fdt_chosen(working_fdt, 1);
+		fdt_initrd(working_fdt, initrd_start, initrd_end, 1);
 	}
 	/* resize the fdt */
 	else if (strncmp(argv[1], "re", 2) == 0) {
diff --git a/common/fdt_support.c b/common/fdt_support.c
index a7773ab..8ceeb0f 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -165,7 +165,7 @@ int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end, int force)
 	return 0;
 }

-int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force)
+int fdt_chosen(void *fdt, int force)
 {
 	int   nodeoffset;
 	int   err;
@@ -215,8 +215,6 @@ int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force)
 		}
 	}

-	fdt_initrd(fdt, initrd_start, initrd_end, force);
-
 #ifdef CONFIG_OF_STDOUT_VIA_ALIAS
 	path = fdt_getprop(fdt, nodeoffset, "linux,stdout-path", NULL);
 	if ((path == NULL) || force)
diff --git a/include/fdt.h b/include/fdt.h
index 48ccfd9..d09efe9 100644
--- a/include/fdt.h
+++ b/include/fdt.h
@@ -57,4 +57,6 @@ struct fdt_property {
 #define FDT_V16_SIZE	FDT_V3_SIZE
 #define FDT_V17_SIZE	(FDT_V16_SIZE + sizeof(uint32_t))

+/* adding a ramdisk needs 0x44 bytes in version 2008-10 */
+#define FDT_RAMDISK_OVERHEAD	0x80
 #endif /* _FDT_H */
diff --git a/include/fdt_support.h b/include/fdt_support.h
index 424c3c6..ceaadc2 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -28,7 +28,7 @@

 #include <fdt.h>

-int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int force);
+int fdt_chosen(void *fdt, int force);
 int fdt_initrd(void *fdt, ulong initrd_start, ulong initrd_end, int force);
 void do_fixup_by_path(void *fdt, const char *path, const char *prop,
 		      const void *val, int len, int create);
diff --git a/lib_ppc/bootm.c b/lib_ppc/bootm.c
index 348421f..d428fa1 100644
--- a/lib_ppc/bootm.c
+++ b/lib_ppc/bootm.c
@@ -145,8 +145,9 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
 	 * if the user wants it (the logic is in the subroutines).
 	 */
 	if (of_size) {
-		/* pass in dummy initrd info, we'll fix up later */
-		if (fdt_chosen(of_flat_tree, images->rd_start, images->rd_end, 0) < 0) {
+		/* we dont have to pass anymore the dummy initrd info!
+                   we'll add this later, immediately with the right values. */
+		if (fdt_chosen(of_flat_tree, 0) < 0) {
 			puts ("ERROR: ");
 			puts ("/chosen node create failed");
 			puts (" - must RESET the board to recover.\n");
@@ -169,6 +170,9 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
 			goto error;
 		of_size = ret;

+		/* adding a ramdisk needs 0x44 bytes in version 2008-10 */
+		if ((of_flat_tree) && (initrd_start && initrd_end))
+			of_size += FDT_RAMDISK_OVERHEAD;
 		/* Create a new LMB reservation */
 		lmb_reserve(lmb, (ulong)of_flat_tree, of_size);
 	}
-- 
1.5.4.1

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH][for 1.3.5] Fix handling of mem reserves for ramdisk

[Wolfgang Denk]

Dear Heiko Schocher,

In message <48C7830B.6010205@denx.de> you wrote:
> 
> [PATCH] powerpc: Fix bootm to boot up again with a Ramdisk

Please omit this rom the commit message - it just duplicates the
Subject line.

Also, it's probably not "[for 1.3.5]" (which will never see the light
of this virtuality) but "for 2008.10".

> Patch
> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=2a1a2cb6e2b87ee550e6f27b647d23331dfd5e1b#patch3
> 
> didnt remove the dummy mem reservation in fdt_chosen, and
> this stopped Linux from booting with a Ramdisk. This patch
> fixes this, by deleting the useless dummy mem reservation.
> 
> When booting with a Ramdisk, a fix offset FDT_RAMDISK_OVERHEAD
> is now added to of_size, so we dont need anymore a dummy
> mem reservation.
...

> +/* adding a ramdisk needs 0x44 bytes in version 2008-10 */

2008.10, not 2008-10.

> --- a/lib_ppc/bootm.c
> +++ b/lib_ppc/bootm.c
> @@ -145,8 +145,9 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
>  	 * if the user wants it (the logic is in the subroutines).
>  	 */
>  	if (of_size) {
> -		/* pass in dummy initrd info, we'll fix up later */
> -		if (fdt_chosen(of_flat_tree, images->rd_start, images->rd_end, 0) < 0) {
> +		/* we dont have to pass anymore the dummy initrd info!
> +                   we'll add this later, immediately with the right values. */

Incorrect multiline format.

But the information is already included with the  #define  above  and
the commit message. I suggest to delete the comment completely.

> @@ -169,6 +170,9 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
>  			goto error;
>  		of_size = ret;
> 
> +		/* adding a ramdisk needs 0x44 bytes in version 2008-10 */

Ditto - I suggest to delete the redudant information.

> +		if ((of_flat_tree) && (initrd_start && initrd_end))
> +			of_size += FDT_RAMDISK_OVERHEAD;
>  		/* Create a new LMB reservation */
>  		lmb_reserve(lmb, (ulong)of_flat_tree, of_size);

Except for this nitpicking:

Acked-by: Wolfgang Denk <wd@denx.de>


Kumar, what do you think?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The use of anthropomorphic terminology when  dealing  with  computing
systems is a symptom of professional immaturity.   -- Edsger Dijkstra

[U-Boot] [PATCH][for 1.3.5] Fix handling of mem reserves for ramdisk

[Heiko Schocher]

Hello Kumar,

Kumar Gala wrote:
[...]
>> Please have a look at the following patch, it deletes the steps a) - c)
>> and fixes the fdt chosen command ...
[...]
>> diff --git a/common/cmd_fdt.c b/common/cmd_fdt.c
>> index 0593bad..288a5c4 100644
>> --- a/common/cmd_fdt.c
>> +++ b/common/cmd_fdt.c
>> @@ -450,7 +450,8 @@ int do_fdt (cmd_tbl_t * cmdtp, int flag, int argc,
>> char *argv[])
>>             initrd_end = simple_strtoul(argv[3], NULL, 16);
>>         }
>>
>> -        fdt_chosen(working_fdt, initrd_start, initrd_end, 1);
>> +        fdt_chosen(working_fdt, 1);
>> +        fdt_initrd(working_fdt, initrd_start, initrd_end, 1);
> 
> You are removing functionality, if you want to do this add a command
> 'fdt initrd' that sets the initrd props and the mem reserve information.

Hmm... really? Your patch

http://git.denx.de/?p=u-boot.git;a=commitdiff;h=2a1a2cb6e2b87ee550e6f27b647d23331dfd5e1b#patch3

moved functionality from fdt_chosen in the fdt_initrd function and
called fdt_initrd() in fdt_chosen ().

My patch removes now this fdt_initrd() call from fdt_chosen(), and
so I call it here to fix the fdt_chosen command ... maybe I miss
something?

>>     }
>>     /* resize the fdt */
>>     else if (strncmp(argv[1], "re", 2) == 0) {
>> diff --git a/common/fdt_support.c b/common/fdt_support.c
>> index a7773ab..8ceeb0f 100644
>> --- a/common/fdt_support.c
>> +++ b/common/fdt_support.c
>> @@ -165,7 +165,7 @@ int fdt_initrd(void *fdt, ulong initrd_start,
>> ulong initrd_end, int force)
>>     return 0;
>> }
>>
>> -int fdt_chosen(void *fdt, ulong initrd_start, ulong initrd_end, int
>> force)
>> +int fdt_chosen(void *fdt, int force)
>> {
>>     int   nodeoffset;
>>     int   err;
>> @@ -215,8 +215,6 @@ int fdt_chosen(void *fdt, ulong initrd_start,
>> ulong initrd_end, int force)
>>         }
>>     }
>>
>> -    fdt_initrd(fdt, initrd_start, initrd_end, force);
>> -
>> #ifdef CONFIG_OF_STDOUT_VIA_ALIAS
[...]
> The reason we had the code before was to try and make sure the size of
> the fdt was as close to its final size as possible before we dealt with
> the ramdisk relocation (boot_ramdisk_high()) that included the
> properties and the memreserve in the fdt.  Step's a)-c) are there to
> make sure the size is correct.

Hmmm... and thats the reason why we risk to forgot a memreservation?
(Keep in mind, I think actually it is not possible to boot Linux
with actual u-boot from a Ramdisk in Flash on a powerpc!)

Is this really so essential to know the final fdt size?

While writing this, and if the final fdt size is so important,
I wanted to propose a fix Ramdisk address we can later find and
delete, but then I saw your patch

http://lists.denx.de/pipermail/u-boot/2008-September/040054.html

does this principally! So this patch is okay for me.

bye
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH][for 1.3.5] Fix handling of mem reserves for ramdisk

[Kumar Gala]

When we call fdt_chosen in bootm we get a dummy mem reserve added for
the ramdisk location before its relocated.  We need to delete that
mem reserve before we call fdt_initrd() for the final fixup.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 lib_ppc/bootm.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/lib_ppc/bootm.c b/lib_ppc/bootm.c
index 348421f..c801021 100644
--- a/lib_ppc/bootm.c
+++ b/lib_ppc/bootm.c
@@ -180,8 +180,22 @@ int do_bootm_linux(int flag, int argc, char *argv[], bootm_headers_t *images)
 
 #if defined(CONFIG_OF_LIBFDT)
 	/* fixup the initrd now that we know where it should be */
-	if ((of_flat_tree) && (initrd_start && initrd_end))
+	if ((of_flat_tree) && (initrd_start && initrd_end)) {
+                uint64_t addr, size;
+                int  total = fdt_num_mem_rsv(of_flat_tree);
+                int  j;
+
+                /* The call to fdt_chosen created a dummy mem rsv, delete it */
+                for (j = 0; j < total; j++) {
+                        fdt_get_mem_rsv(of_flat_tree, j, &addr, &size);
+                        if (addr == images->rd_start) {
+                                fdt_del_mem_rsv(of_flat_tree, j);
+                                break;
+                        }
+                }
+
 		fdt_initrd(of_flat_tree, initrd_start, initrd_end, 1);
+	}
 #endif
 	debug ("## Transferring control to Linux (at address %08lx) ...\n",
 		(ulong)kernel);
-- 
1.5.5.1

[U-Boot] [PATCH][for 1.3.5] Fix handling of mem reserves for ramdisk

[Wolfgang Denk]

Dear Kumar Gala,

In message <1220631234-1051-1-git-send-email-galak@kernel.crashing.org> you wrote:
> When we call fdt_chosen in bootm we get a dummy mem reserve added for
> the ramdisk location before its relocated.  We need to delete that
> mem reserve before we call fdt_initrd() for the final fixup.

There will be no such thing as a release 1.3.5; it will be callsed
2008-10 or so.


And I would like to see a reply to  Heiko's  (IMHO  perfectly  valid)
question:  why  do  we  add  a  memory reservation just so we have to
bother later to remove it again?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Substitute "damn" every time you're inclined to write "very"; your
editor will delete it and the writing will be just as it should be.
                - Mark Twain