Patchwork [6/6] arm/imx6q: add suspend/resume support

login
register
mail settings
Submitter Shawn Guo
Date Sept. 6, 2011, 9:58 a.m.
Message ID <1315303120-24203-7-git-send-email-shawn.guo@linaro.org>
Download mbox | patch
Permalink /patch/113524/
State New
Headers show

Comments

Shawn Guo - Sept. 6, 2011, 9:58 a.m.
It adds suspend/resume support for imx6q.

Signed-off-by: Anson Huang <b20788@freescale.com>
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 arch/arm/mach-imx/Makefile              |    2 +-
 arch/arm/mach-imx/pm-imx6q.c            |   63 +++++++++++++++++++++++++++++++
 arch/arm/plat-mxc/include/mach/common.h |   12 ++++++
 3 files changed, 76 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/mach-imx/pm-imx6q.c
Arnd Bergmann - Sept. 6, 2011, 6:56 p.m.
On Tuesday 06 September 2011 17:58:40 Shawn Guo wrote:
> It adds suspend/resume support for imx6q.
> 
> Signed-off-by: Anson Huang <b20788@freescale.com>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

Ah, no assembler code in there. Very nice!

	Arnd
Barry Song - Sept. 7, 2011, 1:50 p.m.
> +
> +static int imx6q_pm_enter(suspend_state_t state)
> +{
> +       switch (state) {
> +       case PM_SUSPEND_MEM:
> +               imx_local_timer_pre_suspend();
> +               imx_gpc_pre_suspend();
> +               outer_flush_all();
> +               outer_disable();
> +               imx_set_cpu_jump(0, v7_cpu_resume);
> +
> +               /* Zzz ... */
> +               cpu_suspend(0, imx6q_suspend_finish);
> +
> +               imx_smp_prepare();
> +               l2x0_of_init(0, ~0UL);

it is pretty clear and good. i think we can refine
arch/arm/mach-prima2/pm.c to the way you are suspending/resuming l2.
on prima2, l2 will totally lose power in suspend cycle. i remember you
once said imx6q will retain l2 in suspend cycle?

> +               imx_gpc_post_resume();
> +               imx_local_timer_post_resume();
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
thanks
barry
Shawn Guo - Sept. 8, 2011, 6:23 a.m.
On Wed, Sep 07, 2011 at 09:50:44PM +0800, Barry Song wrote:
> > +
> > +static int imx6q_pm_enter(suspend_state_t state)
> > +{
> > +       switch (state) {
> > +       case PM_SUSPEND_MEM:
> > +               imx_local_timer_pre_suspend();
> > +               imx_gpc_pre_suspend();
> > +               outer_flush_all();
> > +               outer_disable();
> > +               imx_set_cpu_jump(0, v7_cpu_resume);
> > +
> > +               /* Zzz ... */
> > +               cpu_suspend(0, imx6q_suspend_finish);
> > +
> > +               imx_smp_prepare();
> > +               l2x0_of_init(0, ~0UL);
> 
> it is pretty clear and good. i think we can refine
> arch/arm/mach-prima2/pm.c to the way you are suspending/resuming l2.
> on prima2, l2 will totally lose power in suspend cycle. i remember you
> once said imx6q will retain l2 in suspend cycle?
> 
Yes, if you lose L2 power in suspend, it's the easiest way for you to
resume L2.

For my case which retains L2, I actually do not want to call this
function which will invalidate L2.  But since we still have problem
to use rmk's generic suspend/resume updates (ARM: pm: add L2 cache
cleaning for suspend), we have to flush the entire L2 on imx6q for now.

Regards,
Shawn

> > +               imx_gpc_post_resume();
> > +               imx_local_timer_post_resume();
> > +               break;
> > +       default:
> > +               return -EINVAL;
> > +       }
> > +
> > +       return 0;
> > +}
> thanks
> barry
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Russell King - ARM Linux - Sept. 8, 2011, 7:47 a.m.
On Thu, Sep 08, 2011 at 02:23:02PM +0800, Shawn Guo wrote:
> Yes, if you lose L2 power in suspend, it's the easiest way for you to
> resume L2.
> 
> For my case which retains L2, I actually do not want to call this
> function which will invalidate L2.  But since we still have problem
> to use rmk's generic suspend/resume updates (ARM: pm: add L2 cache
> cleaning for suspend), we have to flush the entire L2 on imx6q for now.

