From patchwork Thu Jul 9 23:49:33 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Wang Limei-E12499 X-Patchwork-Id: 34891 X-Patchwork-Delegate: khilman@deeprootsystems.com Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n69NoMuY006270 for ; Thu, 9 Jul 2009 23:50:22 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750732AbZGIXuU (ORCPT ); Thu, 9 Jul 2009 19:50:20 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1750842AbZGIXuU (ORCPT ); Thu, 9 Jul 2009 19:50:20 -0400 Received: from mail128.messagelabs.com ([216.82.250.131]:31515 "EHLO mail128.messagelabs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750732AbZGIXuQ (ORCPT ); Thu, 9 Jul 2009 19:50:16 -0400 X-VirusChecked: Checked X-Env-Sender: E12499@motorola.com X-Msg-Ref: server-2.tower-128.messagelabs.com!1247183413!6795721!1 X-StarScan-Version: 6.0.0; banners=-,-,- X-Originating-IP: [129.188.136.8] Received: (qmail 5815 invoked from network); 9 Jul 2009 23:50:14 -0000 Received: from motgate8.mot.com (HELO motgate8.mot.com) (129.188.136.8) by server-2.tower-128.messagelabs.com with DHE-RSA-AES256-SHA encrypted SMTP; 9 Jul 2009 23:50:14 -0000 Received: from il06exr03.mot.com (il06exr03.mot.com [129.188.137.133]) by motgate8.mot.com (8.14.3/8.14.3) with ESMTP id n69NoDk7002334 for ; Thu, 9 Jul 2009 16:50:13 -0700 (MST) Received: from il06vts01.mot.com (il06vts01.mot.com [129.188.137.141]) by il06exr03.mot.com (8.13.1/Vontu) with SMTP id n69NoCxT009499 for ; Thu, 9 Jul 2009 18:50:12 -0500 (CDT) Received: from zmy16exm62.ds.mot.com (zmy16exm62.ap.mot.com [10.179.4.33]) by il06exr03.mot.com (8.13.1/8.13.0) with ESMTP id n69NoAXq009496 for ; Thu, 9 Jul 2009 18:50:11 -0500 (CDT) X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Subject: RE: [PATCH] OMAP3: PM: Fix freeze when scaling CORE dpll to < 83Mhz Date: Fri, 10 Jul 2009 07:49:33 +0800 Message-ID: In-Reply-To: X-MS-Has-Attach: yes X-MS-TNEF-Correlator: Thread-Topic: [PATCH] OMAP3: PM: Fix freeze when scaling CORE dpll to < 83Mhz thread-index: AcoA1fRJrHMdcwXQTleCxVPTqF5iEAAFMcjA References: <1247134702-12704-1-git-send-email-rnayak@ti.com> From: "Wang Limei-E12499" To: "Paul Walmsley" , "Rajendra Nayak" Cc: , , "Woodruff, Richard" , "Ghongdemath, Girish" X-CFilter-Loop: Reflected Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org Paul, Thank you very much for copying me! During the investigation, Richard and Girish from TI helped, the mail is attached. Rajendra, Yesterday night, I sent mail to paul and kevin, I also submmited below patch along with detailed issue report and analysis. -#define FIXEDDELAY_MASK (0xff << FIXEDDELAY_SHIFT) +#define FIXEDDELAY_MASK 0x00FFFFFF Thanks, limei -----Original Message----- From: Paul Walmsley [mailto:paul@pwsan.com] Sent: Thursday, July 09, 2009 3:44 PM To: Rajendra Nayak Cc: Wang Limei-E12499; khilman@deeprootsystems.com; linux-omap@vger.kernel.org Subject: Re: [PATCH] OMAP3: PM: Fix freeze when scaling CORE dpll to < 83Mhz Hi Rajendra, On Thu, 9 Jul 2009, Rajendra Nayak wrote: > This patch fixes a bug in the CORE dpll scaling sequence which was > errouneously clearing some bits in the SDRC DLLA CTRL register and > hence causing a freeze. > The issue was observed only on platforms which scale CORE dpll to < > 83Mhz and hence program the DLL in fixed delay mode. > > Signed-off-by: Rajendra Nayak Thanks, looks good, I'll queue this up for a fixes series to rmk. One question: I got an E-mail on this earlier today from Limei Wang about this same issue. Looks like we should credit Limei also in this patch with finding the bug. Is this acceptable to you? - Paul > --- > arch/arm/mach-omap2/sram34xx.S | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/arch/arm/mach-omap2/sram34xx.S > b/arch/arm/mach-omap2/sram34xx.S index 481f912..9be09a7 100644 > --- a/arch/arm/mach-omap2/sram34xx.S > +++ b/arch/arm/mach-omap2/sram34xx.S > @@ -113,7 +113,7 @@ return_to_sdram: > unlock_dll: > ldr r11, omap3_sdrc_dlla_ctrl > ldr r12, [r11] > - and r12, r12, #FIXEDDELAY_MASK > + bic r12, r12, #FIXEDDELAY_MASK > orr r12, r12, #FIXEDDELAY_DEFAULT > orr r12, r12, #DLLIDLE_MASK > str r12, [r11] @ (no OCP barrier needed) > -- > 1.5.4.7 > > -- > 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 > Good finding. I’m not sure what motivated Paul to make this change. Perhaps we should inquire on list. He is an open source contractor who Nokia has under contract to make code ‘ready’ for open source. I suspect he felt it was an optimization. I am aware he filed some question to TI about the fixed delay value but not about the enable/disable sequence. It has been my experience that he is quick to make changes with code purity in mind. He does gladly take back bug fixes for those changes. There are many cases that the TI code has arrived at a working sequence which is re-coded in open source ‘clean ups’. Generally the code cleanness does improve but functional regressions are not uncommon. This is part of open source maturity. I’m not sure of answer to sequencing issue you raise. Some research is necessary to find the answer. Maybe Girish or Rajendra know off hand. Regards, Richard W. diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S index 91e3213..860a252 100644 --- a/arch/arm/mach-omap2/sram34xx.S +++ b/arch/arm/mach-omap2/sram34xx.S @@ -41,8 +41,7 @@ #define SDRC_UNLOCK_DLL 0x1 /* SDRC_DLLA_CTRL bit settings */ -#define FIXEDDELAY_SHIFT 24 -#define FIXEDDELAY_MASK (0xff << FIXEDDELAY_SHIFT) +#define FIXEDDELAY_MASK 0x00FFFFFF #define DLLIDLE_MASK 0x4 Richard and Girish, Do you agree with this? Any comments? Thanks, limei ________________________________ From: Wang Limei-E12499 Sent: Wednesday, July 08, 2009 1:45 PM To: Wang Limei-E12499; 'Woodruff, Richard'; 'Hunter, Jon'; 'Ghongdemath, Girish'; 'Nayak, Rajendra'; 'mikechan@google.com' Cc: Sripathi Srinivas-A14759; Wang Sawsd-A24013; Falempe Jocelyn-XHP836; De Chanterac Cyril-cdlc01; WIDZER NOAH-KFQG76; Liu Haiyang-DGRW68 Subject: RE: Mem freeze at omap3_sram_configure_core_dpll in K29 open source PM Girish and Ricard, when unlock dll, clear SDRC_DLLA_CTRL. LOCKDLL , is DLL supposed to be disabled(SDRC_DLLA_CTRL.ENADLL) ? if it is disabled, what will happen? I think when dll is unlocked, DLL should still be enabled, right? thanks, limei ________________________________ From: Wang Limei-E12499 Sent: Tuesday, July 07, 2009 6:59 PM To: Woodruff, Richard; Hunter, Jon; Ghongdemath, Girish; Nayak, Rajendra; mikechan@google.com Cc: Sripathi Srinivas-A14759; Wang Sawsd-A24013; Falempe Jocelyn-XHP836; De Chanterac Cyril-cdlc01; WIDZER NOAH-KFQG76; Liu Haiyang-DGRW68; Wang Limei-E12499 Subject: RE: Mem freeze at omap3_sram_configure_core_dpll in K29 open source PM Ricarhd and Girish, I think you refer to fix_dll_freeze, correct ? It is already in sleep34xx.S. I attached the the sleep34xx.s and sram34xx.s I am using. will you take a close look at them? And I set the breakpoint at the return address of omap3_sram_configure_core_dpll 0xC0047FAC, and found that mem content is corrupted after returning from it. pls see attached snapshot before and after omap3_sram_configure_core_dpll is run. Seems like unlock DLL lead to mem corruption. Again, if does not unlock DLL, no this problem, that ist the only difference between good and bad. Thanks, limei ________________________________ From: Woodruff, Richard [mailto:r-woodruff2@ti.com] Sent: Monday, July 06, 2009 9:33 PM To: Wang Limei-E12499; Hunter, Jon; Ghongdemath, Girish; Nayak, Rajendra; mikechan@google.com Cc: Sripathi Srinivas-A14759; Wang Sawsd-A24013; Falempe Jocelyn-XHP836; De Chanterac Cyril-cdlc01; WIDZER NOAH-KFQG76; Liu Haiyang-DGRW68 Subject: RE: Mem freeze at omap3_sram_configure_core_dpll in K29 open source PM Did you apply my force DLL to lock patch? I think Mike did. I didn’t see it in your code. rkw ________________________________ From: Wang Limei-E12499 [mailto:E12499@motorola.com] Sent: Monday, July 06, 2009 9:23 PM To: Wang Limei-E12499; Woodruff, Richard; Hunter, Jon; Ghongdemath, Girish; Nayak, Rajendra; mikechan@google.com Cc: Sripathi Srinivas-A14759; Wang Sawsd-A24013; Falempe Jocelyn-XHP836; De Chanterac Cyril-cdlc01; WIDZER NOAH-KFQG76; Wang Limei-E12499; Liu Haiyang-DGRW68 Subject: RE: Mem freeze at omap3_sram_configure_core_dpll in K29 open source PM More update about this issue: omap3_sdrc_actim_ctrla,omap3_sdrc_actim_ctrlb,omap3_sdrc_rfr_ctrl are not re-configured when L3 clk is changed from 160.5 to 80.25, the reason is in current sram34xx.c/configure_sdrc, a hack was added to skip ddr timing configure, when hit this function, it returns instead of setting timing registers. Mike, will you expain what is the problem with these code? but even after removing the hack, freeze still happen. when it occur, attach LB, find that dataabt happened. pls see attached snapshot. sdrc iclk is enabled and status is accessed.... Does anybody can check if the register setting is correct after freeze happens? In another two instances when occur happened, attached LB, PC stop at FFFFxxxx,seems around exception hander/vector, pls check the snapshot. Any idea about the data abort and unexpected instructions? BTW, Hacking unlock_dll to zero still works with below patch. Cyril or Noah, Will you pls spend sometime with me tomorrow to debug this issue? Below is the patch I applied. diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S index 91e3213..9831981 100644 --- a/arch/arm/mach-omap2/sram34xx.S +++ b/arch/arm/mach-omap2/sram34xx.S @@ -151,11 +151,15 @@ configure_core_dpll: and r12, r12, r10 orr r12, r12, r3, lsl #CORE_DPLL_CLKOUT_DIV_SHIFT str r12, [r11] - ldr r12, [r11] @ posted-write barrier for CM - bx lr + ldr r12, clk_stabilize_delay @ wait for the clock to stabilise wait_clk_stable: subs r12, r12, #1 bne wait_clk_stable + nop + nop + nop + nop + nop bx lr enable_sdrc: ldr r11, omap3_cm_iclken1_core @@ -187,7 +191,6 @@ wait_dll_unlock: bne wait_dll_unlock bx lr configure_sdrc: - bx lr /* Skip ddr timing configurations, these values are bogus */ ldr r11, omap3_sdrc_rfr_ctrl str r0, [r11] ldr r11, omap3_sdrc_actim_ctrla @@ -221,6 +224,8 @@ omap3_sdrc_dlla_ctrl: .word OMAP34XX_SDRC_REGADDR(SDRC_DLLA_CTRL) core_m2_mask_val: .word 0x07FFFFFF +clk_stabilize_delay: + .word 0x000000C8 ENTRY(omap3_sram_configure_core_dpll_sz) .word . - omap3_sram_configure_core_dpll thanks, limei ________________________________ From: Wang Limei-E12499 Sent: Monday, July 06, 2009 1:09 PM To: Woodruff, Richard; Hunter, Jon; Ghongdemath, Girish; Nayak, Rajendra; mikechan@google.com Cc: Sripathi Srinivas-A14759; Wang Sawsd-A24013; Falempe Jocelyn-XHP836; De Chanterac Cyril-cdlc01; WIDZER NOAH-KFQG76; Wang Limei-E12499 Subject: RE: Mem freeze at omap3_sram_configure_core_dpll in K29 open source PM Richard, Thanks for the info! I already applied the patch before, it did not resolve the problem. Remove the posted-write barrier, still does not work. will you pls check the attached sram34xx.S? what is the delay to program for clk stablization? The clk stable delay is 103 mpu cycles when change freq from 160.5 to 80.25M, it is calculated in clock34xx.c. Also tried 200(0xC8), same thing. /* * XXX This only needs to be done when the CPU frequency changes */ mpurate = arm_fck.rate / CYCLES_PER_MHZ; c = (mpurate << SDRC_MPURATE_SCALE) >> SDRC_MPURATE_BASE_SHIFT; c += 1; /* for safety */ c *= SDRC_MPURATE_LOOPS; c >>= SDRC_MPURATE_SCALE; if (c == 0) c = 1; diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S index 91e3213..db7fef5 100644 --- a/arch/arm/mach-omap2/sram34xx.S +++ b/arch/arm/mach-omap2/sram34xx.S @@ -97,6 +97,8 @@ ENTRY(omap3_sram_configure_core_dpll) blne lock_dll bl sdram_in_selfrefresh @ put SDRAM in self refresh, idle SDRC bl configure_core_dpll @ change the DPLL3 M2 divider + mov r12, r5 + bl wait_clk_stable @ wait for SDRC to stabilize bl enable_sdrc @ take SDRC out of idle cmp r4, #SDRC_UNLOCK_DLL @ wait for DLL status to change bleq wait_dll_unlock @@ -104,8 +106,6 @@ ENTRY(omap3_sram_configure_core_dpll) cmp r7, #1 @ if increasing SDRC clk rate, beq return_to_sdram @ return to SDRAM code, otherwise, bl configure_sdrc @ reprogram SDRC regs now - mov r12, r5 - bl wait_clk_stable @ wait for SDRC to stabilize return_to_sdram: isb @ prevent speculative exec past here mov r0, #0 @ return value @@ -151,7 +151,6 @@ configure_core_dpll: and r12, r12, r10 orr r12, r12, r3, lsl #CORE_DPLL_CLKOUT_DIV_SHIFT str r12, [r11] - ldr r12, [r11] @ posted-write barrier for CM bx lr wait_clk_stable: subs r12, r12, #1