[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 12:52
>On 12/18/2017 12:40 PM, Patrick Brünn wrote:
>>> 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 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.
>
>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

>>>> 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

>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.

Regards,
Patrick
---
Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075