From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?Q?Mats_K=E4rrman?= Date: Thu, 24 Jan 2013 11:54:15 +0000 Subject: [U-Boot] mpc512x: Trouble migrating from 2012.07 to 2013.01 In-Reply-To: References: , <20130123212559.309ED2454A1@gemini.denx.de> , Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Joakim Tjernlund/Transmode wrote on 2013/01/24 09:58:35: > > > > > > 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 Thanks for feedback! I will look into this ASAP (maybe not until tomorrow). I'll report back as soon as I have a result. BR, Mats