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

[Alexander Graf <agraf@suse.de>]

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.

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