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

[Tom Rini]

On Fri, Jan 10, 2020 at 03:57:47PM +0100, Giulio Benetti wrote:
> 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

Yes, this is on my list of things to get back to as I clear out parts of
my queue, thanks for the reminder.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200110/cb9536a8/attachment.sig>