[U-Boot] [U-Boot, v2] env: mmc/fat/ext4: make sure that the MMC sub-system is initialized before using it

[Lukasz Majewski]

On Mon, 26 Feb 2018 19:59:46 +0530
Faiz Abbas <faiz_abbas at ti.com> wrote:

> Hi,
> 
> On Sunday 25 February 2018 07:18 PM, Tom Rini wrote:
> > On Sun, Feb 25, 2018 at 09:53:10AM +0100, Wolfgang Denk wrote:  
> >> Dear Tom Rini,
> >>
> >> In message <20180224215325.GQ4311 at bill-the-cat> you wrote:  
> >>>  
> >>>> Why do you ignore this NAK, and why do you add this patch so
> >>>> late in the release cycle anyway?  
> >>>
> >>> Sorry, didn't v2 address your concerns?  We don't initialize the
> >>> device because maybe we'll have env there, we initalize mmc
> >>> because we need to check that it is there.  Thanks!  
> >>
> >> No, it does not.  As is, we initialize MMC in the EXT4 code (in
> >> env_ext4_load()). If we go that route, we would have to add similar
> >> init calls to all other file systemn load routines as well.
> >>
> >> This does not make sense to me.  This pollutes the code with
> >> unrelated things - file system code should not depend on any
> >> underlaying driver suport.  It increases code size, makes the code
> >> harder to maintain (if you need to change this, there is good
> >> chances to forget the same change in other file systems), and there
> >> is a good chance that in the end you will initialzie MMC even if
> >> you don't use it.
> >>
> >> We should keep the code clean and orthogonal.  Driver init code has
> >> no place in file system code.
> >>
> >> If needed, the drivers have to make sure to auto--initialize on
> >> first access.
> >>
> >> I hold my NAK on this patch.  It is the wrong thing to do.  
> > 
> > I think you have this a little bit wrong.  But, it's also where we
> > are with the DM conversion.  This _is_ the first place we're trying
> > to access the MMC.  And it's not in the filesystem code, it's in the
> > environment code.
> > 
> > That said, Faiz, what exactly is the failure sequence (and
> > hardware)? Thanks!
> >  
> 
> 
> The failure happens in SPL when booting from a non-MMC device (say
> NAND) and environment is in MMC. I have seen it in am335x_evm (with
> NAND and ethernet boot mode) and in dra7xx_evm (with qspi boot mode).
> 
> The failure sequence is as follows:
> 
> 1. spl_start_uboot() in the appropriate board file calls env_load().

Just out of curiosity - is the env_load() preceded with env_init() ?

Maybe env_init() is the place to resolve the issue with eMMC
init (to call board_mmc_init() for SPL) ?



> 
> 2. Since ENV_IS_IN_FAT and CONFIG_ENV_FAT_INTERFACE=mmc,
> env_fat_load() eventually leads to a call to find_mmc_device() in
> drivers/mmc/mmc_legacy.c or mmc-uclass.c
> 
> 3. Since the mmc devices have not been probed (by a call to
> mmc_initialize()), SPL is not able to get the environment and I get
> this warning message:
> 
> *** Warning - No MMC card found, using default environment
> 
> 
> Note: In case of legacy mode (CONFIG_BLK is not enabled), SPL crashes
> because of dereferencing a NULL pointer (mmc_devices) in
> find_mmc_device().
> 
> 
> Thanks,
> Faiz
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180228/3324f292/attachment.sig>