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

[Simon Goldschmidt]

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.

Regards,
Simon