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

[Faiz Abbas]

Hi,

On Wednesday 28 February 2018 02:38 PM, Lukasz Majewski wrote:
> 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) ?
> 

That was the case in v1.

Check Wolfgang's comments there.

Thanks,
Faiz