[U-Boot] MMC boot broken on OMAP-L138 LCDK

[Adam Ford]

On Mon, May 20, 2019 at 4:09 AM Sekhar Nori <nsekhar at ti.com> wrote:
>
> On 17/05/19 6:34 PM, Adam Ford wrote:
> > On Fri, May 17, 2019 at 7:36 AM Sekhar Nori <nsekhar at ti.com> wrote:
> >>
> >> Hi Adam,
> >>
> >> MMC/SD boot is broken on OMAP-L138 LCDK. This is since v2018.11.
> >> v2018.09 is fine.
> >>
> >> The first breakage occurred with commit 21af33ed0319 ("ARM: davinci:
> >> omapl138_lcdk: Enable DM_MMC"). This commit moved to DM_MMC for U-Boot
> >> while keeping legacy mode for SPL.
> >
> > I am going to preface this by stating I don't have the LCDK.  Peter
> > Howard (added in CC) asked me to submit this on his behalf, so I
> > reviewed his patch and submitted it which is why both our names are on
> > the s-o-b.
> >
> >>
> >> The "#ifndef CONFIG_DM_MMC" introduced by this commit is incorrect I
> >> think because CONFIG_DM_MMC is defined in SPL build too and because of
> >> this board_mmc_init() never gets defined. I think the intent was to
> >> define board_mmc_init() for SPL case, and for that the following diff
> >> should do:
> >>
> >> diff --git a/board/davinci/da8xxevm/omapl138_lcdk.c b/board/davinci/da8xxevm/omapl138_lcdk.c
> >> index 2c2f885d43e4..fe1bf4410145 100644
> >> --- a/board/davinci/da8xxevm/omapl138_lcdk.c
> >> +++ b/board/davinci/da8xxevm/omapl138_lcdk.c
> >> @@ -353,7 +353,7 @@ int misc_init_r(void)
> >>         return 0;
> >>  }
> >>
> >> -#ifndef CONFIG_DM_MMC
> >> +#if !CONFIG_IS_ENABLED(DM_MMC)
> >>  #ifdef CONFIG_MMC_DAVINCI
> >>  static struct davinci_mmc mmc_sd0 = {
> >>         .reg_base = (struct davinci_mmc_regs *)DAVINCI_MMC_SD0_BASE,
> >>
> >
> > I would agree that the #ifndef should be replaced with if
> > !CONFIG_IS_ENABLED(DM_MMC), but I would recomend checking into
> > migrating the LCDK to DM. and DM_MMC.
>
> Okay. For now, I will submit a fix changing the #ifndef.
>
> >> With this fixed, MMC/SD boot is still broken after commit 15b8c7505819
> >> ("davinci: da850evm/omapl138-lcdk: Move BSS to SDRAM because SRAM is
> >> full").
> >>
> >> After this commit, mmc_initialize() in drivers/mmc/mmc.c fails because
> >> the static variable 'initialized' does not get initialized to 0 as
> >> intended in code and gets set to a random value because of which no
> >> MMC/SD device gets registered. I did notice that 'initialized' is in
> >> BSS, so I believe after this commit BSS is not getting set to 0.
> >
> > I sent Peter H a list of patches to consider because I spent a lot of
> > time trying to modernize the da850_evm using DM, and DM_SPL and
> > eventually SPL_OF_CONTROL which ultimately eliminated a lot of the
> > manual initializing of the various drivers.
> >
> > See:
> > commit a4670f8ebb5b4df6afeb5155fb5b44c1d1d154b9 Enable DM and device
> > tree support for da850-evm
> > commit c4fa049a121457ff38b74daac32e18d7afbd4538 shrunk SPL by 6.5k
> > commit 391328dc30b78be6f0d1181088eda0fd7febd5f3 removed DM_I2C_COMPAT
> > commit cb19c29398cb84e72236ab6bae3763028fce5d44 board: da8xxevm: Add
> > SPL DM for serial, spi (with platdata)
> > commit 591353d011b5ff6dbc8511b7a839cddf5b610495 ARM: da850evm:
> > Increase Malloc Size
> > commit 15b8c7505819fa48dd99fb51e91b9536f341fde1 moved BSS to SDRAM
> > because SRAM is full
> > commit f7c1d53605d9ec528abacda9ba1763c67221fc88 ARM: davinci:
> > da850evm: Enable SPL_OF_CONTROL without PLATDATA
> > commit fd3c26f3f921ad7addf95857cdb99a883c6e497a ARM: da850evm: Fix
> > broken SPI Flash
> >
> > Having said that, the hardware I have for the da850 EVM doesn't
> > support MMC booting, so I cannot verify whether or not MMC initializes
> > properly in SPL.  I know the MMC initializes in U-Boot once it's
> > booted from SPI flash.
>
> MMC/SD boot can be simulated on any board by hacking spl_boot_device()
> in arch/arm/mach-davinci/spl.c

That's a good idea.  I never thought of that.  I'll have to add the
mmc entry to the device tree and enable MMC in SPL, but I'll try to
run some tests this week.  I'm out of town Friday-Tuesday, so if I'll
try to get to it by Wednesday.

>
> >
> > If someone wants to send me an LCDK, I'd be happy to look at
> > attempting to modernize the U-Boot for it.  Otherwise, we may want to
>
> I will try to get you a board.

thanks.

>
> > consider splitting the linker scripts between  LCDK and da850 evm so
> > da850evm can use SPL_OF_CONTROL with enough memory and LCDK can keep
> > it's legacy code status in SPL.
>
> I think this is not an LCDK specific problem. Looking at
> arch/arm/lib/crt0.S, BSS is cleared before board_init_r() is called.
>
> In case of DA850, DDR is initialized in
> board_init_r()->spl_board_init()->arch_cpu_init()->da850_ddr_setup()
>
> So, after you moved BSS to use SDRAM, the BSS clearing code was working
> on uninitialized DDR. I think the right fix may be to call
> arch_cpu_init() in board_init_f()?

What about adding a memset function to the end of spl_board_init() so
after arch_cpu_init() is called,and after preloader_consol_init() is
called, we clear BSS?  I ran some preliminary tests on da850evm and it
didn't see to break anything.  There are a few boards that appear to
do things in this order.

adam
>
> Thanks,
> Sekhar