Re: [PATCH 01/10] board: ti: am62x: Init DRAM size in R5/A53 SPL

[Chintan Vankar <c-vankar@ti.com>]

On 22/01/24 10:11, Siddharth Vadapalli wrote:
> 
> 
> On 20/01/24 22:11, Tom Rini wrote:
>> On Mon, Jan 15, 2024 at 01:42:51PM +0530, Siddharth Vadapalli wrote:
>>> Hello Tom,
>>>
>>> On 12/01/24 18:56, Tom Rini wrote:
> 
> ...
> 
>>>> The list of conditionals in common/spl/spl.c::board_init_r() should be
>>>> updated and probably use SPL_NET as the option to check for.
>>>
>>> Thank you for reviewing the patch and pointing this out. I wasn't aware of it. I
>>> assume that you are referring to the following change:
>>>
>>>          if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) ||
>>> -           IS_ENABLED(CONFIG_SPL_ATF))
>>> +           IS_ENABLED(CONFIG_SPL_ATF) || IS_ENABLED(CONFIG_SPL_NET))
>>>                  dram_init_banksize();
>>>
>>> I shall replace the current patch with the above change in the v2 series. Since
>>> this is in the common section, is there a generic reason I could provide in the
>>> commit message rather than the existing commit message which seems to be board
>>> specific? Also, I hope that the above change will not cause regressions for
>>> other non-TI devices. Please let me know.
>>
>> Yes, that's the area, and just note that networking also requires the
>> DDR to be initialized.
>>
> 
> Thank you for confirming and providing your suggestion for the contents of the
> commit message.
> 
Following Tom's Suggestion of adding CONFIG_SPL_NET in common/spl/spl.c
"dram_init_banksize()", the issue of fetching a file at SPL stage seemed
to be fixed. However the commit "ba20b2443c29", which sets gd->ram_top
for the very first time in "spl_enable_cache()" results in
"arch_lmb_reserve()" function reserving memory region from Stack pointer
at "0x81FFB820" to gd->ram_top pointing to "0x100000000". Previously
when gd->ram_top was zero "arch_lmb_reserve()" was noop. Now using TFTP
to fetch U-Boot image at SPL stage results in "tftp_init_load_addr()"
function call that invokes "arch_lmb_reserve()" function, which reserves
entire memory starting from Stack Pointer to gd->ram_top leaving no
space to load U-Boot image via TFTP since TFTP loads files at pre
configured memory address at "0x82000000".

As a workaround for this issue, one solution we can propose is to
disable the checks "lmb_get_free_size()" at SPL and U-Boot stage. For
that we can define a new config option for LMB reserve checks as
"SPL_LMB". This config will be enable by default for the backword
compatibility and disable for our use case at SPL and U-Boot stage.