From mboxrd@z Thu Jan 1 00:00:00 1970 From: Faiz Abbas Date: Mon, 26 Feb 2018 19:59:46 +0530 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: <20180225134810.GU4311@bill-the-cat> 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> Message-ID: <9f9d9641-5f71-d15c-6e5d-d37645ab8d6a@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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(). 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