On Feb 18, 2008, at 11:51 AM, Jerry Van Baren wrote:

> Kumar Gala wrote:
>> Doing the fdt before the ramdisk allows us to grow the fdt w/o  
>> concern
>> however it does mean we have to go in and fixup the initrd info since
>> we don't know where it will be.
>> Signed-off-by: Kumar Gala <[EMAIL PROTECTED]>
>
> Hi Kumar,
>
>> ---
>> Fixed up the error handling.  The old code tried returning an error  
>> and
>> should have called do_reset().
>> lib_ppc/bootm.c |   40 +++++++++++++++++++++++++++++++++++-----
>> 1 files changed, 35 insertions(+), 5 deletions(-)
>> diff --git a/lib_ppc/bootm.c b/lib_ppc/bootm.c
>> index 5158ccc..09e349e 100644
>> --- a/lib_ppc/bootm.c
>> +++ b/lib_ppc/bootm.c
>> @@ -71,7 +71,7 @@ do_bootm_linux(cmd_tbl_t *cmdtp, int flag,
>>      bd_t    *kbd;
>>      void    (*kernel)(bd_t *, ulong, ulong, ulong, ulong);
>> -    int     has_of = 0;
>> +    int     ret, has_of = 0;
>
> I would strongly prefer to see "ret" declared on a separate line.   
> I'm not a fan of comma declared variables and, when one is  
> initialized and the other isn't, it makes my brain play tricks on me  
> (thinking ret is initialized to 0 too, which it isn't).
>
>> #if defined(CONFIG_OF_LIBFDT)
>>      char    *of_flat_tree;
>> @@ -121,9 +121,6 @@ do_bootm_linux(cmd_tbl_t *cmdtp, int flag,
>>      rd_len = rd_data_end - rd_data_start;
>> -    alloc_current = ramdisk_high (alloc_current, rd_data_start, rd_len,
>> -                    sp_limit, get_sp (), &initrd_start, &initrd_end);
>> -
>> #if defined(CONFIG_OF_LIBFDT)
>>      /* find flattened device tree */
>>      alloc_current = get_fdt (alloc_current,
>> @@ -134,7 +131,8 @@ do_bootm_linux(cmd_tbl_t *cmdtp, int flag,
>>       * if the user wants it (the logic is in the subroutines).
>>       */
>>      if (of_flat_tree) {
>> -            if (fdt_chosen(of_flat_tree, initrd_start, initrd_end, 0) < 0) {
>> +            /* pass in dummy initrd info, we'll fix up later */
>> +            if (fdt_chosen(of_flat_tree, rd_data_start, rd_data_end, 0) < 
>> 0) {
>
> If you pass in 0,0 for rd_data_start, rd_data_end it will not create  
> the ramdisk entry...

If rd_data_start, rd_data_end are 0,0 than we don't have a ramdisk, so  
its ok.

>>                      fdt_error ("/chosen node create failed");
>>                      do_reset (cmdtp, flag, argc, argv);
>>              }
>> @@ -157,6 +155,38 @@ do_bootm_linux(cmd_tbl_t *cmdtp, int flag,
>>      }
>> #endif       /* CONFIG_OF_LIBFDT */
>> +    alloc_current = ramdisk_high (alloc_current, rd_data_start, rd_len,
>> +                    sp_limit, get_sp (), &initrd_start, &initrd_end);
>> +
>> +#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 == rd_data_start) {
>> +                            fdt_del_mem_rsv(of_flat_tree, j);
>> +                            break;
>> +                    }
>> +            }
>
> ...and the above loop and delete will then be unnecessary.

Again, protected by initrd_start/_end being non-zero.

>> +            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));
>> +                    do_reset (cmdtp, flag, argc, argv);
>> +            }
>> +
>> +            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);
>> +    }
>> +#endif
>>      debug ("## Transferring control to Linux (at address %08lx) ...\n",
>>              (ulong)kernel);
>
> ...and you will have to pass in '1' for the create (last) parameter  
> in the do_fixup_by_path_u32() calls to create them if they don't  
> exist.

They should exist by the time we are doing the fixup.

> Is that better, or just a different way to do the same thing?  It  
> seems more straight-forward to me.

Its just re-ordering the way we do things.

- k

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users

Reply via email to