[PATCH v2 1/3] ARM: omap: Enable low-level omap3 PM code to work withCONFIG_THUMB2_KERNEL
Santosh Shilimkar
santosh.shilimkar at ti.com
Tue Dec 7 01:31:58 EST 2010
Dave,
> -----Original Message-----
> From: linaro-dev-bounces at lists.linaro.org [mailto:linaro-dev-
> bounces at lists.linaro.org] On Behalf Of Dave Martin
> Sent: Monday, December 06, 2010 11:06 PM
> To: linux-arm-kernel at lists.infradead.org
> Cc: Tony Lindgren; Dave Martin; linux-omap at vger.kernel.org; linaro-
> dev at lists.linaro.org
> Subject: [PATCH v2 1/3] ARM: omap: Enable low-level omap3 PM code to
work
> withCONFIG_THUMB2_KERNEL
>
> sleep34xx.S, sram34xx.S:
>
> * Added ENDPROC() directives for all exported function symbols.
> Without these, exported function symbols are not correctly
> identified as Thumb by the linker, causing incorrect linkage.
> This is needed to avoid some calls to the functions ending up
> with the CPU in the wrong instruction set.
>
> * Added .align directives where needed to ensure that .word won't
> be misaligned. (Note that ENTRY() implies .align; no extra
> .align has been added for these cases.)
>
> * Exported new "base address" symbols for functions which get
> copied to sram by code outside sleep34xx.S (applies to
> save_secure_ram_context and omap32xx_cpu_suspend), and fix up
> the relevant address arithmetic so that these will be copied
> and called correctly by the Thumb code in the rest of the
> kernel.
>
> * Explicitly build a few parts of sleep34xx.S as ARM.
>
> * lock_scratchpad_sem is kept as ARM because of the need to
> synchronise with hardware (?) using the SWP instruction.
>
> * save_secure_ram_context and omap34xx_cpu_suspend are built
> as ARM in case the Secure World firmware expects to decode
> the comment field from the SMC (aka smi) instructions.
>
> This can be undone later if the firmware is confirmed as
> able to decode the Thumb SMC encoding (or ignores the
> comment field).
>
> * es3_sdrc_fix should presumably only be called from the
> low-level wakeup code. To minimise the diff, switched this
> to ARM and demoted it to be a local symbol, since I believe
> it shouldn't be called from outside anyway.
>
> To prevent future maintainence problems, it would probably be
> a good idea to demote _all_ functions which aren't called
> externally to local. (i.e., everything except for
> get_es3_restore_pointer, get_restore_pointer,
> omap34xx_cpu_suspend and save_secure_ram_context).
>
> For now, I've left these as-is to minimise disruption.
>
> * Use a separate base register instead of PC-relative stores in
> sram34xx.S. This isn't permitted in Thumb-2, but at least
> some versions of gas will silently output non-working code,
> leading to unpredictable run-time behaviour.
>
> pm34xx.c, pm.h, sram.c, sram.h:
>
> * Resolve some memory addressing issues where a function symbol's
> value is assumed to be equal to the start address of the
> function body for the purpose of copying some low-level code
> from sleep34xx.S to SRAM.
>
> This assumption breaks for Thumb, since Thumb functions symbols
> have bit 0 set to indicate the Thumb-ness. This would result
> in a non-working, off-by-one copy of the function body.
>
> Tested on Beagle xM A2:
> * CONFIG_THUMB2_KERNEL && !CONFIG_SMP_ON_UP
> * CONFIG_THUMB2_KERNEL && CONFIG_SMP_ON_UP
> * !CONFIG_THUMB2_KERNEL && !CONFIG_SMP_ON_UP
> * !CONFIG_THUMB2_KERNEL && CONFIG_SMP_ON_UP
>
> Signed-off-by: Dave Martin <dave.martin at linaro.org>
> ---
> KernelVersion: 2.6.37-rc4
No need to mention but this patch changes lot of things around
power management code. You said " Tested on: omap3 (Beagle xM A2)"
What did you test ? Is it just boot test ? For sure just boot
test is not enough for this patch and needs to test the PM.
>
> arch/arm/mach-omap2/pm.h | 2 +
> arch/arm/mach-omap2/pm34xx.c | 13 ++++++++--
> arch/arm/mach-omap2/sleep34xx.S | 37
> +++++++++++++++++++++++++++++--
> arch/arm/mach-omap2/sram34xx.S | 34
+++++++++++++++++++++------
> -
> arch/arm/plat-omap/include/plat/sram.h | 1 +
> arch/arm/plat-omap/sram.c | 10 +++++++-
> 6 files changed, 80 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 0d75bfd..c333bfd 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -80,7 +80,9 @@ extern void save_secure_ram_context(u32 *addr);
> extern void omap3_save_scratchpad_contents(void);
>
> extern unsigned int omap24xx_idle_loop_suspend_sz;
> +extern char *const omap34xx_cpu_suspend_base;
> extern unsigned int omap34xx_suspend_sz;
> +extern char *const save_secure_ram_context_base;
> extern unsigned int save_secure_ram_context_sz;
> extern unsigned int omap24xx_cpu_suspend_sz;
> extern unsigned int omap34xx_cpu_suspend_sz;
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 0ec8a04..93f0ee8 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -982,11 +982,18 @@ static int __init clkdms_setup(struct clockdomain
> *clkdm, void *unused)
>
> void omap_push_sram_idle(void)
> {
> - _omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend,
> + _omap_sram_idle = omap_sram_push(omap34xx_cpu_suspend_base,
> omap34xx_cpu_suspend_sz);
> - if (omap_type() != OMAP2_DEVICE_TYPE_GP)
> - _omap_save_secure_sram =
> omap_sram_push(save_secure_ram_context,
> + _omap_sram_idle += (char *)omap34xx_cpu_suspend -
> + omap34xx_cpu_suspend_base;
> +
> + if (omap_type() != OMAP2_DEVICE_TYPE_GP) {
> + _omap_save_secure_sram = omap_sram_push(
> + save_secure_ram_context_base,
> save_secure_ram_context_sz);
> + _omap_save_secure_sram += (char *)save_secure_ram_context
-
> + save_secure_ram_context_base;
> + }
> }
>
> static int __init omap3_pm_init(void)
> diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-
> omap2/sleep34xx.S
> index 2fb205a..06ae955 100644
> --- a/arch/arm/mach-omap2/sleep34xx.S
> +++ b/arch/arm/mach-omap2/sleep34xx.S
> @@ -61,6 +61,7 @@
>
> .text
> /* Function to acquire the semaphore in scratchpad */
> + .arm @ Do this in ARM for now, due to use of
SWP.
> ENTRY(lock_scratchpad_sem)
> stmfd sp!, {lr} @ save registers on stack
> wait_sem:
> @@ -74,6 +75,9 @@ wait_loop:
> cmp r2, r0 @ did we succeed ?
> beq wait_sem @ no - try again
> ldmfd sp!, {pc} @ restore regs and return
> +ENDPROC(lock_scratchpad_sem)
> + THUMB( .thumb )
> + .align
> sdrc_scratchpad_sem:
> .word SDRC_SCRATCHPAD_SEM_V
> ENTRY(lock_scratchpad_sem_sz)
> @@ -87,6 +91,7 @@ ENTRY(unlock_scratchpad_sem)
> mov r2,#0
> str r2,[r3]
> ldmfd sp!, {pc} @ restore regs and return
> +ENDPROC(unlock_scratchpad_sem)
> ENTRY(unlock_scratchpad_sem_sz)
> .word . - unlock_scratchpad_sem
>
> @@ -96,6 +101,7 @@ ENTRY(get_restore_pointer)
> stmfd sp!, {lr} @ save registers on stack
> adr r0, restore
> ldmfd sp!, {pc} @ restore regs and return
> +ENDPROC(get_restore_pointer)
> ENTRY(get_restore_pointer_sz)
> .word . - get_restore_pointer
>
> @@ -105,10 +111,16 @@ ENTRY(get_es3_restore_pointer)
> stmfd sp!, {lr} @ save registers on stack
> adr r0, restore_es3
> ldmfd sp!, {pc} @ restore regs and return
> +ENDPROC(get_es3_restore_pointer)
> ENTRY(get_es3_restore_pointer_sz)
> .word . - get_es3_restore_pointer
>
> -ENTRY(es3_sdrc_fix)
> +@ For simplicity, make this ARM so it gets called OK from es3_restore.
> +@ Demote to a local symbol, since this gives this function an ARM ABI
> interface
> +@ which won't be callable directly from a Thumb-2 kernel. This code
> +@ shouldn't be called from outside anyway...
> + .arm
> +es3_sdrc_fix:
> ldr r4, sdrc_syscfg @ get config addr
> ldr r5, [r4] @ get value
> tst r5, #0x100 @ is part access blocked
> @@ -134,6 +146,9 @@ ENTRY(es3_sdrc_fix)
> mov r5, #0x2 @ autorefresh command
> str r5, [r4] @ kick off refreshes
> bx lr
> +ENDPROC(es3_sdrc_fix)
> + THUMB( .thumb )
> + .align
> sdrc_syscfg:
> .word SDRC_SYSCONFIG_P
> sdrc_mr_0:
> @@ -150,8 +165,12 @@ sdrc_manual_1:
> .word SDRC_MANUAL_1_P
> ENTRY(es3_sdrc_fix_sz)
> .word . - es3_sdrc_fix
> + THUMB( .thumb )
>
> /* Function to call rom code to save secure ram context */
> + .arm @ Do this in ARM for now, due to use of
SMC,
> + @ in case the Secure World firmware may
depends
> + @ on decoding the SMC instruction.
> ENTRY(save_secure_ram_context)
> stmfd sp!, {r1-r12, lr} @ save registers on stack
> save_secure_ram_debug:
> @@ -175,6 +194,9 @@ save_secure_ram_debug:
> nop
> nop
> ldmfd sp!, {r1-r12, pc}
> +ENDPROC(save_secure_ram_context)
> + THUMB( .thumb )
> + .align
> sram_phy_addr_mask:
> .word SRAM_BASE_P
> high_mask:
> @@ -183,6 +205,8 @@ api_params:
> .word 0x4, 0x0, 0x0, 0x1, 0x1
> ENTRY(save_secure_ram_context_sz)
> .word . - save_secure_ram_context
> +ENTRY(save_secure_ram_context_base)
> + .word save_secure_ram_context_base
>
> /*
> * Forces OMAP into idle state
> @@ -193,6 +217,7 @@ ENTRY(save_secure_ram_context_sz)
> * Note: This code get's copied to internal SRAM at boot. When the OMAP
> * wakes up it continues execution at the point it went to sleep.
> */
> + .arm @ Do this in ARM for now, due to use of
SMC.
> ENTRY(omap34xx_cpu_suspend)
> stmfd sp!, {r0-r12, lr} @ save registers on stack
> loop:
> @@ -563,10 +588,12 @@ loop2:
> mov r9, r4
> /* create working copy of max way size*/
> loop3:
> + mov r1, r9, lsl r5
> + mov r2, r7, lsl r2
> /* factor way and cache number into r11 */
> - orr r11, r10, r9, lsl r5
> + orr r11, r10, r1
> /* factor index number into r11 */
> - orr r11, r11, r7, lsl r2
> + orr r11, r11, r2
> /*clean & invalidate by set/way */
> mcr p15, 0, r11, c7, c10, 2
> /* decrement the way*/
> @@ -631,7 +658,9 @@ wait_dll_lock:
> cmp r5, #0x4
> bne wait_dll_lock
> bx lr
> +ENDPROC(omap34xx_cpu_suspend)
>
> + .align
> cm_idlest1_core:
> .word CM_IDLEST1_CORE_V
> sdrc_dlla_status:
> @@ -670,3 +699,5 @@ control_stat:
> .word CONTROL_STAT
> ENTRY(omap34xx_cpu_suspend_sz)
> .word . - omap34xx_cpu_suspend
> +ENTRY(omap34xx_cpu_suspend_base)
> + .word omap34xx_cpu_suspend
> diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-
> omap2/sram34xx.S
> index 3637274..65fd54f 100644
> --- a/arch/arm/mach-omap2/sram34xx.S
> +++ b/arch/arm/mach-omap2/sram34xx.S
> @@ -105,29 +105,42 @@
> * can satisfy the above requirement can enable the
> CONFIG_OMAP3_SDRC_AC_TIMING
> * option.
> */
> +__omap3_sram_configure_core_dpll_base: @ Separate local symbol
with the
> Thumb
> + @ bit _not_ set (for base address
when
> + @ copying to sram).
> ENTRY(omap3_sram_configure_core_dpll)
> stmfd sp!, {r1-r12, lr} @ store regs to stack
>
> @ pull the extra args off the
stack
> @ and store them in SRAM
> +
> +@ PC-relative stores lead to undefined behaviour in Thumb-2: use a r7
as
> a
> +@ base instead.
> +@ Be careful not to clobber r7 when maintaing this file.
> + THUMB( adr r7, omap3_sram_configure_core_dpll
)
> + .macro strtext Rt:req, label:req
> + ARM( str \Rt, \label
)
> + THUMB( str \Rt, [r7, \label - omap3_sram_configure_core_dpll]
)
> + .endm
> +
> ldr r4, [sp, #52]
> - str r4, omap_sdrc_rfr_ctrl_0_val
> + strtext r4, omap_sdrc_rfr_ctrl_0_val
> ldr r4, [sp, #56]
> - str r4, omap_sdrc_actim_ctrl_a_0_val
> + strtext r4, omap_sdrc_actim_ctrl_a_0_val
> ldr r4, [sp, #60]
> - str r4, omap_sdrc_actim_ctrl_b_0_val
> + strtext r4, omap_sdrc_actim_ctrl_b_0_val
> ldr r4, [sp, #64]
> - str r4, omap_sdrc_mr_0_val
> + strtext r4, omap_sdrc_mr_0_val
> ldr r4, [sp, #68]
> - str r4, omap_sdrc_rfr_ctrl_1_val
> + strtext r4, omap_sdrc_rfr_ctrl_1_val
> cmp r4, #0 @ if SDRC_RFR_CTRL_1 is 0,
> beq skip_cs1_params @ do not use cs1 params
> ldr r4, [sp, #72]
> - str r4, omap_sdrc_actim_ctrl_a_1_val
> + strtext r4, omap_sdrc_actim_ctrl_a_1_val
> ldr r4, [sp, #76]
> - str r4, omap_sdrc_actim_ctrl_b_1_val
> + strtext r4, omap_sdrc_actim_ctrl_b_1_val
> ldr r4, [sp, #80]
> - str r4, omap_sdrc_mr_1_val
> + strtext r4, omap_sdrc_mr_1_val
> skip_cs1_params:
> mrc p15, 0, r8, c1, c0, 0 @ read ctrl register
> bic r10, r8, #0x800 @ clear Z-bit, disable branch
> prediction
> @@ -264,7 +277,9 @@ configure_sdrc:
> skip_cs1_prog:
> ldr r12, [r11] @ posted-write barrier for SDRC
> bx lr
> +ENDPROC(omap3_sram_configure_core_dpll)
>
> + .align
> omap3_sdrc_power:
> .word OMAP34XX_SDRC_REGADDR(SDRC_POWER)
> omap3_cm_clksel1_pll:
> @@ -316,4 +331,5 @@ core_m2_mask_val:
>
> ENTRY(omap3_sram_configure_core_dpll_sz)
> .word . - omap3_sram_configure_core_dpll
> -
> +ENTRY(omap3_sram_configure_core_dpll_base)
> + .word __omap3_sram_configure_core_dpll_base
> diff --git a/arch/arm/plat-omap/include/plat/sram.h b/arch/arm/plat-
> omap/include/plat/sram.h
> index 5905100..2f27167 100644
> --- a/arch/arm/plat-omap/include/plat/sram.h
> +++ b/arch/arm/plat-omap/include/plat/sram.h
> @@ -67,6 +67,7 @@ extern u32 omap3_sram_configure_core_dpll(
> u32 sdrc_rfr_ctrl_1, u32 sdrc_actim_ctrl_a_1,
> u32 sdrc_actim_ctrl_b_1, u32 sdrc_mr_1);
> extern unsigned long omap3_sram_configure_core_dpll_sz;
> +extern char *omap3_sram_configure_core_dpll_base;
>
> #ifdef CONFIG_PM
> extern void omap_push_sram_idle(void);
> diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
> index e2c8eeb..61282f4 100644
> --- a/arch/arm/plat-omap/sram.c
> +++ b/arch/arm/plat-omap/sram.c
> @@ -386,8 +386,11 @@ void omap3_sram_restore_context(void)
> omap_sram_ceil = omap_sram_base + omap_sram_size;
>
> _omap3_sram_configure_core_dpll =
> - omap_sram_push(omap3_sram_configure_core_dpll,
> + omap_sram_push(omap3_sram_configure_core_dpll_base,
> omap3_sram_configure_core_dpll_sz);
> + _omap3_sram_configure_core_dpll +=
> + (char *)omap3_sram_configure_core_dpll -
> + omap3_sram_configure_core_dpll_base;
> omap_push_sram_idle();
> }
> #endif /* CONFIG_PM */
> @@ -395,8 +398,11 @@ void omap3_sram_restore_context(void)
> static int __init omap34xx_sram_init(void)
> {
> _omap3_sram_configure_core_dpll =
> - omap_sram_push(omap3_sram_configure_core_dpll,
> + omap_sram_push(omap3_sram_configure_core_dpll_base,
> omap3_sram_configure_core_dpll_sz);
> + _omap3_sram_configure_core_dpll +=
> + (char *)omap3_sram_configure_core_dpll -
> + omap3_sram_configure_core_dpll_base;
> omap_push_sram_idle();
> return 0;
> }
> --
> 1.7.1
>
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev at lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
More information about the linux-arm-kernel
mailing list