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

[Sekhar Nori]

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

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

> 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()?

Thanks,
Sekhar