[U-Boot] [PATCH 2/2] ARM: davinci: SPL: fix BSS initialization

[Adam Ford]

On Fri, May 24, 2019 at 5:10 AM Sekhar Nori <nsekhar at ti.com> wrote:
>
> On 21/05/19 9:01 PM, Adam Ford wrote:
> > On Tue, May 21, 2019 at 10:09 AM Sekhar Nori <nsekhar at ti.com> wrote:
> >>
> >> U-Boot README recommends initializing SDRAM in board_init_f(). DA850
> >> was doing it as part of board_init_r() (through call to spl_board_init()
> >> which calls arch_cpu_init() which calls da850_ddr_setup())
> >>
> >> This worked fine till commit 15b8c7505819 ("davinci:
> >> da850evm/omapl138-lcdk: Move BSS to SDRAM because SRAM is full") moved
> >> BSS to SDRAM.
> >>
> >> Functions like mmc_initialize() called in board_init_r() assume BSS is
> >> available. Since SDRAM was not initialized when arch/arm/lib/crt0.S tried
> >> to initialize BSS to 0, BSS is not initialized correctly.
> >>
> >> Fix this by simply calling arch_cpu_init() from board_init_f(). Since the
> >> README recommends calling preloader_console_init() from spl_board_init(),
> >> we keep it as-it-is.
> >
> > The README also states preloader_console_init() can get called from
> > board_init_f().  Doing this enables for debugging of board_init_r
>
> Okay, I can change it. spl_board_init() will be empty so I will turn off
>  CONFIG_SPL_BOARD_INIT also.
>
> I was put off by README saying "preloader_console_init() can be called
> here in extremis" which I meant as saying it should be rare to do that.
>
> Debugging board_init_r() should be pretty useful, so perhaps the wording
> there should be more easy.

At least for the da850evm which uses the device tree, you'll need to
call spl_early_init() before the preloader_console_init() in order for
the UART to work correctly and debug board_init_r.

>
> >
> >>
> >> Tested using MMC/SD boot on OMAP-L138 LCDK board.
> >>
> >> Signed-off-by: Sekhar Nori <nsekhar at ti.com>
> >> ---
> >>  arch/arm/mach-davinci/spl.c | 6 +++++-
> >>  1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/mach-davinci/spl.c b/arch/arm/mach-davinci/spl.c
> >> index 103639e34757..117b5ee836f8 100644
> >> --- a/arch/arm/mach-davinci/spl.c
> >> +++ b/arch/arm/mach-davinci/spl.c
> >> @@ -33,10 +33,14 @@ void putc(char c)
> >>
> >>  void spl_board_init(void)
> >>  {
> >> -       arch_cpu_init();
> >>         preloader_console_init();
> >>  }
> >>
> >> +void board_init_f(ulong dummy)
> >> +{
> >> +       arch_cpu_init();
> >
> > What about a reference to board_early_init_f()?
> > Looking at other boards, it appears that we should call that here.
> > Both the LCDK and da850 evm have the function enabled to configure the
> > DSP.
>
> This is only needed at U-Boot stage, right? Looks like it will be called
> because CONFIG_BOARD_EARLY_INIT_F is enabled.

That is my understanding.

Thanks for being flexible on this.  Sorry I broke the lcdk, but I'm
willing to help modernize it as well (ie, help support SPL_OF_CONTROL)
if someone can get me a board.  The general trend is to try and
support these newer DM functions and remove the old legacy code
functions.

adam
>
> Thanks,
> Sekhar