Re: [PATCH 1/3] hw/mips/boston: Massage memory map information

[Philippe Mathieu-Daudé]

On 7/29/21 9:55 AM, Philippe Mathieu-Daudé wrote:
> On 7/29/21 5:39 AM, Jiaxun Yang wrote:
>> Use memmap array to unfiy address of memory map.

Typo 'unify'.

>> That would allow us reuse address information for FDT generation.
>>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>  hw/mips/boston.c | 95 ++++++++++++++++++++++++++++++++++++------------
>>  1 file changed, 71 insertions(+), 24 deletions(-)
> 
>> +static const MemMapEntry boston_memmap[] = {
>> +    [BOSTON_LOWDDR] =     {        0x0,    0x10000000 },
>> +    [BOSTON_PCIE0] =      { 0x10000000,     0x2000000 },
>> +    [BOSTON_PCIE1] =      { 0x12000000,     0x2000000 },
>> +    [BOSTON_PCIE2] =      { 0x14000000,     0x2000000 },
>> +    [BOSTON_PCIE2_MMIO] = { 0x16000000,      0x100000 },
>> +    [BOSTON_CM] =         { 0x16100000,       0x20000 },
>> +    [BOSTON_GIC] =        { 0x16120000,       0x20000 },
>> +    [BOSTON_CDMM] =       { 0x16140000,        0x8000 },
>> +    [BOSTON_CPC] =        { 0x16200000,        0x8000 },
>> +    [BOSTON_PLATREG] =    { 0x17ffd000,        0x1000 },
>> +    [BOSTON_UART] =       { 0x17ffe000,        0x1000 },
> 
> 0x20?
> 
>> +    [BOSTON_LCD] =        { 0x17fff000,           0x8 },
>> +    [BOSTON_FLASH] =      { 0x18000000,     0x8000000 },
>> +    [BOSTON_PCIE1_MMIO] = { 0x20000000,    0x20000000 },
>> +    [BOSTON_PCIE0_MMIO] = { 0x40000000,    0x40000000 },
>> +    [BOSTON_HIGHDDR] =    { 0x80000000,           0x0 },
>> +};
> 
>>      /*
>>       * Setup argument registers to follow the UHI boot protocol:
>> @@ -333,8 +369,9 @@ static const void *boston_fdt_filter(void *opaque, const 
>> void *fdt_orig,
>>      ram_low_sz = MIN(256 * MiB, machine->ram_size);
>>      ram_high_sz = machine->ram_size - ram_low_sz;
>>      qemu_fdt_setprop_sized_cells(fdt, "/memory@0", "reg",
>> -                                 1, 0x00000000, 1, ram_low_sz,
>> -                                 1, 0x90000000, 1, ram_high_sz);
>> +                                 1, boston_memmap[BOSTON_LOWDDR].base, 1, 
>> ram_low_sz,
>> +                                 1, boston_memmap[BOSTON_HIGHDDR].base + 
>> ram_low_sz
>> +                                 , 1, ram_high_sz);
> 
> Misplaced comma.
> 
> BTW shouldn't we check whether ram_high_sz > 0 before adding this cell?
> 
> Otherwise:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>