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

[Rob Herring <robherring2@gmail.com>]

On 02/19/2013 05:14 PM, Wolfgang Denk wrote:
> Dear Rob Herring,
> 
> In message <5123E311.1070607@gmail.com> you wrote:
>>

[snip]

>> 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.
> 
> Normally we use get_ramsize() to check / verify the amount of
> available RAM.  And on PPC, we nearly always set the ram size in the
> DT.  I don't know why you think the default was no fixup; this is not
> true.  It's the other way round.

No it is not.

The fixup is in bootm.c for all of arm, yet appears in all the board/soc
files for powerpc. It may be nearly always fixed up because every platform
duplicates the fixup code, but it is not the default. It is a per platform
decision on powerpc and it is not on arm.

$ git grep fixup_memory
arch/arm/lib/bootm.c:static int fixup_memory_node(void *blob)
arch/arm/lib/bootm.c:   return fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
arch/arm/lib/bootm.c:   fixup_memory_node(*of_flat_tree);
arch/powerpc/cpu/74xx_7xx/cpu.c:        fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
arch/powerpc/cpu/mpc512x/cpu.c: fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
arch/powerpc/cpu/mpc5xxx/cpu.c: fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
arch/powerpc/cpu/mpc8260/cpu.c: fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
arch/powerpc/cpu/mpc83xx/fdt.c: fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
arch/powerpc/cpu/mpc85xx/fdt.c: fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
arch/powerpc/cpu/mpc86xx/fdt.c: fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
arch/powerpc/cpu/mpc8xx/fdt.c:  fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
arch/powerpc/cpu/ppc4xx/fdt.c:  fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
board/exmeritus/hww1u1a/hww1u1a.c:      fdt_fixup_memory(blob, (u64)base, (u64)size);
board/freescale/bsc9131rdb/bsc9131rdb.c:        fdt_fixup_memory(blob, (u64)base, (u64)size);
board/freescale/corenet_ds/corenet_ds.c:        fdt_fixup_memory(blob, (u64)base, (u64)size);
board/freescale/mpc7448hpc2/mpc7448hpc2.c:      fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
board/freescale/mpc8572ds/mpc8572ds.c:  fdt_fixup_memory(blob, (u64)base, (u64)size);
board/freescale/p1010rdb/p1010rdb.c:    fdt_fixup_memory(blob, (u64)base, (u64)size);
board/freescale/p1022ds/p1022ds.c:      fdt_fixup_memory(blob, (u64)base, (u64)size);
board/freescale/p1023rds/p1023rds.c:    fdt_fixup_memory(blob, (u64)base, (u64)size);
board/freescale/p1_p2_rdb/p1_p2_rdb.c:  fdt_fixup_memory(blob, (u64)base, (u64)size);
board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c:    fdt_fixup_memory(blob, (u64)base, (u64)size);
board/freescale/p2020come/p2020come.c:  fdt_fixup_memory(blob, (u64)base, (u64)size);
board/freescale/p2020ds/p2020ds.c:      fdt_fixup_memory(blob, (u64)base, (u64)size);
board/freescale/p2041rdb/p2041rdb.c:    fdt_fixup_memory(blob, (u64)base, (u64)size);
board/freescale/t4qds/t4qds.c:  fdt_fixup_memory(blob, (u64)base, (u64)size);
board/ipek01/ipek01.c:  fdt_fixup_memory (blob, (u64) bd->bi_memstart, (u64) bd->bi_memsize);
board/pdm360ng/pdm360ng.c:      fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
common/cmd_fdt.c:               err = fdt_fixup_memory(working_fdt, addr, size);
common/fdt_support.c:int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks)
common/fdt_support.c:int fdt_fixup_memory(void *blob, u64 start, u64 size)
common/fdt_support.c:   return fdt_fixup_memory_banks(blob, &start, &size, 1);
include/fdt_support.h:int fdt_fixup_memory(void *blob, u64 start, u64 size);
include/fdt_support.h:int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks);

>>> 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.
> 
> Well, let's face it - no matter what you do, if the RAM sizes as used
> in U-Boot and in Linux differ, at least one of these is wrong.  This
> is a bug that should be fixed.  What you attempt to do is implementing
> a method to paper over such bugs.  This is not a good thing to do.

We'll probably just have to disagree that u-boot needs to know about 
memory it can't access on 32-bit PAE system unless you want to 
implement paging.

Rob