On Mon, 18 Feb 2013, Shawn Guo wrote:
> On Sat, Feb 16, 2013 at 12:14:49AM -0500, Nicolas Pitre wrote:
> > Something like this should work:
> >
> > diff --git a/arch/arm/mach-imx/headsmp.S b/arch/arm/mach-imx/headsmp.S
> > index 7e49deb128..9de26f3edb 100644
> > --- a/arch/arm/mach-imx/headsmp.S
> > +++ b/arch/arm/mach-imx/headsmp.S
> > @@ -99,8 +99,11 @@ phys_l2x0_saved_regs:
> > #endif
> >
> > ENTRY(v7_cpu_resume)
> > - bl v7_invalidate_l1
> > + ldr ip, 2f
> > + mov lr, pc
> > +1: add pc, pc, ip
> > pl310_resume
> > b cpu_resume
> > +2: .word v7_invalidate_l1 - 1b - 8
> > ENDPROC(v7_cpu_resume)
> > #endif
> >
> Yes, it works.
>
> >
> > However it is probably best to move all the code to the .text section
> > where it belongs and fixup the data access instead. I must plead guilty
> > for introducing this idea of putting code in the .data section with the
> > SA1100resume code over 10 years ago that everyone copied since.
> >
> > So here's how I think it should look instead, and how I should have done
> > the SA1100 code back then:
> >
> > diff --git a/arch/arm/mach-imx/headsmp.S b/arch/arm/mach-imx/headsmp.S
> > index 7e49deb128..38a544a037 100644
> > --- a/arch/arm/mach-imx/headsmp.S
> > +++ b/arch/arm/mach-imx/headsmp.S
> > @@ -73,16 +73,16 @@ ENDPROC(v7_secondary_startup)
> >
> > #ifdef CONFIG_PM
> > /*
> > - * The following code is located into the .data section. This is to
> > - * allow phys_l2x0_saved_regs to be accessed with a relative load
> > - * as we are running on physical address here.
> > + * The following code must assume it is running from physical address
> > + * where absolute virtual addresses to the data section have to be
> > + * turned into relative ones.
> > */
> > - .data
> > - .align
> >
> > #ifdef CONFIG_CACHE_L2X0
> > .macro pl310_resume
> > - ldr r2, phys_l2x0_saved_regs
> > + adr r0, phys_l2x0_saved_ptr_offset
> > + ldr r2, [r0]
> > + add r2, r2, r0
> > ldr r0, [r2, #L2X0_R_PHY_BASE] @ get physical base of l2x0
> > ldr r1, [r2, #L2X0_R_AUX_CTRL] @ get aux_ctrl value
> > str r1, [r0, #L2X0_AUX_CTRL] @ restore aux_ctrl
> > @@ -90,9 +90,13 @@ ENDPROC(v7_secondary_startup)
> > str r1, [r0, #L2X0_CTRL] @ re-enable L2
> > .endm
> >
> > + .bss
> > .globl phys_l2x0_saved_regs
> > phys_l2x0_saved_regs:
> > - .long 0
> > + .space 4
> > + .previous
> > +phys_l2x0_saved_ptr_offset:
> > + .word phys_l2x0_saved_regs - .
> > #else
> > .macro pl310_resume
> > .endm
> >
> But this does not work. It seems the execution jumps to the start of
> kernel on system resuming.
Try the following instead. It makes the code simpler and easier to
debug.
diff --git a/arch/arm/mach-imx/headsmp.S b/arch/arm/mach-imx/headsmp.S
index 7e49deb128..27bc06e910 100644
--- a/arch/arm/mach-imx/headsmp.S
+++ b/arch/arm/mach-imx/headsmp.S
@@ -73,16 +73,16 @@ ENDPROC(v7_secondary_startup)
#ifdef CONFIG_PM
/*
- * The following code is located into the .data section. This is to
- * allow phys_l2x0_saved_regs to be accessed with a relative load
- * as we are running on physical address here.
+ * The following code must assume it is running from physical address
+ * where absolute virtual addresses to the data section have to be
+ * turned into relative ones.
*/
- .data
- .align
#ifdef CONFIG_CACHE_L2X0
.macro pl310_resume
- ldr r2, phys_l2x0_saved_regs
+ adr r0, l2x0_saved_regs_offset
+ ldr r2, [r0]
+ add r2, r2, r0
ldr r0, [r2, #L2X0_R_PHY_BASE] @ get physical base of l2x0
ldr r1, [r2, #L2X0_R_AUX_CTRL] @ get aux_ctrl value
str r1, [r0, #L2X0_AUX_CTRL] @ restore aux_ctrl
@@ -90,9 +90,9 @@ ENDPROC(v7_secondary_startup)
str r1, [r0, #L2X0_CTRL] @ re-enable L2
.endm
- .globl phys_l2x0_saved_regs
-phys_l2x0_saved_regs:
- .long 0
+l2x0_saved_regs_offset:
+ .word l2x0_saved_regs - .
+
#else
.macro pl310_resume
.endm
diff --git a/arch/arm/mach-imx/pm-imx6q.c b/arch/arm/mach-imx/pm-imx6q.c
index f7b0c2b1b9..f3791f980d 100644
--- a/arch/arm/mach-imx/pm-imx6q.c
+++ b/arch/arm/mach-imx/pm-imx6q.c
@@ -21,8 +21,6 @@
#include <mach/common.h>
#include <mach/hardware.h>
-extern unsigned long phys_l2x0_saved_regs;
-
static int imx6q_suspend_finish(unsigned long val)
{
cpu_do_idle();
@@ -55,18 +53,5 @@ static const struct platform_suspend_ops imx6q_pm_ops = {
void __init imx6q_pm_init(void)
{
- /*
- * The l2x0 core code provides an infrastucture to save and restore
- * l2x0 registers across suspend/resume cycle. But because imx6q
- * retains L2 content during suspend and needs to resume L2 before
- * MMU is enabled, it can only utilize register saving support and
- * have to take care of restoring on its own. So we save physical
- * address of the data structure used by l2x0 core to save registers,
- * and later restore the necessary ones in imx6q resume entry.
- */
-#ifdef CONFIG_CACHE_L2X0
- phys_l2x0_saved_regs = __pa(&l2x0_saved_regs);
-#endif
-
suspend_set_ops(&imx6q_pm_ops);
}
Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
