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 variables are not available during board_init_f(). >>>> >>>> They are, since the board runs from RAM at that point already. >>>> >>> From reading the README and common/board_f.c I got the impression, >>> that dram_init() is called before it's save to access mx53_dram_size. >>> And as that change seemed to cure the strange behavior I described >>> in [1] and [2], I prepared a patch for my board, which ended up to be >>> requested for m53evk and mx53loco, too. >> >> Technically yes, board_init_f means it runs from FLASH and has no or >> minimal storage available. On the MX53 with SPL, it's running from RAM >> so it's safe to use static variables too. >> > Thank's for your explanation.
No problem, it's a bit weird. >>>>> '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. >>> 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? btw do you use SPL ? If not, you should ... [...] -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
