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: 8806141 Return-Path: X-Original-To: patchwork-linux-omap@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 89F64C0553 for ; Mon, 11 Apr 2016 23:16:46 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9E9B920219 for ; Mon, 11 Apr 2016 23:16:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5ECC8201B9 for ; Mon, 11 Apr 2016 23:16:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752777AbcDKXQm (ORCPT ); Mon, 11 Apr 2016 19:16:42 -0400 Received: from arroyo.ext.ti.com ([192.94.94.40]:34802 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751615AbcDKXQm (ORCPT ); Mon, 11 Apr 2016 19:16:42 -0400 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 CC: , , Grygorii Strashko , Nishanth Menon , Richard Woodruff , Tero Kristo 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> Sender: linux-omap-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org X-Spam-Status: No, score=-7.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, 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 > --- 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 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);