[U-Boot] [PATCH] arm: rmobile: bugfix: wrong register saving in lowlevel_init

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

Hi Jeroen,

On Sun, 07 Oct 2012 17:18:27 +0200, Jeroen Hofstee
<dasuboot@myspectrum.nl> wrote:

> Hello All,
> 
> On 10/07/2012 01:34 PM, Enric Balletb? i Serra wrote:
> > Hi Albert,
> >
> > 2012/10/5 Albert ARIBAUD <albert.u.boot@aribaud.net>:
> >> Hi Tetsuyuki,
> >>
> >> On Fri,  5 Oct 2012 13:39:22 +0900, Tetsuyuki Kobayashi
> >> <koba@kmckk.co.jp> wrote:
> >>
> >>> lowlevel_init() of rmobile badly assumed that ip register holds return address.
> >>> The commit "63ee53a7 armv7 cpu_init_crit: Simplify code" breaks this assumption.
> >>> This patch removes this bad assumption and simplify code.
> >>>
> >>> Signed-off-by: Tetsuyuki Kobayashi <koba@kmckk.co.jp>
> >>> ---
> >>>
> >> ...
> > Note that the patch that Tetsuyuki says also breaks SPL support for
> > OMAP3 boards, at least my IGEP boards doesn't boot and hangs at SPL
> > level.
> >
> >    U-Boot SPL 2012.10-rc1-00244-g28e5ac2 (Oct 07 2012 - 13:11:29)
> >
> > Bisecting the problem I encountered the problem is the commit
> > "63ee53a7 armv7 cpu_init_crit: Simplify code".
> >
> > Cheers,
> >      Enric
> >
> I can confirm above. Also the tam3517 som (omap3) fails to boot due to
> mentioned commit. The patch from Tetsuyuki is arch specific (rmobile) so
> that won't fix the omap case. Reverting the patch, 63ee53a, does help.
> 
> Is there anything against reverting the patch (at least for the release...)?

Here is my opinion:

1) I think patch 63ee53a7 is right in considering there is no need for
cpu_init_crit to save lr in ip before calling lowlevel_init especially
considering this is a tail call.

Only lowlevel_init can tell if it uses ip or lr for its own purposes,
thus any saving of ip and/or lr due to the workings of lowlevel_init
should be performed in lowlevel_init.

2) I am not sure that the patch in this discussion depends on 63ee53a7,
because IIUC, the patch simply saves ip "on a stack" then lr into ip,
and after running s_init, restores from ip and ip from the stack; it
never assumes ip contains a return address.

I know we're that close to the release, but I want to be sure we
understand what needs fixing. Kobayashi, Jeroen, can you indicate
precisely how the issues you encounter are related to 63ee53a7?

> Regards,
> Jeroen

Amicalement,
-- 
Albert.