On Feb 15, 2008, at 8:09 AM, Jerry Van Baren wrote:

> Kumar Gala wrote:
>> fdt header                          - Display header info
>> fdt bootcpu <id>                    - Set boot cpuid
>> fdt memory <addr> <size>            - Add/Update memory node
>> fdt memrsv print                    - Show current mem reserves
>> fdt memrsv add    <addr> <size>     - Add a mem reserve
>> fdt memrsv delete <index>           - Delete a mem reserves
>> Signed-off-by: Kumar Gala <[EMAIL PROTECTED]>
>
> Cool, procrastination pays off again!  :-)
>
>> ---
>> common/cmd_fdt.c |  111 ++++++++++++++++++++++++++++++++++++++++++++ 
>> ++++++++++
>> 1 files changed, 111 insertions(+), 0 deletions(-)
>> diff --git a/common/cmd_fdt.c b/common/cmd_fdt.c
>> index 9cd22ee..d0f8c27 100644
>> --- a/common/cmd_fdt.c
>> +++ b/common/cmd_fdt.c
>> @@ -296,6 +296,111 @@ int do_fdt (cmd_tbl_t * cmdtp, int flag, int  
>> argc, char *argv[])
>>                              return err;
>>                      }
>>              }
>> +
>> +    / 
>> ********************************************************************
>> +     * Display header info
>> +      
>> ********************************************************************/
>> +    } else if (argv[1][0] == 'h') {
>> +            u32 version = fdt_version(fdt);
>> +            printf("magic:\t\t\t0x%x\n", fdt_magic(fdt));
>> +            printf("totalsize:\t\t0x%x (%d)\n", fdt_totalsize(fdt),  
>> fdt_totalsize(fdt));
>> +            printf("off_dt_struct:\t\t0x%x\n", fdt_off_dt_struct(fdt));
>> +            printf("off_dt_strings:\t\t0x%x\n", fdt_off_dt_strings(fdt));
>> +            printf("off_mem_rsvmap:\t\t0x%x\n", fdt_off_mem_rsvmap(fdt));
>> +            printf("version:\t\t%d\n", version);
>> +            printf("last_comp_version:\t%d\n", fdt_last_comp_version(fdt));
>> +            if (version >= 2)
>> +                    printf("boot_cpuid_phys:\t0x%x\n",
>> +                            fdt_boot_cpuid_phys(fdt));
>> +            if (version >= 3)
>> +                    printf("size_dt_strings:\t0x%x\n",
>> +                            fdt_size_dt_strings(fdt));
>> +            if (version >= 17)
>> +                    printf("size_dt_struct:\t\t0x%x\n",
>> +                            fdt_size_dt_struct(fdt));
>> +            printf("\n");
>
> Very nice.
> * I think we should add the size of the reserved map.

done

> * It would be very useful to know how much space is available in the  
> reserved map

not sure how we do this.

> * It would be very useful to know how much space is available in the  
> dt_struct and dt_space space (I believe the "spare" in the blob can  
> be used for both the dt_struct and the dt_strings, but I'm not sure,  
> don't have time to look it up right now).

How about I leave that as an exercise for the reader :)

>> +    / 
>> ********************************************************************
>> +     * Set boot cpu id
>> +      
>> ********************************************************************/
>> +    } else if ((argv[1][0] == 'b') && (argv[1][1] == 'o')) {
>> +            unsigned long tmp = simple_strtoul(argv[2], NULL, 16);
>> +            fdt_set_boot_cpuid_phys(fdt, tmp);
>> +
>> +    / 
>> ********************************************************************
>> +     * memory command
>> +      
>> ********************************************************************/
>> +    } else if ((argv[1][0] == 'm') && (argv[1][1] == 'e') &&
>> +               (argv[1][3] == 'o')) {
>
> Did we miss (argv[1][2] == 'm'), is the term checking (argv[1][3] ==  
> 'o') wrong, or are we trying to accommodate dyslecix people?  I can  
> picture our fearless leader typing "fdt meromy", assuming he types  
> that many characters. :-D

I was trying to be slightly smart, I can add the check for argv[1][2]  
== 'm'

>
>
>> +            uint64_t addr, size;
>> +            int err;
>> +#ifdef CFG_64BIT_STRTOUL
>> +                    addr = simple_strtoull(argv[2], NULL, 16);
>> +                    size = simple_strtoull(argv[3], NULL, 16);
>> +#else
>> +                    addr = simple_strtoul(argv[2], NULL, 16);
>> +                    size = simple_strtoul(argv[3], NULL, 16);
>> +#endif
>> +            err = fdt_fixup_memory(fdt, addr, size);
>> +            if (err < 0)
>> +                    return err;
>> +
>> +    / 
>> ********************************************************************
>> +     * mem reserve commands
>> +      
>> ********************************************************************/
>> +    } else if ((argv[1][0] == 'm') && (argv[1][1] == 'e') &&
>> +               (argv[1][3] == 'r')) {
>
> Hmm, missing (argv[1][2] == 'm') again.  Once is an accident, twice  
> is by design.  I take it you are skipping the argv[1][2] check since  
> it isn't a significant character - code savings.  I can accept that,  
> although it really hurts my humor to have a logical explanation (pun  
> intended ;-).

ditto.
>
>
>> +            if (argv[2][0] == 'p') {
>> +                    uint64_t addr, size;
>> +                    int total = fdt_num_mem_rsv(fdt);
>> +                    int j, err;
>> +                    printf("index\t\t   start\t\t    size\n");
>> +                    printf("-------------------------------"
>> +                            "-----------------\n");
>> +                    for (j = 0; j < total; j++) {
>> +                            err = fdt_get_mem_rsv(fdt, j, &addr, &size);
>> +                            if (err < 0) {
>> +                                    printf("libfdt fdt_get_mem_rsv():  
>> %s\n",
>> +                                                    fdt_strerror(err));
>> +                                    return err;
>> +                            }
>> +                            printf("    %x\t%08x%08x\t%08x%08x\n", j,
>> +                                    (u32)(addr >> 32),
>> +                                    (u32)(addr & 0xffffffff),
>> +                                    (u32)(size >> 32),
>> +                                    (u32)(size & 0xffffffff));
>
> Trivia: If the compiler supports CFG_64BIT_STRTOUL, it /probably/  
> supports the "%llx" format.  I ran into trouble with this originally  
> (the compiler for my mpc8360 had problems passing a 64 bit varargs  
> arg, IIRC).  The above works in both 32bit and 64bit support cases  
> and I personally prefer the above slightly extra work rather than  
> Yet Another #ifdef (YA#).

yeah, wanted less #ifdef's.

>
>
>> +                    }
>> +            } else if (argv[2][0] == 'a') {
>> +                    uint64_t addr, size;
>> +                    int err;
>> +#ifdef CFG_64BIT_STRTOUL
>
> ...but some are unavoidable.
>
>> +                    addr = simple_strtoull(argv[3], NULL, 16);
>> +                    size = simple_strtoull(argv[4], NULL, 16);
>> +#else
>> +                    addr = simple_strtoul(argv[3], NULL, 16);
>> +                    size = simple_strtoul(argv[4], NULL, 16);
>> +#endif
>
> [snip]
>
>> #ifdef CONFIG_OF_BOARD_SETUP
>>      /* Call the board-specific fixup routine */
>> @@ -689,6 +794,12 @@ U_BOOT_CMD(
>>      "fdt set    <path> <prop> [<val>]    - Set <property> [to <val>]\n"
>>      "fdt mknode <path> <node>            - Create a new node after  
>> <path>\n"
>>      "fdt rm     <path> [<prop>]          - Delete the node or  
>> <property>\n"
>> +    "fdt header                          - Display header info\n"
>> +    "fdt bootcpu <id>                    - Set boot cpuid\n"
>> +    "fdt memory <addr> <size>            - Add/Update memory node\n"
>> +    "fdt memrsv print                    - Show current mem reserves\n"
>> +    "fdt memrsv add    <addr> <size>     - Add a mem reserve\n"
>                        ^^^
> > +   "fdt memrsv add <addr> <size>        - Add a mem reserve\n"
> It is a weakly held personal preference, but I would kill the extra  
> spaces between add and <addr>, I don't see that it helps readability.

will fix.

>> +    "fdt memrsv delete <index>           - Delete a mem reserves\n"
>>      "fdt chosen - Add/update the /chosen branch in the tree\n"
>> #ifdef CONFIG_OF_HAS_UBOOT_ENV
>>      "fdt env    - Add/replace the /u-boot-env branch in the tree\n"
>
> THANKS!
> gvb


-------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/u-boot-users

Reply via email to