OMAP44xx has the same problem but they re-initialize L2 on resume in
their pre-cpu_resume() assembly code.
Shawn Guo - Sept. 8, 2011, 3:22 p.m.
On Thu, Sep 08, 2011 at 08:47:18AM +0100, Russell King - ARM Linux wrote:
> On Thu, Sep 08, 2011 at 02:23:02PM +0800, Shawn Guo wrote:
> > Yes, if you lose L2 power in suspend, it's the easiest way for you to
> > resume L2.
> > 
> > For my case which retains L2, I actually do not want to call this
> > function which will invalidate L2.  But since we still have problem
> > to use rmk's generic suspend/resume updates (ARM: pm: add L2 cache
> > cleaning for suspend), we have to flush the entire L2 on imx6q for now.
> 
> OMAP44xx has the same problem but they re-initialize L2 on resume in
> their pre-cpu_resume() assembly code.
> 
Are you suggesting that this might be the reason why imx6q has problem
with your patch?  Are we supposed to re-initialize L2 before calling
into generic cpu_resume()?
Russell King - ARM Linux - Sept. 8, 2011, 4:24 p.m.
On Thu, Sep 08, 2011 at 11:22:23PM +0800, Shawn Guo wrote:
> On Thu, Sep 08, 2011 at 08:47:18AM +0100, Russell King - ARM Linux wrote:
> > On Thu, Sep 08, 2011 at 02:23:02PM +0800, Shawn Guo wrote:
> > > Yes, if you lose L2 power in suspend, it's the easiest way for you to
> > > resume L2.
> > > 
> > > For my case which retains L2, I actually do not want to call this
> > > function which will invalidate L2.  But since we still have problem
> > > to use rmk's generic suspend/resume updates (ARM: pm: add L2 cache
> > > cleaning for suspend), we have to flush the entire L2 on imx6q for now.
> > 
> > OMAP44xx has the same problem but they re-initialize L2 on resume in
> > their pre-cpu_resume() assembly code.
> > 
> Are you suggesting that this might be the reason why imx6q has problem
> with your patch?

Consider the mechanics of what is happening.

On suspend, when we enter cpu_suspend(), we assume the L2 cache is
still enabled.

We _always_ assume that L1 cache state is lost, so we always flush the
entire L1 cache.

As SoCs can (and do) preserve the L2 contents over suspend/resume cycles,
we leave it to the platform's finisher to decide whether it needs to
flush the entire L2 cache to RAM or not.

