From patchwork Mon Apr 11 23:16:16 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Gerlach X-Patchwork-Id: 8806151 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id CF9089F36E for ; Mon, 11 Apr 2016 23:18:22 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BECBE201EF for ; Mon, 11 Apr 2016 23:18:21 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9C2D2201CD for ; Mon, 11 Apr 2016 23:18:19 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1apl4U-00063Q-HN; Mon, 11 Apr 2016 23:16:42 +0000 Received: from arroyo.ext.ti.com ([192.94.94.40]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1apl4R-0005x9-GM for linux-arm-kernel@lists.infradead.org; Mon, 11 Apr 2016 23:16:40 +0000 Received: from dflxv15.itg.ti.com ([128.247.5.124]) by arroyo.ext.ti.com (8.13.7/8.13.7) with ESMTP id u3BNGEvL027651; Mon, 11 Apr 2016 18:16:15 -0500 Received: from DLEE70.ent.ti.com (dlemailx.itg.ti.com [157.170.170.113]) by dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id u3BNGEE0020032; Mon, 11 Apr 2016 18:16:14 -0500 Received: from dlep33.itg.ti.com (157.170.170.75) by DLEE70.ent.ti.com (157.170.170.113) with Microsoft SMTP Server id 14.3.224.2; Mon, 11 Apr 2016 18:16:14 -0500 Received: from [192.168.110.131] (ileax41-snat.itg.ti.com [10.172.224.153]) by dlep33.itg.ti.com (8.14.3/8.13.8) with ESMTP id u3BNGEZD002581; Mon, 11 Apr 2016 18:16:14 -0500 Message-ID: <570C3040.3030001@ti.com> Date: Mon, 11 Apr 2016 18:16:16 -0500 From: Dave Gerlach User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Tony Lindgren Subject: Re: [PATCH] ARM: OMAP3: Fix imprecise external abort for off mode on 36xx References: <1455140107-3328-1-git-send-email-tony@atomide.com> <5706C062.4040803@ti.com> <20160407231604.GP16484@atomide.com> <570BEAFB.6000409@ti.com> <20160411211339.GI5995@atomide.com> In-Reply-To: <20160411211339.GI5995@atomide.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160411_161639_673105_ACAE8A3F X-CRM114-Status: GOOD ( 21.85 ) X-Spam-Score: -7.9 (-------) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Nishanth Menon , Grygorii Strashko , Tero Kristo , Richard Woodruff , linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 04/11/2016 04:13 PM, Tony Lindgren wrote: > * Dave Gerlach [160411 11:21]: >> Tony, >> On 04/07/2016 06:16 PM, Tony Lindgren wrote: >>> * Dave Gerlach [160407 13:18]: >>>> >>>> I have a series to convert omap3 PM code to using generic SRAM driver but >>>> when testing I see an external abort on BBXM off mode resume very similar to >>>> this that seems to be timing related caused by using generic SRAM driver to >>>> re-copy PM code rather than omap3_sram_restore_context. >>>> >>>> By tracing the resume path I believe the abort is caused by >>>> omap3_intc_resume_idle in pm34xx.c, which writes to two registers in the >>>> INTC. Removing this call makes the abort unreproducible in my experiments >>>> and changing the writes to reads causes a bus lock, so I'm pretty confident >>>> it's coming from this call attempting to write to an idled INTC. >>>> >>>> Advisory 1.106 in DM3730 Silicon Errata Document [1] describes an issue with >>>> "MPU Leaves MSTANDBY State Before IDLEREQ of Interrupt Controller is >>>> Released" which sounds like a very similar failure situation to what we are >>>> seeing even though the timing of INTC access is a bit further from WFI exit >>>> than it describes. Perhaps there are more conditions where it can occur. >>>> Pushed my WIP branch for SRAM series that shows the same failure here [2]. >>> >>> Interesting, I think you may have something with the errata 1.106.. And >>> looks like we also need still errata 1.62 handled also on 36xx in the >>> same pdf. And need to disable intc autoidle early at least for HS omaps >>> as save_secure_ram context supposedly also can do WFI. >>> >>> Maybe something like the following would make sense? It seems to work >>> here without external aborts with your test branch on dm3730, and boots >>> fine on omap3430 hs (n900). >>> >>> Or do you have some better ideas for a fix? >> >> I can also confirm that this fixes the external abort, I can not cause it >> with your attached patch. > > OK. I still can't quite see why exactly this patch fixes things. So > I'm afraid it might be just hiding the problem.. I agree, moving the omap3_intc_resume_idle may just be masking the issue for that exact situation, but I think we can get rid of it for off mode... > >> I would be ok with the solution you have proposed and I was unable to come >> up with anything better while trying to debug the issue originally. >> >> We still need the call to omap3_intc_resume_idle because the intc restore >> context only gets called on resume from off mode. Perhaps we only need to >> call omap3_intc_resume_idle when coming back from non-off modes, otherwise >> let the context restore handle the reconfig of the INTC idle/sysconfig >> registers? > > OK. Did you actually test by commenting out omap3_intc_resume_idle()? > > Yeah sounds like we can optimize out the restore there for non-off > modes. Yes I removed it entirely for testing, and I also tried something like this for a possible workable solution (without your patch applied): Regards, Dave > > Tony > diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c index fcf975eb5e9d..8d39b44ba3a3 100644 --- a/arch/arm/mach-omap2/pm34xx.c +++ b/arch/arm/mach-omap2/pm34xx.c @@ -268,7 +268,6 @@ void omap_sram_idle(void) int per_next_state = PWRDM_POWER_ON; int core_next_state = PWRDM_POWER_ON; int per_going_off; - int core_prev_state; u32 sdrc_pwr = 0; mpu_next_state = pwrdm_read_next_pwrst(mpu_pwrdm); @@ -348,17 +347,16 @@ void omap_sram_idle(void) sdrc_write_reg(sdrc_pwr, SDRC_POWER); /* CORE */ - if (core_next_state < PWRDM_POWER_ON) { - core_prev_state = pwrdm_read_prev_pwrst(core_pwrdm); - if (core_prev_state == PWRDM_POWER_OFF) { + if (core_next_state < PWRDM_POWER_ON && + pwrdm_read_prev_pwrst(core_pwrdm) == PWRDM_POWER_OFF) { omap3_core_restore_context(); omap3_cm_restore_context(); omap3_push_sram_idle(); omap3_push_sram_secure_idle(); omap2_sms_restore_context(); - } - } - omap3_intc_resume_idle(); + } else + omap3_intc_resume_idle(); + pwrdm_post_transition(NULL);