Hi Albert, On Sun, 7 Oct 2012 19:21:27 +0200, Albert ARIBAUD <albert.u.b...@aribaud.net> wrote:
> Hi Albert, > > On Sun, 7 Oct 2012 19:19:37 +0200, Albert ARIBAUD > <albert.u.b...@aribaud.net> wrote: > > > Hi Jeroen, > > > > On Sun, 07 Oct 2012 17:18:27 +0200, Jeroen Hofstee > > <dasub...@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.b...@aribaud.net>: > > > >> Hi Tetsuyuki, > > > >> > > > >> On Fri, 5 Oct 2012 13:39:22 +0900, Tetsuyuki Kobayashi > > > >> <k...@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 <k...@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? > > (adding back Tetsuyuki's mail in the Cc: list -- why had it > disappeared?) > > > > Regards, > > > Jeroen > > > > Amicalement, > > Amicalement, Hmm... I notice only now that I had mentally 'fixed' the order of the restoring lines removed by the patch. Had they been in the right order (mov lr, ip then ldr ip, [sp]) the original code would have worked, albeit probably useless. I suspect that the bad ordering was actually a mistake unseen, and that the dependence on ip being a return address is only due to this mistake. In any case, this makes me *more* determined that 63ee53a7 is right,as well as this patch. Jeroen, I suspect that your problem comes from the fact that the same bug that this patch uncovers and fixes exists also in arch/arm/cpu/armv7/omap3/lowlevel_init.S (lines 216-218 and 228-229) ... and would be better fixed there than by reverting 63ee53a7. Amicalement, -- Albert. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
