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

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2256 bytes --]

On Mon, Jan 15, 2024 at 01:42:51PM +0530, Siddharth Vadapalli wrote:
> Hello Tom,
> 
> On 12/01/24 18:56, Tom Rini wrote:
> > On Fri, Jan 12, 2024 at 12:17:50PM +0530, Siddharth Vadapalli wrote:
> > 
> >> From: Kishon Vijay Abraham I <kishon@ti.com>
> >>
> >> Call dram_init_banksize() from spl_board_init() otherwise TFTP download
> >> fails with error "TFTP error: trying to overwrite reserved memory..."
> >> due to lmb_get_free_size() not able to find unreserved region due
> >> to lack of DRAM size info. Required to support Ethernet boot on AM62x.
> >>
> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> >> ---
> >>  board/ti/am62x/evm.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/board/ti/am62x/evm.c b/board/ti/am62x/evm.c
> >> index ad93908840..35f291d83a 100644
> >> --- a/board/ti/am62x/evm.c
> >> +++ b/board/ti/am62x/evm.c
> >> @@ -85,6 +85,9 @@ void spl_board_init(void)
> >>  	if (IS_ENABLED(CONFIG_SPL_SPLASH_SCREEN) && IS_ENABLED(CONFIG_SPL_BMP))
> >>  		splash_display();
> >>  
> >> +	if (IS_ENABLED(CONFIG_SPL_ETH))
> >> +		/* Init DRAM size for R5/A53 SPL */
> >> +		dram_init_banksize();
> > 
> > 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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]