If L2 is preserved, then we want to ensure that as much data as possible
is retained in the L2 cache (if the hardware preserves its contents, we
don't want waste time flushing data out of the cache needlessly -
especially if we're using these paths for cpuidle.)  However, we need
access to a certain amount of data to bring the system back up, and as
the L2 cache typically will not be searched before the MMU is enabled,
we have to flush out a certain minimal amount of data (the location
of the stacked restore information and the stacked restore information
itself.)

However, we don't flush out anything else from the L2 cache.

Now, upon resume, the resume code will be able to read the data it needs
to restore the system as we ensured that was flushed out to memory - as
I mentioned above, the L2 cache won't be searched for this irrespective
of whether the control registers have enabled it or not.

We will continue to the point where we hit the first bit of information
stored in L2, which will probably be the stacked SVC register set.  If
at this point we're not able to search the L2 cache, then we'll read
stale data from the backing memory instead, and the system will crash.

Consider what generic code could do about this - if we flushed out that
register set to memory, then we could get past that point - but we then
would need to call a function to enable the L2 cache.  What if that
function needs data which is sitting in the L2 cache to function (eg,
it may need values from the device tree).  We would need to ensure
that that data were also flushed out of the L2 cache.  What about
spinlocks?  Maybe some other CPU has dragged the spinlock data into
the L2 cache.  That gets _much_ harder to solve.

Now to the physical act of enabling the L2 cache.  The L2 cache control
registers are subject to security restrictions when running in non-secure
mode, needing platform specific SMC calls to reprogram the cache.  Generic
code is unable to do this.

Hence why it's left up to the platform to figure out how to enable the
L2 cache before calling cpu_resume().  The platform is best placed to
work out what it needs to do to setup the L2 cache so that the L2 cache
is available by the time the system control register is written, enabling
the MMU and caches.

> Are we supposed to re-initialize L2 before calling
> into generic cpu_resume()?

So, the above is the long way of saying "yes" to this question.  I hope
it gives the full picture about why this is so.
Lorenzo Pieralisi - Sept. 8, 2011, 5:09 p.m.
On Thu, Sep 08, 2011 at 05:24:52PM +0100, Russell King - ARM Linux wrote:
> On Thu, Sep 08, 2011 at 11:22:23PM +0800, Shawn Guo wrote:
> > On Thu, Sep 08, 2011 at 08:47:18AM +0100, Russell King - ARM Linux wrote:
> > > On Thu, Sep 08, 2011 at 02:23:02PM +0800, Shawn Guo wrote:
> > > > Yes, if you lose L2 power in suspend, it's the easiest way for you to
> > > > resume L2.
> > > > 
> > > > For my case which retains L2, I actually do not want to call this
> > > > function which will invalidate L2.  But since we still have problem
> > > > to use rmk's generic suspend/resume updates (ARM: pm: add L2 cache
> > > > cleaning for suspend), we have to flush the entire L2 on imx6q for now.
> > > 
> > > OMAP44xx has the same problem but they re-initialize L2 on resume in
> > > their pre-cpu_resume() assembly code.
> > > 
> > Are you suggesting that this might be the reason why imx6q has problem
> > with your patch?
> 
> Consider the mechanics of what is happening.
> 
> On suspend, when we enter cpu_suspend(), we assume the L2 cache is
> still enabled.
> 
> We _always_ assume that L1 cache state is lost, so we always flush the
> entire L1 cache.
> 
> As SoCs can (and do) preserve the L2 contents over suspend/resume cycles,
> we leave it to the platform's finisher to decide whether it needs to
> flush the entire L2 cache to RAM or not.
> 
> If L2 is preserved, then we want to ensure that as much data as possible
> is retained in the L2 cache (if the hardware preserves its contents, we
> don't want waste time flushing data out of the cache needlessly -
> especially if we're using these paths for cpuidle.)  However, we need
> access to a certain amount of data to bring the system back up, and as
> the L2 cache typically will not be searched before the MMU is enabled,
> we have to flush out a certain minimal amount of data (the location
> of the stacked restore information and the stacked restore information
> itself.)
> 
> However, we don't flush out anything else from the L2 cache.
> 
> Now, upon resume, the resume code will be able to read the data it needs
> to restore the system as we ensured that was flushed out to memory - as
> I mentioned above, the L2 cache won't be searched for this irrespective
> of whether the control registers have enabled it or not.
> 
> We will continue to the point where we hit the first bit of information
> stored in L2, which will probably be the stacked SVC register set.  If
> at this point we're not able to search the L2 cache, then we'll read
> stale data from the backing memory instead, and the system will crash.
> 
> Consider what generic code could do about this - if we flushed out that
> register set to memory, then we could get past that point - but we then
> would need to call a function to enable the L2 cache.  What if that
> function needs data which is sitting in the L2 cache to function (eg,
> it may need values from the device tree).  We would need to ensure
> that that data were also flushed out of the L2 cache.  What about
> spinlocks?  Maybe some other CPU has dragged the spinlock data into
> the L2 cache.  That gets _much_ harder to solve.
> 
> Now to the physical act of enabling the L2 cache.  The L2 cache control
> registers are subject to security restrictions when running in non-secure
> mode, needing platform specific SMC calls to reprogram the cache.  Generic
> code is unable to do this.
> 
> Hence why it's left up to the platform to figure out how to enable the
> L2 cache before calling cpu_resume().  The platform is best placed to
> work out what it needs to do to setup the L2 cache so that the L2 cache
> is available by the time the system control register is written, enabling
> the MMU and caches.
> 
> > Are we supposed to re-initialize L2 before calling
> > into generic cpu_resume()?
> 
> So, the above is the long way of saying "yes" to this question.  I hope
> it gives the full picture about why this is so.
> 

I could not agree more. On top of that l2x0_of_init invalidates L2 which is
not what we want on retention. 
Furthermore, parsing the device tree every given suspend, as fast as it can be,
that seems silly to me for registers that will never ever change (maybe perf
counters, but perf will save them). And if they do change parsing them
from the device tree might not be what we want.

Let's save those registers once for all at boot and reprogramme them before 
jumping to cpu_resume, that's by far the best solution.
Barry Song - Sept. 9, 2011, 6:31 a.m.
2011/9/9 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Thu, Sep 08, 2011 at 11:22:23PM +0800, Shawn Guo wrote:
>> On Thu, Sep 08, 2011 at 08:47:18AM +0100, Russell King - ARM Linux wrote:
>> > On Thu, Sep 08, 2011 at 02:23:02PM +0800, Shawn Guo wrote:
>> > > Yes, if you lose L2 power in suspend, it's the easiest way for you to
>> > > resume L2.
>> > >
>> > > For my case which retains L2, I actually do not want to call this
>> > > function which will invalidate L2.  But since we still have problem
>> > > to use rmk's generic suspend/resume updates (ARM: pm: add L2 cache
>> > > cleaning for suspend), we have to flush the entire L2 on imx6q for now.
>> >
>> > OMAP44xx has the same problem but they re-initialize L2 on resume in
>> > their pre-cpu_resume() assembly code.
>> >
>> Are you suggesting that this might be the reason why imx6q has problem
>> with your patch?
>
> Consider the mechanics of what is happening.
>
> On suspend, when we enter cpu_suspend(), we assume the L2 cache is
> still enabled.
>
> We _always_ assume that L1 cache state is lost, so we always flush the
> entire L1 cache.
>
> As SoCs can (and do) preserve the L2 contents over suspend/resume cycles,
> we leave it to the platform's finisher to decide whether it needs to
> flush the entire L2 cache to RAM or not.
>
> If L2 is preserved, then we want to ensure that as much data as possible
> is retained in the L2 cache (if the hardware preserves its contents, we
> don't want waste time flushing data out of the cache needlessly -
> especially if we're using these paths for cpuidle.)  However, we need
> access to a certain amount of data to bring the system back up, and as
> the L2 cache typically will not be searched before the MMU is enabled,
> we have to flush out a certain minimal amount of data (the location
> of the stacked restore information and the stacked restore information
> itself.)
>
> However, we don't flush out anything else from the L2 cache.
>
> Now, upon resume, the resume code will be able to read the data it needs
> to restore the system as we ensured that was flushed out to memory - as
> I mentioned above, the L2 cache won't be searched for this irrespective
> of whether the control registers have enabled it or not.
>
> We will continue to the point where we hit the first bit of information
> stored in L2, which will probably be the stacked SVC register set.  If
> at this point we're not able to search the L2 cache, then we'll read
> stale data from the backing memory instead, and the system will crash.
>
> Consider what generic code could do about this - if we flushed out that
> register set to memory, then we could get past that point - but we then
> would need to call a function to enable the L2 cache.  What if that
> function needs data which is sitting in the L2 cache to function (eg,
> it may need values from the device tree).  We would need to ensure
> that that data were also flushed out of the L2 cache.  What about
> spinlocks?  Maybe some other CPU has dragged the spinlock data into
> the L2 cache.  That gets _much_ harder to solve.
>
> Now to the physical act of enabling the L2 cache.  The L2 cache control
> registers are subject to security restrictions when running in non-secure
> mode, needing platform specific SMC calls to reprogram the cache.  Generic
> code is unable to do this.
>
> Hence why it's left up to the platform to figure out how to enable the
> L2 cache before calling cpu_resume().  The platform is best placed to
> work out what it needs to do to setup the L2 cache so that the L2 cache
> is available by the time the system control register is written, enabling
> the MMU and caches.

Thanks for your great document to explain what happen. it is really
clear and good.

but in case l2 is not preserved and it is lost in suspend cycles,  we
will flush all data out of l2 before suspending. and we don't need to
re-enable l2
before calling cpu_resume() too since nobody will hit data in l2 not in memory.
after resuming, we just re-init the l2 like cold boot. then
l2x0_of_init() and moving l2x0_init out of init section might be
suitable?

>
>> Are we supposed to re-initialize L2 before calling
>> into generic cpu_resume()?
>
> So, the above is the long way of saying "yes" to this question.  I hope
> it gives the full picture about why this is so.
>

thanks
barry
Shawn Guo - Sept. 9, 2011, 7:32 a.m.
Hi Russell,

Really appreciate the explanation.

On Thu, Sep 08, 2011 at 05:24:52PM +0100, Russell King - ARM Linux wrote:
> On Thu, Sep 08, 2011 at 11:22:23PM +0800, Shawn Guo wrote:
> > On Thu, Sep 08, 2011 at 08:47:18AM +0100, Russell King - ARM Linux wrote:
> > > On Thu, Sep 08, 2011 at 02:23:02PM +0800, Shawn Guo wrote:
> > > > Yes, if you lose L2 power in suspend, it's the easiest way for you to
> > > > resume L2.
> > > > 
> > > > For my case which retains L2, I actually do not want to call this
> > > > function which will invalidate L2.  But since we still have problem
> > > > to use rmk's generic suspend/resume updates (ARM: pm: add L2 cache
> > > > cleaning for suspend), we have to flush the entire L2 on imx6q for now.
> > > 
> > > OMAP44xx has the same problem but they re-initialize L2 on resume in
> > > their pre-cpu_resume() assembly code.
> > > 
> > Are you suggesting that this might be the reason why imx6q has problem
> > with your patch?
> 
> Consider the mechanics of what is happening.
> 
> On suspend, when we enter cpu_suspend(), we assume the L2 cache is
> still enabled.
> 
> We _always_ assume that L1 cache state is lost, so we always flush the
> entire L1 cache.
> 
> As SoCs can (and do) preserve the L2 contents over suspend/resume cycles,
> we leave it to the platform's finisher to decide whether it needs to
> flush the entire L2 cache to RAM or not.
> 
> If L2 is preserved, then we want to ensure that as much data as possible
> is retained in the L2 cache (if the hardware preserves its contents, we
> don't want waste time flushing data out of the cache needlessly -
> especially if we're using these paths for cpuidle.)  However, we need
> access to a certain amount of data to bring the system back up, and as
> the L2 cache typically will not be searched before the MMU is enabled,
> we have to flush out a certain minimal amount of data (the location
> of the stacked restore information and the stacked restore information
> itself.)
> 
> However, we don't flush out anything else from the L2 cache.
> 
> Now, upon resume, the resume code will be able to read the data it needs
> to restore the system as we ensured that was flushed out to memory - as
> I mentioned above, the L2 cache won't be searched for this irrespective
> of whether the control registers have enabled it or not.
> 
> We will continue to the point where we hit the first bit of information
> stored in L2, which will probably be the stacked SVC register set.  If
> at this point we're not able to search the L2 cache, then we'll read
> stale data from the backing memory instead, and the system will crash.
> 
> Consider what generic code could do about this - if we flushed out that
> register set to memory, then we could get past that point - but we then
> would need to call a function to enable the L2 cache.  What if that
> function needs data which is sitting in the L2 cache to function (eg,
> it may need values from the device tree).  We would need to ensure
> that that data were also flushed out of the L2 cache.  What about
> spinlocks?  Maybe some other CPU has dragged the spinlock data into
> the L2 cache.  That gets _much_ harder to solve.
> 
> Now to the physical act of enabling the L2 cache.  The L2 cache control
> registers are subject to security restrictions when running in non-secure
> mode, needing platform specific SMC calls to reprogram the cache.  Generic
> code is unable to do this.
> 
I could be very possibly wrong here.  But isn't the core in secure mode
upon reset?  Do we really have to reprogram L2 through SMC calls there?

> Hence why it's left up to the platform to figure out how to enable the
> L2 cache before calling cpu_resume().  The platform is best placed to
> work out what it needs to do to setup the L2 cache so that the L2 cache
> is available by the time the system control register is written, enabling
> the MMU and caches.
> 
> > Are we supposed to re-initialize L2 before calling
> > into generic cpu_resume()?
> 
> So, the above is the long way of saying "yes" to this question.  I hope
> it gives the full picture about why this is so.
> 
Yes, it does.  Thanks, again.
Shawn Guo - Sept. 9, 2011, 7:40 a.m.
On Thu, Sep 08, 2011 at 06:09:02PM +0100, Lorenzo Pieralisi wrote:
> On Thu, Sep 08, 2011 at 05:24:52PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Sep 08, 2011 at 11:22:23PM +0800, Shawn Guo wrote:
> > > On Thu, Sep 08, 2011 at 08:47:18AM +0100, Russell King - ARM Linux wrote:
> > > > On Thu, Sep 08, 2011 at 02:23:02PM +0800, Shawn Guo wrote:
> > > > > Yes, if you lose L2 power in suspend, it's the easiest way for you to
> > > > > resume L2.
> > > > > 
> > > > > For my case which retains L2, I actually do not want to call this
> > > > > function which will invalidate L2.  But since we still have problem
> > > > > to use rmk's generic suspend/resume updates (ARM: pm: add L2 cache
> > > > > cleaning for suspend), we have to flush the entire L2 on imx6q for now.
> > > > 
> > > > OMAP44xx has the same problem but they re-initialize L2 on resume in
> > > > their pre-cpu_resume() assembly code.
> > > > 
> > > Are you suggesting that this might be the reason why imx6q has problem
> > > with your patch?
> > 
> > Consider the mechanics of what is happening.
> > 
> > On suspend, when we enter cpu_suspend(), we assume the L2 cache is
> > still enabled.
> > 
> > We _always_ assume that L1 cache state is lost, so we always flush the
> > entire L1 cache.
> > 
> > As SoCs can (and do) preserve the L2 contents over suspend/resume cycles,
> > we leave it to the platform's finisher to decide whether it needs to
> > flush the entire L2 cache to RAM or not.
> > 
> > If L2 is preserved, then we want to ensure that as much data as possible
> > is retained in the L2 cache (if the hardware preserves its contents, we
> > don't want waste time flushing data out of the cache needlessly -
> > especially if we're using these paths for cpuidle.)  However, we need
> > access to a certain amount of data to bring the system back up, and as
> > the L2 cache typically will not be searched before the MMU is enabled,
> > we have to flush out a certain minimal amount of data (the location
> > of the stacked restore information and the stacked restore information
> > itself.)
> > 
> > However, we don't flush out anything else from the L2 cache.
> > 
> > Now, upon resume, the resume code will be able to read the data it needs
> > to restore the system as we ensured that was flushed out to memory - as
> > I mentioned above, the L2 cache won't be searched for this irrespective
> > of whether the control registers have enabled it or not.
> > 
> > We will continue to the point where we hit the first bit of information
> > stored in L2, which will probably be the stacked SVC register set.  If
> > at this point we're not able to search the L2 cache, then we'll read
> > stale data from the backing memory instead, and the system will crash.
> > 
> > Consider what generic code could do about this - if we flushed out that
> > register set to memory, then we could get past that point - but we then
> > would need to call a function to enable the L2 cache.  What if that
> > function needs data which is sitting in the L2 cache to function (eg,
> > it may need values from the device tree).  We would need to ensure
> > that that data were also flushed out of the L2 cache.  What about
> > spinlocks?  Maybe some other CPU has dragged the spinlock data into
> > the L2 cache.  That gets _much_ harder to solve.
> > 
> > Now to the physical act of enabling the L2 cache.  The L2 cache control
> > registers are subject to security restrictions when running in non-secure
> > mode, needing platform specific SMC calls to reprogram the cache.  Generic
> > code is unable to do this.
> > 
> > Hence why it's left up to the platform to figure out how to enable the
> > L2 cache before calling cpu_resume().  The platform is best placed to
> > work out what it needs to do to setup the L2 cache so that the L2 cache
> > is available by the time the system control register is written, enabling
> > the MMU and caches.
> > 
> > > Are we supposed to re-initialize L2 before calling
> > > into generic cpu_resume()?
> > 
> > So, the above is the long way of saying "yes" to this question.  I hope
> > it gives the full picture about why this is so.
> > 
> 
> I could not agree more. On top of that l2x0_of_init invalidates L2 which is
> not what we want on retention. 
> Furthermore, parsing the device tree every given suspend, as fast as it can be,
> that seems silly to me for registers that will never ever change (maybe perf
> counters, but perf will save them). And if they do change parsing them
> from the device tree might not be what we want.
> 
> Let's save those registers once for all at boot and reprogramme them before 
> jumping to cpu_resume, that's by far the best solution.
> 
For cases like imx6q where L2 content will be retained across the
suspend/resume cycle, I agree that this is the best.  I used
l2x0_of_init() approach for imx6q as a temp workaround, because I
have something to sort out with applying rmk's patches.

However, I still think l2x0_of_init() reinitialization is the best
approach for cases like Prima2 where even L2 content gets lost during
suspend/resume cycle.
Russell King - ARM Linux - Sept. 9, 2011, 8:15 a.m.
On Fri, Sep 09, 2011 at 03:32:48PM +0800, Shawn Guo wrote:
> > Now to the physical act of enabling the L2 cache.  The L2 cache control
> > registers are subject to security restrictions when running in non-secure
> > mode, needing platform specific SMC calls to reprogram the cache.  Generic
> > code is unable to do this.
>
> I could be very possibly wrong here.  But isn't the core in secure mode
> upon reset?  Do we really have to reprogram L2 through SMC calls there?

It probably will be, but the core won't be running the resume function
directly on reset.  (No ARM CPU does this - not even the pre-security
ones.  They've traditionally run the boot loader first.)

Take a moment to think about this in terms of security of data stored on
the secure side (where there maybe private keys stored.)

The kernel runs in the non-secure side of the partition.  If it were to be
entered on resume in secure mode, it could access those keys.  That would
be a really great security hole, which would make the whole thing pointless.

What would happen is the secure monitor ROM code gets control first, just
like it does at normal boot time on secure devices.  Eventually, the kernel
will gain control but only after the secure monitor has switched to
non-secure mode.
Shawn Guo - Sept. 9, 2011, 10:15 a.m.
On Fri, Sep 09, 2011 at 09:15:20AM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 09, 2011 at 03:32:48PM +0800, Shawn Guo wrote:
> > > Now to the physical act of enabling the L2 cache.  The L2 cache control
> > > registers are subject to security restrictions when running in non-secure
> > > mode, needing platform specific SMC calls to reprogram the cache.  Generic
> > > code is unable to do this.
> >
> > I could be very possibly wrong here.  But isn't the core in secure mode
> > upon reset?  Do we really have to reprogram L2 through SMC calls there?
> 
> It probably will be, but the core won't be running the resume function
> directly on reset.  (No ARM CPU does this - not even the pre-security
> ones.  They've traditionally run the boot loader first.)
> 
Some naive boot ROM may simply jump to the resume entry address saved
in persistent register.

> Take a moment to think about this in terms of security of data stored on
> the secure side (where there maybe private keys stored.)
> 
> The kernel runs in the non-secure side of the partition.  If it were to be
> entered on resume in secure mode, it could access those keys.  That would
> be a really great security hole, which would make the whole thing pointless.
> 
> What would happen is the secure monitor ROM code gets control first, just
> like it does at normal boot time on secure devices.  Eventually, the kernel
> will gain control but only after the secure monitor has switched to
> non-secure mode.
> 
Then the question is that for given bad boot ROM, whether kernel resume
routine should stand up to fill the hole up.
Russell King - ARM Linux - Sept. 9, 2011, 6:47 p.m.
On Fri, Sep 09, 2011 at 06:15:10PM +0800, Shawn Guo wrote:
> On Fri, Sep 09, 2011 at 09:15:20AM +0100, Russell King - ARM Linux wrote:
> > On Fri, Sep 09, 2011 at 03:32:48PM +0800, Shawn Guo wrote:
> > > > Now to the physical act of enabling the L2 cache.  The L2 cache control
> > > > registers are subject to security restrictions when running in non-secure
> > > > mode, needing platform specific SMC calls to reprogram the cache.  Generic
> > > > code is unable to do this.
> > >
> > > I could be very possibly wrong here.  But isn't the core in secure mode
> > > upon reset?  Do we really have to reprogram L2 through SMC calls there?
> > 
> > It probably will be, but the core won't be running the resume function
> > directly on reset.  (No ARM CPU does this - not even the pre-security
> > ones.  They've traditionally run the boot loader first.)
> 
> Some naive boot ROM may simply jump to the resume entry address saved
> in persistent register.

That would mean that the platform code needs to deal with this too - and
it's likely that it too is platform specific.

> Then the question is that for given bad boot ROM, whether kernel resume
> routine should stand up to fill the hole up.

That can only be done with knowledge of the platform (eg, where the
secure mode vectors should be pointed) and so can't be dealt with by
generic code.

I would also imagine that it's also far easier to sort out before we
enable the MMU, rather than dealing with the aftermath of having pulled
secure mode entries from the non-secure world into the caches.

Patch

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index 84bd18c..531da76 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -70,5 +70,5 @@  obj-$(CONFIG_CPU_V7) += head-v7.o
 obj-$(CONFIG_SMP) += platsmp.o
 obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
 obj-$(CONFIG_LOCAL_TIMERS) += localtimer.o
-obj-$(CONFIG_SOC_IMX6Q) += clock-imx6q.o
+obj-$(CONFIG_SOC_IMX6Q) += clock-imx6q.o pm-imx6q.o
 obj-$(CONFIG_MACH_IMX6Q) += mach-imx6q.o
diff --git a/arch/arm/mach-imx/pm-imx6q.c b/arch/arm/mach-imx/pm-imx6q.c
new file mode 100644
index 0000000..cbbc3c8
--- /dev/null
+++ b/arch/arm/mach-imx/pm-imx6q.c
@@ -0,0 +1,63 @@ 
+/*
+ * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011 Linaro Ltd.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/suspend.h>
+#include <asm/proc-fns.h>
+#include <asm/suspend.h>
+#include <asm/hardware/cache-l2x0.h>
+#include <asm/hardware/gic.h>
+#include <mach/common.h>
+
+static int imx6q_suspend_finish(unsigned long val)
+{
+	cpu_do_idle();
+	return 0;
+}
+
+static int imx6q_pm_enter(suspend_state_t state)
+{
+	switch (state) {
+	case PM_SUSPEND_MEM:
+		imx_local_timer_pre_suspend();
+		imx_gpc_pre_suspend();
+		outer_flush_all();
+		outer_disable();
+		imx_set_cpu_jump(0, v7_cpu_resume);
+
+		/* Zzz ... */
+		cpu_suspend(0, imx6q_suspend_finish);
+
+		imx_smp_prepare();
+		l2x0_of_init(0, ~0UL);
+		imx_gpc_post_resume();
+		imx_local_timer_post_resume();
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct platform_suspend_ops imx6q_pm_ops = {
+	.enter = imx6q_pm_enter,
+	.valid = suspend_valid_only_mem,
+};
+
+static int __init imx6q_pm_init(void)
+{
+	suspend_set_ops(&imx6q_pm_ops);
+	return 0;
+}
+late_initcall(imx6q_pm_init);
diff --git a/arch/arm/plat-mxc/include/mach/common.h b/arch/arm/plat-mxc/include/mach/common.h
index ff86063..7c3d078 100644
--- a/arch/arm/plat-mxc/include/mach/common.h
+++ b/arch/arm/plat-mxc/include/mach/common.h
@@ -80,12 +80,24 @@  extern void imx_lluart_map_io(void);
 #else
 static inline void imx_lluart_map_io(void) {}
 #endif
+extern void v7_cpu_resume(void);
 #ifdef CONFIG_SMP
 extern void imx_scu_map_io(void);
+extern void imx_smp_prepare(void);
 #else
 static inline void imx_scu_map_io(void) {}
+static inline void imx_smp_prepare(void) {}
 #endif
 extern void imx_enable_cpu(int cpu, bool enable);
 extern void imx_set_cpu_jump(int cpu, void *jump_addr);
 extern void imx_gpc_init(void);
+extern void imx_gpc_pre_suspend(void);
+extern void imx_gpc_post_resume(void);
+#ifdef CONFIG_LOCAL_TIMERS
+extern void imx_local_timer_pre_suspend(void);
+extern void imx_local_timer_post_resume(void);
+#else
+static inline void imx_local_timer_pre_suspend(void) {}
+static inline void imx_local_timer_post_resume(void) {}
+#endif
 #endif