On 12/18/2017 01:16 PM, Patrick Brünn wrote: >> From: Marek Vasut [mailto:ma...@denx.de] >> Sent: Montag, 18. Dezember 2017 12:52 >> On 12/18/2017 12:40 PM, Patrick Brünn wrote: >>>> From: Marek Vasut [mailto:ma...@denx.de] >>>> Sent: Montag, 18. Dezember 2017 11:57 >>>> On 12/18/2017 11:47 AM, Patrick Brünn wrote: >>>>>> From: Marek Vasut [mailto:ma...@denx.de] >>>>>> Sent: Montag, 18. Dezember 2017 10:17 >>>>>> On 12/18/2017 10:02 AM, linux-kernel-...@beckhoff.com wrote: >>>>>>> From: Patrick Bruenn <p.bru...@beckhoff.com> >>>>>>> >>>>>>> 'static uint32_t mx53_dram_size[2];' was used in board specific >>>>>>> dram_init(), dram_init_banksize() and get_effective_memsize() to >> avoid >>>>>>> multiple calls to get_ram_size(). >>>>>>> >>>>>>> Reused dram initialization functions from arch/arm/mach- >>>>>> imx/mx5/mx53_dram.c >>>>>>> >>>>>>> Signed-off-by: Patrick Bruenn <p.bru...@beckhoff.com> >>>>>>> >>>>>>> --- >>>>>>> >>>>>>> Changes in v2: None >>>>>>> >>>>>>> >>>>>>> Only compile tested with make m53evk_defconfig >>>>>> >>>>>> Are you sure this patch retains the original functionality ? Note the >>>>>> warning ... >>>>> >>>>> Effectively it changes: >>>>> - return mx53_dram_size[0]; >>>>> + return get_ram_size((void *)PHYS_SDRAM_1, 1 << 30); >>>>> >>>>> So, yes I am convinced that get_effective_memsize() still returns only the >>>> size >>>>> of the first dram bank. >>>> >>>> I suspect that will fails on M53 due to it's split-bank configuration. >>>> The board has two DRAM banks with a hole between them. >>>> >>> This is how mx53_dram_size was initialized on m53 before that patch: >>> - mx53_dram_size[0] = get_ram_size((void *)PHYS_SDRAM_1, 1 << 30); >>> - mx53_dram_size[1] = get_ram_size((void *)PHYS_SDRAM_2, 1 << 30); >>> >>> So to me that's, absolutely the same. With the only difference, that >> get_ram_size(bank0) >>> is called multiple times, now. >> >> The get_ram_size() above is called for two different bank (addresses), >> not for bank0 twice. Maybe I'm missing something. >> >> btw where's the patch adding mx53_dram.c ? I don't see it in mainline yet. >> > It's #2 of this series: > https://lists.denx.de/pipermail/u-boot/2017-December/314742.html
Ah, sorry, I missed that. >>>>> However, I would be fine with just keeping the changes to my board >>>> (cx9020). >>>>> And if anyone has a better idea of what might be the root cause of [1] >> and >>>> [2], >>>>> I would absolute appreciate any input to that. >>>> >>>> If your board also has two DRAM banks with a hole between them and you >>>> can test if this works fine, then I'm OK with this change. >>>> >>> Yes, cx9020 uses two 512MB banks with 1GB between PHYS_SDRAM_1/2, >> too. >> >> Then that should be the same situation. Can you share "bdinfo" from that >> board of yours? >> > => bdinfo > arch_number = 0x00000000 > boot_params = 0x70000100 > DRAM bank = 0x00000000 > -> start = 0x70000000 > -> size = 0x20000000 > DRAM bank = 0x00000001 > -> start = 0xB0000000 > -> size = 0x20000000 > eth0name = FEC > ethaddr = 00:01:05:23:87:85 > current eth = FEC > ip_addr = <NULL> > baudrate = 115200 bps > TLB addr = 0x8FFF0000 > relocaddr = 0x8FF8B000 > reloc off = 0x1878B000 > irq_sp = 0x8F586960 > sp start = 0x8F586950 > FB base = 0x8F58C7C0 > Early malloc usage: 134 / 400 > fdt_blob = 8f586978 Looks fine then. >> btw do you use SPL ? If not, you should ... > I will read more about SPL. Until now, I thought I will only benefit, > if my initial boot media is limited in size. As we boot from sdcard, > that wasn't a problem to store 360k u-boot. The other is that you can ie. skip the whole u-boot altogether and boot linux directly. I wonder if it would be better to keep the static variable and avoid calling the get_ram_size() twice, it can save some CPU cycles. Besides that, thanks for the explanation/discussion !1 -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot