From patchwork Thu Jan 24 09:21:53 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 Date: Wed, 23 Jan 2013 23:21:53 -0000 From: Joakim Tjernlund X-Patchwork-Id: 215280 Message-Id: Cc: "u-boot@lists.denx.de" , =?ISO-8859-15?Q?Mats_K=E4rrman?= 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 , > > Cc: "u-boot@lists.denx.de" , Wolfgang Denk > > Date: 2013/01/24 09:40 > > Subject: RE: [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 > > > > Mats Kärrman 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: Not sure about the 4 and 8 offsets though Jocke --- 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