[U-Boot] spl_mmc: allow to load raw image

[Guillaume Gardet <guillaume.gardet@free.fr>]

Le 02/05/2016 11:12, Alexander Graf a ?crit :
> On 05/02/2016 10:58 AM, Guillaume Gardet wrote:
>>
>>
>> Le 02/05/2016 09:35, Alexander Graf a ?crit :
>>>
>>> On 02.05.16 04:29, Derald D. Woods wrote:
>>>> On 05/01/2016 09:17 PM, Derald D. Woods wrote:
>>>>> On 05/01/2016 08:57 PM, Tom Rini wrote:
>>>>>> On Sun, May 01, 2016 at 08:32:48PM -0500, Derald D. Woods wrote:
>>>>>>> On 05/01/2016 03:37 AM, Masahiro Yamada wrote:
>>>>>>>> Hi Adam,
>>>>>>>>
>>>>>>>>
>>>>>>>> 2016-04-30 3:06 GMT+09:00 Adam Ford <aford173@gmail.com>:
>>>>>>>>> On Fri, Apr 29, 2016 at 12:53 PM, Tom Rini <trini@konsulko.com>
>>>>>>>>> wrote:
>>>>>>>>>> On Fri, Apr 29, 2016 at 09:59:00AM -0500, Adam Ford wrote:
>>>>>>>>>>
>>>>>>>>>>> Does anyone with an OMAP3 board have any issues with this patch? I
>>>>>>>>>>> will admit I haven't stayed on top of stuff due to moving, and
>>>>>>>>>>> other
>>>>>>>>>>> issues at home, but I pulled down the master to reviews some on
>>>>>>>>>>> related stuff, and found that master doesn't boot.  I used git
>>>>>>>>>>> bisect
>>>>>>>>>>> this morning and it narrowed down a problem with booting to this
>>>>>>>>>>> patch.
>>>>>>>>>>>
>>>>>>>>>>> With the patch, I get:
>>>>>>>>>>>
>>>>>>>>>>> U-Boot SPL 2016.03-00378-g4976f48 (Apr 29 2016 - 09:25:27)
>>>>>>>>>>> Trying to boot from MMC
>>>>>>>>>> OK.  Do you have u-boot.bin or u-boot.img (which?) written to the
>>>>>>>>>> raw
>>>>>>>>>> offset in MMC or from filesystem?  Based on the log it looks like
>>>>>>>>>> filesystem.
>>>>>>>>> I have u-boot.img copied to the fatfs on the card, but I didn't
>>>>>>>>> put it
>>>>>>>>> in a specific location.
>>>>>>>>>
>>>>>>>>> I never used to have to do that.  Is this a new behavior and is it
>>>>>>>>> documented somewhere?
>>>>>>>>>
>>>>>>>>> adam
>>>>>>>> You are expecting to boot it from FAT,
>>>>>>>> but I think spl_boot_mode() on your board returns MMCSD_MODE_RAW.
>>>>>>>>
>>>>>>>> Can you fix the function to return MMCSD_MODE_FS?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> This commit changed to allow to load raw U-Boot image,
>>>>>>>> so MMCSD_MODE_RAW never fails.
>>>>>>>>
>>>>>>>> So, you can no longer rely on the former behavior
>>>>>>>> "try MMCSD_MODE_RAW first, and fallback to MMCSD_MODE_FS".
>>>>>>>>
>>>>>>> So everyone loading MLO from the FAT filesystem is now wrong? I am
>>>>>>> trying to understand how this came into being the default.
>>>>>> ... yes, we can't break the case of SPL+U-Boot being on FS on MMC1.  I
>>>>>> wonder if:
>>>>>> commit 22d90d560a2b01c47f180e196e6c6485eb8e65db
>>>>>> Author: Alexander Graf <agraf@suse.de>
>>>>>> Date:   Tue Mar 1 09:56:34 2016 +0100
>>>>>>
>>>>>>       omap3: Use raw SPL by default for mmc1
>>>>>>
>>>>>> Isn't part of what's going wrong now.
>>>>>>
>>>>> Reverting that commit worked for me!
>>>>>
>>>>> ---8<----------------------------------------------
>>>>>
>>>>> U-Boot SPL 2016.05-rc3-00012-gfccdb28-dirty (May 01 2016 - 21:10:05)
>>>>> Trying to boot from MMC1
>>>>> reading args
>>>>> spl_load_image_fat_os: error reading image args, err - -1
>>>>> reading u-boot.img
>>>>> reading u-boot.img
>>>>>
>>>>>
>>>>> U-Boot 2016.05-rc3-00012-gfccdb28-dirty (May 01 2016 - 21:10:05 -0500)
>>>>>
>>>>> OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 Ghz
>>>>> Logic DM37x/OMAP35x reference board + LPDDR/NAND
>>>>>
>>>>> ---8<----------------------------------------------
>>>>>
>>>>> Derald
>>>>>
>>>>>
>>>> I should also mention that I have Tom's patch from this mailing list
>>>> thread:
>>>>
>>>> "[U-Boot] [PATCH] omap3: Reduce logic/overo SPL max image size"
>>>>
>>>> So with the reversion and patch I can boot master on 'omap3_logic' again.
>>> Ok, so I'm puzzled. The raw boot path can basically never fail, since
>>> all it does is to verify whether we could read the sectors - and that
>>> usually works:
>>>
>>> http://git.denx.de/?p=u-boot.git;a=blob;f=common/spl/spl.c;h=82e7f58e80f028f7517ec52bd0d73566dae82d28;hb=HEAD#l115
>>>
>>> So how could we ever fall back from raw to fs mode? I can see how we
>>> could fall back from fs loading to raw loading, but the other way around?
>>>
>>> Guillaume, you posted a patch a while back to handle exactly that case.
>>> How did you get there?
>>
>> I think there is (was?) a signature or a header check or something like this.
>>
>> I did not check latest git version. Maybe a patch removed a check?
>
> I can't find any patch that removes such a check. Hrm.

Beaglebone black does not boot either with u-boot on FS. So, I did a git bisect and it returns the following patch as the problem:
**********************************************************************
commit 4976f4827546154bb296dd0fb33b2cdff94be0c1
Author: Masahiro Yamada <yamada.masahiro@socionext.com>
Date:   Wed Mar 16 12:10:00 2016 +0900

     spl_mmc: allow to load raw image

     The function spl_parse_image_header() falls back to a raw image
     if the U-Boot header is missing and CONFIG_SPL_PANIC_ON_RAW_IMAGE
     is undefined.  While, mmc_load_image_raw_sector() only accepts a
     U-Boot legacy image or an FIT image, preventing us from loading a
     raw image.

     Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
     Reviewed-by: Tom Rini <trini@konsulko.com>
**********************************************************************


Guillaume

>
> So Tom, how would you like to roll this? We can either
>
>   1) Check raw after fs, default to fs and revert my patch or
>   2) Leave fs boot broken (regression) or
>   3) Leave raw boot broken (same as 2016.03)
>
> Given that release is in 1 week, I'm wary on option 1. I also don't like regressions. So how about we revert my patch and fix it up with fs-before-raw boot for 2016.07?
>
> I'm also not quite sure what to do about the other patch I mentioned earlier that moves the u-boot.img offset. Maybe revert for this release and add a fallback sector to read from if we couldn't find the header?
>
>
> Alex
>
>