[U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01
Joakim Tjernlund
joakim.tjernlund at transmode.se
Thu Jan 24 10:21:53 CET 2013
Joakim Tjernlund/Transmode wrote on 2013/01/24 09:58:35:
>
> Joakim Tjernlund/Transmode wrote on 2013/01/24 09:40:45:
>
> > From: Joakim Tjernlund/Transmode
> > To: Mats Kärrman <Mats.Karrman at tritech.se>,
> > Cc: "u-boot at lists.denx.de" <u-boot at lists.denx.de>, Wolfgang Denk
<wd at denx.de>
> > Date: 2013/01/24 09:40
> > Subject: RE: [U-Boot] mpc512x: Trouble migrating from 2012.07 to
2013.01
> >
> > Mats Kärrman <Mats.Karrman at tritech.se> wrote on 2013/01/23 22:58:56:
> > >
> > > Dear Wolfgang Denk,
> > >
> > > >> Found that it was looping endlessly in
arch/powerpc/lib/ticks.S::wait_ticks
> > > (). Reverting commit "ppc: Create a stack frame for wait_ticks()"
made
> > > everything work again.
> > > >
> > > > This makes no sense to me - especially as it works on all other
> > > > systems.
> > >
> >
> > Me neither, there is not a lot details. I do recall having other
problems with
> > wait_ticks from time to time, sometimes the TB counter(mtfbu, mftb in
get_ticks)
> > would not increment so one would just loop forever in wait_ticks. This
problem
> > had nothing to do with my patch though.
> > Never got around to finding out what caused it, we ended up blaming
unstable HW.
> >
> > Some ideas though:
> > - Perhaps you have some alignment problem, try adding nop's here and
there.
> > - My patch uses r0(which is what one should use to make gdb happy)
instead of r8
> > to stash the LR. Possibly you have some odd dependency on r0, try
using r8 again.
>
> Try getting LR from stack instead of trusting r0 to be valid:
> - mtlr r0 /* restore link register */
> + lwz r0, 20(r1) /* restore link register */
> + mtlr r0
> This is what one should do but as we have complete control of r0 here we
don't need to.
>
> hmm, do you have WATCHDOG_RESET defined? does it use r0?
> I guess the above patch would make wait_ticks safer from accidental use
by
> WATCHDOG_RESET,
> if it works for you, please send a proper patch to u-boot.
Looking at the watchdog impl. I see it can be normal C code. This makes
wait_ticks unsafe
(even before my patch) as wait_ticks relies on r6 and r7 (and with my
patch r0 too)
to be unmodified.
This MIGHT be the fix:
--- a/arch/powerpc/lib/ticks.S
+++ b/arch/powerpc/lib/ticks.S
@@ -56,13 +56,17 @@ wait_ticks:
/* Calculate end time */
addc r7, r4, r7 /* Compute end time lower */
addze r6, r3 /* and end time upper */
-
+ stw r7, 4(r1) /* Stash r6 and r7 */
+ stw r6, 8(r1)
WATCHDOG_RESET /* Trigger watchdog, if needed */
+ lwz r7, 4(r1)
+ lwz r6, 8(r1)
1: bl get_ticks /* Get current time */
subfc r4, r4, r7 /* Subtract current time from end time */
subfe. r3, r3, r6
bge 1b /* Loop until time expired */
- mtlr r0 /* restore link register */
+ lwz r0, 20(r1) /* restore link register */
+ mtlr r0
addi r1,r1,16
blr
Not sure about the 4 and 8 offsets though
Jocke
More information about the U-Boot
mailing list