From: Joakim Tjernlund <joakim.tjernlund@transmode.se>
To: u-boot@lists.denx.de
Subject: [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01
Date: Thu, 24 Jan 2013 10:21:53 +0100 [thread overview]
Message-ID: <OF78771BBB.E9A12F58-ONC1257AFD.0031DD1A-C1257AFD.003371A1@transmode.se> (raw)
In-Reply-To: <OF5A9C9414.751DE82C-ONC1257AFD.0030970D-C1257AFD.00314F3A@LocalDomain>
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@tritech.se>,
> > Cc: "u-boot at lists.denx.de" <u-boot@lists.denx.de>, Wolfgang Denk
<wd@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@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
next prev parent reply other threads:[~2013-01-24 9:21 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-23 15:24 [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 Mats Kärrman
2013-01-23 21:25 ` Wolfgang Denk
2013-01-23 21:58 ` Mats Kärrman
2013-01-24 8:40 ` Joakim Tjernlund
[not found] ` <OFD3C888A1.560BEE31-ONC1257AFD.002DB246-C1257AFD.002FAD45@LocalDomain>
2013-01-24 8:58 ` Joakim Tjernlund
[not found] ` <OF5A9C9414.751DE82C-ONC1257AFD.0030970D-C1257AFD.00314F3A@LocalDomain>
2013-01-24 9:21 ` Joakim Tjernlund [this message]
2013-01-24 11:54 ` Mats Kärrman
2013-01-24 13:31 ` Mats Kärrman
2013-01-24 13:48 ` Joakim Tjernlund
-- strict thread matches above, loose matches on Subject: below --
2013-01-23 13:17 Mats Kärrman
2013-01-23 20:40 ` Wolfgang Denk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=OF78771BBB.E9A12F58-ONC1257AFD.0031DD1A-C1257AFD.003371A1@transmode.se \
--to=joakim.tjernlund@transmode.se \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.