From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukasz Majewski Date: Wed, 28 Feb 2018 10:08:46 +0100 Subject: [U-Boot] [U-Boot, v2] env: mmc/fat/ext4: make sure that the MMC sub-system is initialized before using it In-Reply-To: <9f9d9641-5f71-d15c-6e5d-d37645ab8d6a@ti.com> References: <1518443671-11417-1-git-send-email-faiz_abbas@ti.com> <20180220220328.GC4311@bill-the-cat> <20180224205845.739E7240112@gemini.denx.de> <20180224215325.GQ4311@bill-the-cat> <20180225085310.248C324018D@gemini.denx.de> <20180225134810.GU4311@bill-the-cat> <9f9d9641-5f71-d15c-6e5d-d37645ab8d6a@ti.com> Message-ID: <20180228100846.2f7ec7b7@jawa> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Mon, 26 Feb 2018 19:59:46 +0530 Faiz Abbas 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@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: