[U-Boot] Falcon boot breaks on DRA7 because of commit b9c8ccab "env_mmc.c: Allow environment to be used within SPL"

[Tom Rini]

On Wed, Jan 25, 2017 at 11:21:32AM +0100, Jean-Jacques Hiblot wrote:
> 
> 
> On 24/01/2017 20:11, Tom Rini wrote:
> >On Tue, Jan 24, 2017 at 06:04:47PM +0100, Jean-Jacques Hiblot wrote:
> >>
> >>On 24/01/2017 16:46, Tom Rini wrote:
> >>>>I had noticed that it's quite old indeed. I didn't mean that it's a
> >>>>regression. I'm just puzzled by the commit. what is its purpose ?
> >>>>why is SPL not using  CONFIG_SYS_MMC_ENV_DEV ?
> >>>Because in SPL we do not have both MMC devices initialized.
> >>That is not always the case. Actually in spl_mmc.c the code requires
> >>us to register more than one MMC device to work properly when
> >>multiple MMC boot devices can be used (see
> >>spl_mmc_get_device_index())
> >>I did the test of registering only MMC2 when booting from eMMC, the
> >>SPL fails because it can't find device 1:
> >>Trying to boot from MMC2_2
> >>MMC Device 1 not found
> >>spl: could not find mmc device. error: -19
> >>
> >>>We register
> >>>the one we booted from and thus it is device 0 to U-Boot in this case.
> >>>I suspect the rest of the issues stem from this quirk, or something
> >>>having broken around this quirk.  Thanks!
> >Right.  So I suspect the problem is that some level of the env_mmc.c
> >code needs to be adapted again for the change in how SPL now works with
> >the possibility of multiple devices.  It's possible that the change you
> >found is the right fix, please investigate a bit more and confirm
> >things before submitting a proper patch, thanks!
> I did more tests and it turns out that there I find no real benefit
> of registering only the controller for the boot device.
> The initialization of the MMC/SD/eMMC is done only prior accessing
> it, not when it's registered. So in terms of boot time the impact of
> registering many controllers is not significant.
> By registering the same controllers in SPL and in u-boot, we would
> get the same mapping for the MMC devices in SPL and u-boot. It would
> remove a source of confusion and of #ifdef CONFIG_SPL_BUILD
> 
> The catch is that many boards register only one MMC controller in
> the SPL, depending on what the boot source is (ex: board_mmc_init()
> in board/freescale/mx6slevk/mx6slevk.c)
> To reduce the risk of regression, we could deal with those boards in
> 2 steps:
> 1) Don't change the code of the board except to override the weak
> function mmc_get_env_dev() and make it return 0. This is not likely
> to introduce a regression
> 2) One by one, change the code of the boards to register all the
> controllers in SPL as done in u-boot. Also we need to adapt
> spl_boot_device() to return the right boot device. There has been a
> partial attempt at this ""ARM: mx6: add MMC2 boot device detection
> support in SPL" but had to be reverted probably because it was not
> coherent with the registration of the controllers.

Due to the issue you mention in #2, we probably need to do #1 and with
care and testing, as there's enough places that assume SPL is a single
MMC device that it'll be problematic to do them one at a time.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170126/2f61773d/attachment.sig>