Hello.

(2012/10/08 2:35), Albert ARIBAUD wrote:
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.

I have the same opinion as Albert.
63ee53a7 is right. It should not be reverted.
lowlevel_init.S in rmobile and omap have mistake, it should be fixed.

I checked 2012.10-rc3. It has already done.
Thank you very much.


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to