RE: [PATCH -pm 1/2] SDRC: check for stuck DLL state machine and kick - Wang Sawsd-A24013

From: "Wang Sawsd-A24013" <cqwang@motorola.com>
To: "Woodruff, Richard" <r-woodruff2@ti.com>,
	Mike Chan <mike@android.com>,
	Kevin Hilman <khilman@deeprootsystems.com>
Cc: linux-omap@vger.kernel.org
Subject: RE: [PATCH -pm 1/2] SDRC: check for stuck DLL state machine and kick
Date: Fri, 19 Jun 2009 03:27:25 +0800	[thread overview]
Message-ID: <B00E06E2766C2744B022DE9BAF3C59D53F268F@zmy16exm69.ds.mot.com> (raw)
In-Reply-To: <13B9B4C6EF24D648824FF11BE8967162038E2ACBC9@dlee02.ent.ti.com>

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org 
> [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of 
> Woodruff, Richard
> Sent: 2009年6月19日 2:55
> To: Mike Chan; Kevin Hilman
> Cc: linux-omap@vger.kernel.org; Mike Chan
> Subject: RE: [PATCH -pm 1/2] SDRC: check for stuck DLL state 
> machine and kick
> 
> Mike,
> 
> >From: Mike Chan [mailto:mike@android.com]
> >Sent: Thursday, June 18, 2009 1:42 PM
> >
> >
> >+        /* Kick DLL state machine if lock not started */
> >+kick_dll:
> >+       ldr     r4, sdrc_dlla_ctrl      /* get dlla addr */
> >+       ldr     r5, [r4]                /* grab value */
> >+       mov     r6, r5                  /* save value */
> >
> >Richard, could this be done in one instruction, eliminating 
> the need for the r5 >temporary register? Or was this done 
> intentionally?
> 
> Its on purpose I suppose, perhaps there is an optimization.
> 
> Above I want to make sure I restore the value I came in with 
> for dlla_ctrl.  I save off value in r5 for this.
> 
> Below I clear and set dllidle bit.  I don't take the time to 
> figure out if it was set or not coming into function.  I 
> saved off value in r5 to allow for this.  It is legal for it 
> to be set or cleared in normal operation.
> 
> The below set/clear needs to happen as 2 writes.  The 3rd 
> write is a restore.  You could be mindful of input value and 
> perhaps not do a 3rd write but my guess is that will take 
> more code then it saves.
> 
> >+       orr     r6, r6, #0x10           /* dllidle on */
> >+       str     r6, [r4]
> >+       dsb
> >+       bic     r6, #0x10               /* dllidle off */
> >+       str     r6, [r4]
> >+       dsb
> >+       str     r6, [r4]                /* restore old value */

Richard, I think here it should be: str r5, [r4], not r6.
I modifed this when I was verifying your patch.

> >+       b wait_dll_lock_timed
> >+
> 
> Regards,
> Richard W.
> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html