[PATCH v2 6/6] arm/imx6q: add suspend/resume support
Shawn Guo
shawn.guo at freescale.com
Fri Sep 16 02:09:00 EDT 2011
Hi Lorenzo,
On Thu, Sep 15, 2011 at 05:28:29PM +0100, Lorenzo Pieralisi wrote:
> On Thu, Sep 15, 2011 at 03:45:26PM +0100, Shawn Guo wrote:
> > It adds suspend/resume support for imx6q.
> >
> > Signed-off-by: Anson Huang <b20788 at freescale.com>
> > Signed-off-by: Shawn Guo <shawn.guo at linaro.org>
> > ---
> > arch/arm/mach-imx/Makefile | 2 +-
> > arch/arm/mach-imx/head-v7.S | 27 +++++++++
> > arch/arm/mach-imx/pm-imx6q.c | 88 +++++++++++++++++++++++++++++++
> > arch/arm/plat-mxc/include/mach/common.h | 8 +++
> > 4 files changed, 124 insertions(+), 1 deletions(-)
> > create mode 100644 arch/arm/mach-imx/pm-imx6q.c
> >
> > diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> > index 16737ba..c787151 100644
> > --- a/arch/arm/mach-imx/Makefile
> > +++ b/arch/arm/mach-imx/Makefile
> > @@ -70,4 +70,4 @@ 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 mach-imx6q.o
> > +obj-$(CONFIG_SOC_IMX6Q) += clock-imx6q.o mach-imx6q.o pm-imx6q.o
> > diff --git a/arch/arm/mach-imx/head-v7.S b/arch/arm/mach-imx/head-v7.S
> > index ede908b..0a86685 100644
> > --- a/arch/arm/mach-imx/head-v7.S
> > +++ b/arch/arm/mach-imx/head-v7.S
> > @@ -69,3 +69,30 @@ ENTRY(v7_secondary_startup)
> > b secondary_startup
> > ENDPROC(v7_secondary_startup)
> > #endif
> > +
> > +ENTRY(v7_cpu_resume)
> > + bl v7_invalidate_l1
> > +
> > + /*
> > + * Restore L2 AUX_CTRL register saved by suspend procedure
> > + * and enable L2
> > + */
> > + adr r4, 1f
> > + ldmia r4, {r5, r6, r7}
> > + sub r4, r4, r5
> > + add r6, r6, r4
> > + add r7, r7, r4
> > + ldr r0, [r6]
> > + ldr r7, [r7]
> > + ldr r1, [r7]
> > + str r1, [r0, #L2X0_AUX_CTRL]
> > + ldr r1, =0x1
> > + str r1, [r0, #L2X0_CTRL]
> > +
> > + b cpu_resume
> > +
> > + .align
> > +1: .long .
> > + .long pl310_pbase
> > + .long pl310_aux_ctrl_paddr
>
> Would not something like:
>
> adr r4, pl310_pbase
> ldmia r4, {r6, r7}
> [...]
>
> pl310_pbase:
> .long 0
> pl310_aux_ctrl:
> .long 0
>
> be better and faster ? Why play with virtual addresses ?
> Of course you should initialize the values, but then you can access them
> through a PC relative load when running physical.
Thanks for the comment. I agree with you that it's better, though my
first thought on the existing approach is I can access the global
variables defined in C file directly from assembly code.
> Your code should be in the .data section for it to be writable (adr does not
> work across sections), have a look at Russell's code in sleep.S it is
> very well commented and similar to what you need.
Thanks for pointing me the example.
>
> > +ENDPROC(v7_cpu_resume)
> > diff --git a/arch/arm/mach-imx/pm-imx6q.c b/arch/arm/mach-imx/pm-imx6q.c
> > new file mode 100644
> > index 0000000..124bcd5
> > --- /dev/null
> > +++ b/arch/arm/mach-imx/pm-imx6q.c
> > @@ -0,0 +1,88 @@
> > +/*
> > + * 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/of.h>
> > +#include <linux/suspend.h>
> > +#include <asm/proc-fns.h>
> > +#include <asm/suspend.h>
> > +#include <asm/hardware/cache-l2x0.h>
> > +#include <mach/common.h>
> > +#include <mach/hardware.h>
> > +
> > +static void __iomem *pl310_vbase;
> > +void __iomem *pl310_pbase;
> > +
> > +static volatile unsigned long pl310_aux_ctrl;
> > +volatile unsigned long pl310_aux_ctrl_paddr;
>
> I think that by defining those variables in assembly you would make
> your life much simpler.
Yes. But I need a function call to learn the address of those variables
from assembly now.
> I think you know your L2 is already initialized here to make sure you
> save the right aux value. Hence you should clean the variables above from
> L2 to make sure they are available at reset from DRAM (L2 is retained
> and you do not clean it on suspend, correct ?)
Yes, agreed. It's right thing to do for being safe. Actually, I did it
when I saved the variables during suspend. If I do not do, it simply
does not work. But later, when I moved the saving to init function
since it needs to be done for only once, I found it works even without
the cache clean. Then I dropped it. To be safe, now I'm adding it
back with following your comment.
>
> I do not think that code to save/restore L2 config belongs here though.
> More below.
>
> > +
> > +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:
> > + imx6q_set_lpm(STOP_POWER_OFF);
> > + imx_gpc_pre_suspend();
> > + imx_set_cpu_jump(0, v7_cpu_resume);
> > + /* Zzz ... */
> > + cpu_suspend(0, imx6q_suspend_finish);
> > + imx_smp_prepare();
> > + imx_gpc_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,
> > +};
> > +
> > +void __init imx6q_pm_init(void)
> > +{
> > + struct device_node *np;
> > + u32 reg[2];
> > +
> > + np = of_find_compatible_node(NULL, NULL, "arm,pl310-cache");
> > + of_property_read_u32_array(np, "reg", reg, ARRAY_SIZE(reg));
> > + pl310_vbase = ioremap(reg[0], reg[1]);
>
> Mmmm...is this vma ever released ? L2 is already mapped in the L2
> driver from DT or through static mappings.
I think we can do another mapping even it's been done in the L2 driver,
no?
> Overall, I think that code to restore PL310 belongs in cache-l2x0.c, not here.
> We can easily write an assembly stub that reinitialize L2 before
> resume if that's something we should and can do (security ?).
>
I would be definitely happy to see that, but before rmk agrees on that,
I have to find a way around in the platform code.
Here is the updated patch. If it looks better to you, I will
incorporate it in the v3 of the series.
8<---
diff --git a/arch/arm/mach-imx/head-v7.S b/arch/arm/mach-imx/head-v7.S
index ede908b..5a486a9 100644
--- a/arch/arm/mach-imx/head-v7.S
+++ b/arch/arm/mach-imx/head-v7.S
@@ -69,3 +69,35 @@ ENTRY(v7_secondary_startup)
b secondary_startup
ENDPROC(v7_secondary_startup)
#endif
+
+ENTRY(pl310_get_save_ptr)
+ ldr r0, =pl310_pbase
+ mov pc, lr
+ENDPROC(pl310_get_save_ptr)
+
+ENTRY(v7_cpu_resume)
+ bl v7_invalidate_l1
+ bl pl310_resume
+ b cpu_resume
+ENDPROC(v7_cpu_resume)
+
+/*
+ * The following code is located into the .data section. This is to
+ * allow pl310_pbase and pl310_aux_ctrl to be accessed with a relative
+ * load as we are running on physical address here.
+ */
+ .data
+ .align
+ENTRY(pl310_resume)
+ adr r2, pl310_pbase
+ ldmia r2, {r0, r1}
+ str r1, [r0, #L2X0_AUX_CTRL] @ restore aux_ctrl
+ mov r1, #0x1
+ str r1, [r0, #L2X0_CTRL] @ re-enable L2
+ mov pc, lr
+ENDPROC(pl310_resume)
+
+pl310_pbase:
+ .long 0
+pl310_aux_ctrl:
+ .long 0
diff --git a/arch/arm/mach-imx/pm-imx6q.c b/arch/arm/mach-imx/pm-imx6q.c
new file mode 100644
index 0000000..59cb8d2
--- /dev/null
+++ b/arch/arm/mach-imx/pm-imx6q.c
@@ -0,0 +1,88 @@
+/*
+ * 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/of.h>
+#include <linux/suspend.h>
+#include <asm/cacheflush.h>
+#include <asm/proc-fns.h>
+#include <asm/suspend.h>
+#include <asm/hardware/cache-l2x0.h>
+#include <mach/common.h>
+#include <mach/hardware.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:
+ imx6q_set_lpm(STOP_POWER_OFF);
+ imx_gpc_pre_suspend();
+ imx_set_cpu_jump(0, v7_cpu_resume);
+ /* Zzz ... */
+ cpu_suspend(0, imx6q_suspend_finish);
+ imx_smp_prepare();
+ imx_gpc_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,
+};
+
+void __init imx6q_pm_init(void)
+{
+ struct device_node *np;
+ u32 reg[2], *ptr;
+ void __iomem *base;
+
+ np = of_find_compatible_node(NULL, NULL, "arm,pl310-cache");
+ of_property_read_u32_array(np, "reg", reg, ARRAY_SIZE(reg));
+ base = ioremap(reg[0], reg[1]);
+ WARN_ON(!base);
+
+ /*
+ * On imx6q, during system suspend, ARM core gets powered off,
+ * but L2 cache is retained. To avoid cleaning the entire L2,
+ * we need to save L2 controller registers, and when system gets
+ * woke up, restore the registers and re-enable L2 before
+ * calling into cpu_resume().
+ *
+ * Most of pl310 configuration upon reset work just fine for
+ * imx6q, and the only one register we actually need to save is
+ * AUX_CTRL. Also since pl310 configuration won't change in a
+ * live system, we can save it here only once, and restore it
+ * every time system resumes back from v7_cpu_resume().
+ */
+ ptr = pl310_get_save_ptr();
+ /* save pl310 physical base address */
+ *ptr = reg[0];
+ /* save pl310 aux_ctrl register */
+ *(ptr + 1) = readl_relaxed(base + L2X0_AUX_CTRL);
+ /* ensure they are written into external memory */
+ __cpuc_flush_dcache_area((void *) ptr, sizeof(*ptr) * 2);
+ outer_clean_range(__pa(ptr), __pa(ptr + 2));
+
+ suspend_set_ops(&imx6q_pm_ops);
+}
--
Regards,
Shawn
More information about the linux-arm-kernel
mailing list