[U-Boot] [PATCH v2] spl: eMMC/SD: Provide one __weak spl_boot_mode() function

[Marek Vasut]

On 02/10/2018 11:25 AM, Lukasz Majewski wrote:
> On Sat, 10 Feb 2018 11:00:12 +0100
> Marek Vasut <marex at denx.de> wrote:
> 
>> On 02/10/2018 10:57 AM, Lukasz Majewski wrote:
>>> Hi Jonathan,
>>>   
>>>> On Sat, Feb 10, 2018 at 01:45:16AM +0100, Lukasz Majewski wrote:  
>>>>> Hi Jonathan,
>>>>>     
>>>>>> On Sat, Feb 03, 2018 at 11:00:35AM -0200, Fabio Estevam
>>>>>> wrote:    
>>>>>>> On Sat, Feb 3, 2018 at 5:29 AM, Lukasz Majewski <lukma at denx.de>
>>>>>>> wrote:      
>>>>>>>> The goal of this patch is to clean up the code related to
>>>>>>>> choosing SPL MMC boot mode.
>>>>>>>>
>>>>>>>> The spl_boot_mode() now is called only in spl_mmc_load_image()
>>>>>>>> function, which is only compiled in if CONFIG_SPL_MMC_SUPPORT
>>>>>>>> is enabled.
>>>>>>>>
>>>>>>>> To achieve the goal, all per mach/arch implementations
>>>>>>>> eligible for unification has been replaced with one __weak
>>>>>>>> implementation.
>>>>>>>>
>>>>>>>> Signed-off-by: Lukasz Majewski <lukma at denx.de>
>>>>>>>> Reviewed-by: Marek Vasut <marex at denx.de>
>>>>>>>> Reviewed-by: Stefano Babic <sbabic at denx.de>
>>>>>>>> Acked-by: Michal Simek <michal.simek at xilinx.com> (For
>>>>>>>> ZynqMP)      
>>>>>>>
>>>>>>> Nice cleanup:
>>>>>>>
>>>>>>> Reviewed-by: Fabio Estevam <fabio.estevam at nxp.com>      
>>>>>>
>>>>>> This has broken booting via mmc with mx6cuboxi for me.
>>>>>>
>>>>>> SPL loops on
>>>>>>
>>>>>> U-Boot SPL 2018.03-rc1-00212-g48914fc119 (Feb 10 2018 - 11:04:33
>>>>>> +1300) Trying to boot from MMC1
>>>>>> Failed to mount ext2 filesystem...
>>>>>> spl_load_image_ext: ext4fs mount err - 0    
>>>>>
>>>>> Could you check what is the status of following defines in
>>>>> your .config file:
>>>>>
>>>>> CONFIG_SPL_FAT_SUPPORT
>>>>> CONFIG_SUPPORT_EMMC_BOOT    
>>>>
>>>> "# CONFIG_SPL_FAT_SUPPORT is not set"
>>>>
>>>> CONFIG_SUPPORT_EMMC_BOOT is not in the .config at all
>>>>
>>>> CONFIG_SPL_EXT_SUPPORT previously for imx6 would result in
>>>> MMCSD_MODE_RAW but now it results in MMCSD_MODE_FS.
>>>>  
>>>>>
>>>>> And if by any chance your don't have:
>>>>> CONFIG_SPL_EXT_SUPPORT defined?    
>>>>
>>>> CONFIG_SPL_EXT_SUPPORT=y  
>>>
>>> Could you for test comment out the above define?  
>>
>> If you look at the logic in spl_mmc.c , you'll see this will work. But
>> what about users who have EXT enabled and want to use it as fallback
>> after loading from RAW failed ? :)
>>
> 
> The code now in __weak function has been took directly from socfpga
> port (as looked as clean and simple). 
> 
> Apparently, this port did not provide such fallback facility :-)

That's correct (for reasons which are beyond the scope of this discussion).

> It seems to me that the code from imx function would need to be used
> instead.
The code in IMX will fail if you have only CONFIG_SPL_FAT_SUPPORT
enabled in SPL .

Give it some more thought and look esp. for the case fallthroughs in the
spl_mmc code, that makes it nasty.

The RAW support is there only to support legacy stuff until those ports
discover filesystems and their benefits (IMO).

-- 
Best regards,
Marek Vasut