[U-Boot] [PATCH] spl: fix entry_point equal to load_addr

[Giulio Benetti]

Hi All,

On 12/7/19 10:28 PM, Simon Goldschmidt wrote:
> Hi Tom,
> 
> On Fri, Dec 6, 2019 at 3:55 PM Tom Rini <trini at konsulko.com> wrote:
>>
>> On Fri, Dec 06, 2019 at 03:05:55PM +0100, Simon Goldschmidt wrote:
>>> On Fri, Dec 6, 2019 at 2:49 PM Giulio Benetti
>>> <giulio.benetti at benettiengineering.com> wrote:
>>>>
>>>> Hello Tom, all,
>>>>
>>>> On 12/6/19 2:34 PM, Tom Rini wrote:
>>>>> On Fri, Dec 06, 2019 at 01:58:46PM +0100, Simon Goldschmidt wrote:
>>>>>> On Fri, Dec 6, 2019 at 1:46 PM Patrice CHOTARD <patrice.chotard at st.com> wrote:
>>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> This patch is breaking the STM32MP15 basic boot (spl => u-boot).
>>>>>>
>>>>>> Looking at socfpga gen5 u-boot.img, this is probably broken as well.
>>>>>>
>>>>>> And I don't even see any RB or TB tags here :-(
>>>>>
>>>>> Ugh, what the heck?  I applied this because I looked over the changes
>>>>> and they seemed correct.  I'm quite willing to just revert this but I
>>>>> would like to know how it's breaking.  Sorry all!
>>>>>
>>>>
>>>> IMHO this is due to wrong images creation with mkinage, especially when
>>>> passing parameters with -a and -e flags.
>>>>
>>>> In my case I need them to be:
>>>> -a 0x80002000 (load address) => CONFIG_SYS_TEXT_BASE
>>>> -e 0x800023FD (entry point where SPL jumps to) => CONFIG_SYS_UBOOT_START
>>>
>>> Well, CONFIG_SYS_UBOOT_START is only set in 15 files in include/configs, so I
>>> guess a lot more boards might be broken...
>>>
>>>>
>>>> So *maybe* on STM32MP1 and other broken boards -e
>>>> (CONFIG_SYS_UBOOT_START) is not equal to -a (CONFIG_SYS_TEXT_BASE) as
>>>> was assumed before(but wrong).
>>>>
>>>> Indeed CONFIG_SYS_UBOOT_START is set to 0 if not specified in
>>>> u-boot/Makefile:
>>>> `
>>>> # U-Boot entry point, needed for booting of full-blown U-Boot
>>>> # from the SPL U-Boot version.
>>>> #
>>>> ifndef CONFIG_SYS_UBOOT_START
>>>> CONFIG_SYS_UBOOT_START := 0
>>>> endif
>>>> `
>>>>
>>>> So probably broken boards try to jump to absolute 0.
>>>> A solving patch would be:
>>>> ifndef CONFIG_SYS_UBOOT_START
>>>> CONFIG_SYS_UBOOT_START := CONFIG_SYS_TEXT_BASE
>>>> endif
>>>
>>> That might work, but I wonder if this is the correct time in the release to do
>>> so.
>>
>> Yes, at this point in the cycle the best option is to revert the
>> original commit and for the next release bring it back after applying
>> Patrice's series to fix the bogus default to CONFIG_SYS_UBOOT_START and
>> cleaning up defconfigs.  Sorry again for all the troubles!
> 
> I just wanted to confirm socfpga_gen5 doesn't boot with this patch
> but it's ok again now you reverted it.

Now that patches:
https://patchwork.ozlabs.org/patch/1205064/
https://patchwork.ozlabs.org/patch/1205063/

and 2020.01 has been released, can you please commit this patch?

I've re-submitted also on patchset to Add i.MXRT family since it's 
needed to boot Linux:
https://patchwork.ozlabs.org/project/uboot/list/?series=152468

Kind regards
-- 
Giulio Benetti
Benetti Engineering sas