From mboxrd@z Thu Jan  1 00:00:00 1970
From: Heiko Schocher <hs@denx.de>
Date: Tue, 09 Sep 2008 08:18:41 +0200
Subject: [U-Boot] [PATCH][for 1.3.5] Fix handling of mem reserves for
 ramdisk
In-Reply-To: <6E892604-9B2A-4338-8DF7-C58688FC5A94@kernel.crashing.org>
References: <48C21BBE.3080601@denx.de>
	<6E892604-9B2A-4338-8DF7-C58688FC5A94@kernel.crashing.org>
Message-ID: <48C61541.7060902@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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