Dear Tom,
In message 20140409182426.GV23803@bill-the-cat you wrote:
I understand what you mean and what you want, but I'm not really happy about it.
Code or comment wise?
Both...
Right, so we have a mismatch between function name (fdt_fixup_memory_bank) and function of the node (Documentation/devicetree/booting-without-of.txt in the kernel is, sadly, the best description I can find of the memory node bindings). We itterate over the number of "banks" passed in (1 on PowerPC, CONFIG_NR_DRAM_BANKS on ARM, which is between 1 and 4) and do, as the
If I u nderstand correctly, CONFIG_NR_DRAM_BANKS gives only the maximum possible number of banks. On the actual system less banks may be present.
binding expects, set the reg property correctly (base, size) for each "bank". It would be more correct to call this "ranges" rather than "banks", or perhaps "nr_ranges".
Yes, ture. But then, AFAICT ARM has never made such clear definition of terms, and for the tyical ARM memory controllers "range" and "bank" are actually synonyms, so this never bothered anybody.
And is dropping the (u64) not a problem? bd->bi_memstart is just an "unsigned long", but fdt_fixup_memory_banks() expects a u64 ?
Oops, I don't know how I missed that. Or rather, what the hell is up with calling fdt_fixup_memory() in two places on PowerPC? I just changed the call in board/freescale/t1040qds/t1040qds.c::ft_board_setup, which uses phy_addr_t/phy_size_t on getenv_lowmem_... and this is fine, but arch/powerpc/cpu/mpc85xx/fdt.c::ft_cpu_setup calls bd->bi_mem* and needs a cast.
This should probably best be answered by the MPC85xx experts...
Best regards,
Wolfgang Denk