[U-Boot] [PATCH 1/2] ARM: bootm: allow skipping fdt memory node fixup

[Rob Herring <robherring2@gmail.com>]

On 02/19/2013 01:26 PM, Wolfgang Denk wrote:
> Dear Rob Herring,
> 
> In message <1359085160-28675-1-git-send-email-robherring2@gmail.com> you wrote:
>>
>> Currently, u-boot will always fixup the DT memory node on ARM. If the dtb
>> has correct memory information, then we don't want or need u-boot to touch
>> the memory node. Allow platforms to skip this by not filling in dram bank
>> information.
> 
> Well, if this is what you want, then please implement just this, and
> do not do much more.
> 
>>  void __dram_init_banksize(void)
>>  {
>> +#if CONFIG_NR_DRAM_BANKS
>>  	gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
>>  	gd->bd->bi_dram[0].size =  gd->ram_size;
>> +#endif
>>  }
> 
> This change will leave the memory information in struct bd_info
> uninitialized, so the "bdinfo" command will print just bogus data.
> 
> This is not a good idea.

This hunk can be dropped I think.

>> --- a/arch/arm/lib/bootm.c
>> +++ b/arch/arm/lib/bootm.c
>> @@ -79,6 +79,7 @@ void arch_lmb_reserve(struct lmb *lmb)
>>  #ifdef CONFIG_OF_LIBFDT
>>  static int fixup_memory_node(void *blob)
>>  {
>> +#if CONFIG_NR_DRAM_BANKS
>>  	bd_t	*bd = gd->bd;
>>  	int bank;
>>  	u64 start[CONFIG_NR_DRAM_BANKS];
>> @@ -90,6 +91,9 @@ static int fixup_memory_node(void *blob)
>>  	}
>>  
>>  	return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
>> +#else
>> +	return 0;
>> +#endif
> 
> I dislike this.  If you want to make a property of your device tree
> "immutable", then you should mark it there as such.  Instead of
> removing the code here, U-Boot should then check for such a property
> and leave the value unchanged.

There is no such way defined to flag that and I'm not going to invent
one. It's not that the memory node is immutable, but the default should
not be "needs fixups". No one puts purposely wrong data in their DT. So
therefore all data should be immutable? What happens when we need to
fixup immutable data?

What I would really prefer to do is like powerpc where platforms
override this if they need the fixup and the default behavior is no
fixup. But that would require knowing which platforms do and don't need
the fixup. It only going to get harder to change that.

> Actually you are reaching here a point where it seems necessary that
> U-Boot itself is able to read the DT to configure itself correctly,
> i. e. the memory size information it holds should then also be
> extracted from the DT.

I thought about doing this. That's really an orthogonal issue. The
problem is you cannot adjust the amount of memory u-boot uses to be
different than the amount of RAM in the /memory node. If you adjust the
size in u-boot, the adjusted size is passed to the kernel no matter what.

There's issues with LPAE systems having >4GB of RAM as all the size and
address values are 32-bit. This could be fixed, but I'm not sure there
is much value in u-boot knowing about memory above 4GB as it can't
really access all of it and it would be purely informational.

Rob

> 
> Best regards,
> 
> Wolfgang Denk
>