[U-Boot] [PATCH v2 3/4] arm: imx: m53evk: remove usage of mx53_dram_size

[Patrick Brünn]

>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Montag, 18. Dezember 2017 11:57
>On 12/18/2017 11:47 AM, Patrick Brünn wrote:
>>> From: Marek Vasut [mailto:marex at denx.de]
>>> Sent: Montag, 18. Dezember 2017 10:17
>>> On 12/18/2017 10:02 AM, linux-kernel-dev at beckhoff.com wrote:
>>>> From: Patrick Bruenn <p.bruenn at 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.

>>>> '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.bruenn at 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.
>> 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.

>> Best regards and thanks in advance for any feedback,
>> Patrick
>>
>> [1] https://lists.denx.de/pipermail/u-boot/2017-November/313214.html
>> [2] https://lists.denx.de/pipermail/u-boot/2017-December/314480.html
>>
>> ---
>> Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans
>Beckhoff
>> Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
>>
>>
>
>
>--
>Best regards,
>Marek Vasut
Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075