Re: [PATCH] arm: imx: suspend/resume: use outer_disable/resume

[Dong Aisheng]

On Tue, Dec 26, 2017 at 09:49:01AM +0000, Peng Fan wrote:
> Hi Shawn
> 
> > -----Original Message-----
> > From: Shawn Guo [mailto:shawnguo@xxxxxxxxxx]
> > Sent: Tuesday, December 26, 2017 5:13 PM
> > To: Peng Fan <peng.fan@xxxxxxx>
> > Cc: A.s. Dong <aisheng.dong@xxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; Russell
> > King <linux@xxxxxxxxxxxxxxx>; Fabio Estevam <fabio.estevam@xxxxxxx>;
> > Sascha Hauer <kernel@xxxxxxxxxxxxxx>; van.freenix@xxxxxxxxx; linux-arm-
> > kernel@xxxxxxxxxxxxxxxxxxx
> > Subject: Re: [PATCH] arm: imx: suspend/resume: use outer_disable/resume
> > 
> > On Sun, Dec 10, 2017 at 08:07:18PM +0800, Peng Fan wrote:
> > > Use outer_disable/resume for suspend/resume.
> > > With the two APIs used, code could be simplified and easy to extend to
> > > introduce l2c_write_sec for i.MX platforms when moving Linux Kernel
> > > runs in non-secure world.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> > > Cc: Shawn Guo <shawnguo@xxxxxxxxxx>
> > > Cc: Sascha Hauer <kernel@xxxxxxxxxxxxxx>
> > > Cc: Fabio Estevam <fabio.estevam@xxxxxxx>
> > > Cc: Russell King <linux@xxxxxxxxxxxxxxx>
> > > Cc: Dong Aisheng <aisheng.dong@xxxxxxx>
> > 
> > Changed 'arm: ' prefix to 'ARM: ', and applied patch.
> 
> I just tested it on 6sx-sdb, seems this patch breaks 6sx. Could you first drop this
> patch? I'll send out v2 fix the 6sx issue soon.
> 

Yes, i tested mx6ul/mx6sl ok but mx6sx sdb failed.

After a few debug, it seems you removed the l2c310_early_resume in v7_cpu_resume
which is shared between lower power idle and suspend.

Current only mx6sx/mx6ul supports low power idle in upstream, but mx6ul is A7 with
internal L2, that's probably why only mx6sx showed the issue.

I did the following quick try to restore L2 after exit lower power idle, but still
can meet occasional crash during booting.

diff --git a/arch/arm/mach-imx/cpuidle-imx6sx.c b/arch/arm/mach-imx/cpuidle-imx6sx.c
index c5a5c3a..edce5bd 100644
--- a/arch/arm/mach-imx/cpuidle-imx6sx.c
+++ b/arch/arm/mach-imx/cpuidle-imx6sx.c
@@ -26,7 +26,7 @@ static int imx6sx_idle_finish(unsigned long val)
         * to adding conditional code for L2 cache type,
         * just call flush_cache_all() is fine.
         */
-       flush_cache_all();
+//     flush_cache_all();
        cpu_do_idle();
 
        return 0;
@@ -49,7 +49,9 @@ static int imx6sx_enter_wait(struct cpuidle_device *dev,
                cpu_pm_enter();
                cpu_cluster_pm_enter();
 
+               outer_disable();
                cpu_suspend(0, imx6sx_idle_finish);
+               outer_resume();
 
                cpu_cluster_pm_exit();
                cpu_pm_exit();

As this changed the order to L2 restore and cpu resume, so i'm not quite
sure if lower power idle still requres L2 restore before CPU resume or
something else we're missing.

Regards
Dong Aisheng

> Thanks,
> Peng.
> 
> > 
> > Shawn