[U-Boot] [POWERPC][BUG] ramdisk: u-boot no longer boots a ramdisk with Linux 2.6.27-rc4

[Heiko Schocher <hs@denx.de>]

Hello Kumar,

Kumar Gala wrote:
> On Sep 5, 2008, at 3:27 AM, Heiko Schocher wrote:
>> I actually trying to boot a Linux 2.6.27-rc4 Kernel with a Ramdisk
>> and actual u-boot on a MPC82xx based board. And what should I say,
>> it doesnt work anymore :-(
>>
>> Reason is the following commit:
>>
>> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=2a1a2cb6e2b87ee550e6f27b647d23331dfd5e1b#patch3
>>
>>
>> After this commit, u-boot no longer deletes the mem reservation
>> for the initrd in Flash, what stops Linux from booting.
>>
>> So I did the following, but maybe there is a better way for it?
> 
> I don't follow.. can you provide a bit more info about what's going on
> w/your mem reserves and what locations the ramdisk is at.

OK, I try it ...

We start in lib_ppc/bootm.c (actual u-boot):

 147         if (of_size) {
 148                 /* pass in dummy initrd info, we'll fix up later */
 149                 if (fdt_chosen(of_flat_tree, images->rd_start, images->rd_end, 0) < 0) {

This calls fdt_chosen with images->rd_start which are on my Hardware
in Flash at 0xff21000

fdt_chosen calls

fdt_initrd(of_flat_tree, initrd_start, initrd_end, force);

with initrd = images->rd_start = 0xff210000

fdt initr ():
 125         /*
 126          * Look for an existing entry and update it.  If we don't find
 127          * the entry, we will j be the next available slot.
 128          */
 129         for (j = 0; j < total; j++) {
 130                 err = fdt_get_mem_rsv(fdt, j, &addr, &size);
 131                 if (addr == initrd_start) {
 132                         fdt_del_mem_rsv(fdt, j);
 133                         break;
 134                 }
 135         }

didnt find this addres -> didnt call fdt_del_mem_rsv() This is OK here,
because its the first call ...

fdt_initrd calls fdt_add_mem_rsv with this value
-> One dummy mem reserv for the 0xff210000 value

Ok lets go on, later in lib_ppc/bootm.c:

Here is called fdt_initrd again, now with the values from the initrd
somewhere in RAM (which are the right ones) ...

 181 #if defined(CONFIG_OF_LIBFDT)
 182         /* fixup the initrd now that we know where it should be */
 183         if ((of_flat_tree) && (initrd_start && initrd_end))
 184                 fdt_initrd(of_flat_tree, initrd_start, initrd_end, 1);
 185 #endif

in fdt initr ():

 125         /*
 126          * Look for an existing entry and update it.  If we don't find
 127          * the entry, we will j be the next available slot.
 128          */
 129         for (j = 0; j < total; j++) {
 130                 err = fdt_get_mem_rsv(fdt, j, &addr, &size);
 131                 if (addr == initrd_start) {
 132                         fdt_del_mem_rsv(fdt, j);
 133                         break;
 134                 }
 135         }

didnt find this addresses in RAM -> didnt call fdt_del_mem_rsv()
for the dummy reservation (on my Hardware 0xff210000)...

This is NOT OK here!

> The code should have been identical to before.

Hmm... seems not so to me ...
... taking a look at your Patch:

Here your Patch for lib_ppc_bootm.c:

 #if defined(CONFIG_OF_LIBFDT)
        /* fixup the initrd now that we know where it should be */
-       if ((of_flat_tree) && (initrd_start && initrd_end)) {
-               uint64_t addr, size;
-               int  total = fdt_num_mem_rsv(of_flat_tree);
-               int  j;
-
-               /* Look for the dummy entry and 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);

This looks for me that the old code explicitly searches for the
dummy entry with images->rd_start!!

-                               break;
-                       }
-               }
-
-               ret = fdt_add_mem_rsv(of_flat_tree, initrd_start,
-                                       initrd_end - initrd_start + 1);
-               if (ret < 0) {
-                       printf("fdt_chosen: %s\n", fdt_strerror(ret));
-                       goto error;
-               }
-
-               do_fixup_by_path_u32(of_flat_tree, "/chosen",
-                                       "linux,initrd-start", initrd_start, 0);
-               do_fixup_by_path_u32(of_flat_tree, "/chosen",
-                                       "linux,initrd-end", initrd_end, 0);
-       }
+       if ((of_flat_tree) && (initrd_start && initrd_end))
+               fdt_initrd(of_flat_tree, initrd_start, initrd_end, 1);
                                         ^^^^^^^^^^^^
                                         At this place (see explanation before)
                                         initrd_start has the value for the
                                         initrd in RAM! -> the search for this
                                         new address in fdt_initrd () is useless,
                                         even dangerous, if the initrd is not moved,
                                         to RAM this search is succesful and for this
                                         case the right mem reservation gets deleted ...
 #endif


Nevertheless, the actual Code works with my patch fine, so
again the question, why we need this dummy call in fdt_chosen for
the initrd, when we call it later with the right values.

I think we can drop this useless dummy call from fdt_chosen.
Has somebody any suggestions?

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