[U-Boot] [PATCH 1/7] common: Display >=4GiB memory bank size

[Bin Meng]

Hi Simon,

On Fri, Jul 31, 2015 at 3:53 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 30 July 2015 at 04:49, Bin Meng <bmeng.cn at gmail.com> wrote:
>> bd->bi_dram[] has both start address and size defined as 32-bit,
>> which is not the case on some platforms where >=4GiB memory bank
>> is used. Change them to support such memory banks.
>>
>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>> ---
>>
>>  common/board_f.c             | 2 +-
>>  include/asm-generic/u-boot.h | 4 ++--
>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/common/board_f.c b/common/board_f.c
>> index 21be26f..a8ed6c8 100644
>> --- a/common/board_f.c
>> +++ b/common/board_f.c
>> @@ -206,7 +206,7 @@ static int show_dram_config(void)
>>         debug("\nRAM Configuration:\n");
>>         for (i = size = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
>>                 size += gd->bd->bi_dram[i].size;
>> -               debug("Bank #%d: %08lx ", i, gd->bd->bi_dram[i].start);
>> +               debug("Bank #%d: %016llx ", i, gd->bd->bi_dram[i].start);
>
> This makes all boards use an unreadable 16-character hex string. Can I
> suggest we only use the larger value when the size demands it? A bit
> messy but few things have >4GB memory so far.

OK, we can do:

if (gd->bd->bi_dram[i].start >= 0x100000000ULL)
    debug("Bank #%d: %016llx ", i, (unsigned long
long)(gd->bd->bi_dram[i].start));
else
    debug("Bank #%d: %08lx ", i, (unsigned long)(gd->bd->bi_dram[i].start));

But this is debug output anyway, does that really matter?

>
>>  #ifdef DEBUG
>>                 print_size(gd->bd->bi_dram[i].size, "\n");
>>  #endif
>> diff --git a/include/asm-generic/u-boot.h b/include/asm-generic/u-boot.h
>> index c918049..4e8b481 100644
>> --- a/include/asm-generic/u-boot.h
>> +++ b/include/asm-generic/u-boot.h
>> @@ -130,8 +130,8 @@ typedef struct bd_info {
>>         ulong           bi_boot_params; /* where this board expects params */
>>  #ifdef CONFIG_NR_DRAM_BANKS
>>         struct {                        /* RAM configuration */
>> -               ulong start;
>> -               ulong size;
>> +               unsigned long long start;
>
> Probably should add a comment why we can't use phys_addr_t?

I think we can use phys_addr_t.

>
>> +               phys_size_t size;
>
> Should this be ULL too?

OK, we can change them to:

    phys_addr_t start;
    phys_size_t size;

It this OK?

>
>>         } bi_dram[CONFIG_NR_DRAM_BANKS];
>>  #endif /* CONFIG_NR_DRAM_BANKS */
>>  } bd_t;
>> --

Regards,
Bin