From: d-gerlach@ti•com (Dave Gerlach)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH] ARM: OMAP3: Fix imprecise external abort for off mode on 36xx
Date: Mon, 11 Apr 2016 13:20:43 -0500 [thread overview]
Message-ID: <570BEAFB.6000409@ti.com> (raw)
In-Reply-To: <20160407231604.GP16484@atomide.com>
Tony,
On 04/07/2016 06:16 PM, Tony Lindgren wrote:
> * Dave Gerlach <d-gerlach@ti•com> [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.
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?
Regards,
Dave
>
> Regards,
>
> Tony
>
> 8< ------------------------
> From: Tony Lindgren <tony@atomide•com>
> Date: Thu, 7 Apr 2016 16:06:35 -0700
> Subject: [PATCH] ARM: OMAP3: Fix external abort on 36xx waking from off mode
> idle
>
> Depending on the timings affected by .config options, we may get
> external aborts with 36xx. These seem to be caused by the following:
>
> - omap3 errata 1.62 "MPU Cannot Exit from Standby" says we need to
> disable disable intc autoidle before WFI
>
> - dm3730 errata 1.106 "MPU Leaves MSTANDBY State Before IDLEREQ of
> Interrupt Controller is Released" says we need to wait before
> accessing intc
>
> Looks like we're currently disabling intc autoidle after we save
> the intc context. And then we attempt to re-enable intc autoidle
> after we restore intc context.
>
> This means we're trying to restore intc autoidle twice, and it
> occasionally fails as the intc restore may take a while for
> things to get configured?
>
> So we should either completely leave out omap3_intc_resume_idle()
> assuming that the context restore really also enables the intc
> autoidle.
>
> Or we can disable intc autoidle before saving context, and then
> reenable intc autoidle only after the context restore. This pairs
> the calls, can we can call omap3_intc_resume_idle() later to
> allow some time for intc to settle down.
>
> Note that supposedly save_secure_ram() also can cause a WFI
> in the secure ram code, so we need to disable intc autoidle
> early. There's some information about that in the old thread
> "[PATCH 09/13] OMAP3: PM: Apply errata i540 before save
> secure ram".
>
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -303,6 +303,8 @@ void omap_sram_idle(void)
> omap2_gpio_prepare_for_idle(per_going_off);
> }
>
> + omap3_intc_prepare_idle();
> +
> /* CORE */
> if (core_next_state < PWRDM_POWER_ON) {
> if (core_next_state == PWRDM_POWER_OFF) {
> @@ -314,8 +316,6 @@ void omap_sram_idle(void)
> /* Configure PMIC signaling for I2C4 or sys_off_mode */
> omap3_vc_set_pmic_signaling(core_next_state);
>
> - omap3_intc_prepare_idle();
> -
> /*
> * On EMU/HS devices ROM code restores a SRDC value
> * from scratchpad which has automatic self refresh on timeout
> @@ -358,13 +358,14 @@ void omap_sram_idle(void)
> omap2_sms_restore_context();
> }
> }
> - omap3_intc_resume_idle();
>
> pwrdm_post_transition(NULL);
>
> /* PER */
> if (per_next_state < PWRDM_POWER_ON)
> omap2_gpio_resume_after_idle();
> +
> + omap3_intc_resume_idle();
> }
>
> static void omap3_pm_idle(void)
>
next prev parent reply other threads:[~2016-04-11 18:20 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-10 21:35 [PATCH] ARM: OMAP3: Fix imprecise external abort for off mode on 36xx Tony Lindgren
2016-04-07 20:17 ` Dave Gerlach
2016-04-07 23:16 ` Tony Lindgren
2016-04-11 18:20 ` Dave Gerlach [this message]
2016-04-11 21:13 ` Tony Lindgren
2016-04-11 23:16 ` Dave Gerlach
2016-04-12 0:01 ` Tony Lindgren
2016-04-13 15:45 ` Dave Gerlach
2016-04-13 15:50 ` Tony Lindgren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=570BEAFB.6000409@ti.com \
--to=d-gerlach@ti$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox