[PATCH 7/7] OMAP3: ASM sleep code format rework
Nishanth Menon
nm at ti.com
Fri Dec 17 10:58:17 EST 2010
jean.pihet at newoldbits.com had written, on 12/17/2010 04:08 AM, the
following:
> From: Jean Pihet <j-pihet at ti.com>
Thanks for doing this, could you pull in the other cosmetic changes from
patches 1-6 here as well?
Also, please run checkpatch.pl --strict:
ERROR: trailing whitespace
#426: FILE: arch/arm/mach-omap2/sleep34xx.S:590:
+^I * The caches and prediction are not enabled here, they $
total: 1 errors, 0 warnings, 0 checks, 447 lines checked
NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
scripts/cleanfile
Also reported by git am:
linux-2.6/.git/rebase-apply/patch:355: trailing whitespace.
* The caches and prediction are not enabled here, they
warning: 1 line adds whitespace errors.
>
> Cosmetic fixes to the code:
> - white spaces and tabs,
> - alignement,
> - comments rephrase and typos,
> - multi-line comments
>
> Tested on N900 and Beagleboard with full RET and OFF modes,
> using cpuidle and suspend.
>
> Signed-off-by: Jean Pihet <j-pihet at ti.com>
> ---
> arch/arm/mach-omap2/sleep34xx.S | 226 +++++++++++++++++++++------------------
> 1 files changed, 122 insertions(+), 104 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
> index 8e5004a..6376427 100644
> --- a/arch/arm/mach-omap2/sleep34xx.S
> +++ b/arch/arm/mach-omap2/sleep34xx.S
> @@ -1,6 +1,10 @@
> /*
> * linux/arch/arm/mach-omap2/sleep.S
if you are cleaning things up, you might as well throw this out.
> *
> + * (C) Copyright 2010
> + * Texas Instruments
if you do use this, please follow the requirements that have been
standardized in side TI now:
Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com/
> + * Jean Pihet <j-pihet at ti.com>
> + *
umm.. will leave it for Kevin to comment. the series was a cleanup I
agree, functionally there is much contribution from lot of other folks
as well.. personally, since all contributions are maintained in git
history anyways.. I dont usually bother about touching the original
copyrights - but that is just me
> * (C) Copyright 2007
> * Texas Instruments
> * Karthik Dasu <karthik-dp at ti.com>
> @@ -81,20 +85,20 @@
> .text
> /* Function call to get the restore pointer for resume from OFF */
> ENTRY(get_restore_pointer)
> - stmfd sp!, {lr} @ save registers on stack
> + stmfd sp!, {lr} @ save registers on stack
> adr r0, restore
> - ldmfd sp!, {pc} @ restore regs and return
> + ldmfd sp!, {pc} @ restore regs and return
> ENTRY(get_restore_pointer_sz)
> - .word . - get_restore_pointer
> + .word . - get_restore_pointer
>
> .text
> /* Function call to get the restore pointer for 3630 resume from OFF */
> ENTRY(get_omap3630_restore_pointer)
> - stmfd sp!, {lr} @ save registers on stack
> + stmfd sp!, {lr} @ save registers on stack
> adr r0, restore_3630
> - ldmfd sp!, {pc} @ restore regs and return
> + ldmfd sp!, {pc} @ restore regs and return
> ENTRY(get_omap3630_restore_pointer_sz)
> - .word . - get_omap3630_restore_pointer
> + .word . - get_omap3630_restore_pointer
>
> .text
> /* Function call to get the restore pointer for ES3 to resume from OFF */
> @@ -112,16 +116,16 @@ ENTRY(get_es3_restore_pointer_sz)
> * place on 3630. Hopefully some version in the future may not need this.
> */
> ENTRY(enable_omap3630_toggle_l2_on_restore)
> - stmfd sp!, {lr} @ save registers on stack
> + stmfd sp!, {lr} @ save registers on stack
> /* Setup so that we will disable and enable l2 */
> mov r1, #0x1
> str r1, l2dis_3630
> - ldmfd sp!, {pc} @ restore regs and return
> + ldmfd sp!, {pc} @ restore regs and return
>
> + .text
> /* Function to call rom code to save secure ram context */
> ENTRY(save_secure_ram_context)
> stmfd sp!, {r1-r12, lr} @ save registers on stack
> -
> adr r3, api_params @ r3 points to parameters
> str r0, [r3,#0x4] @ r0 has sdram address
> ldr r12, high_mask
> @@ -150,6 +154,7 @@ api_params:
> ENTRY(save_secure_ram_context_sz)
> .word . - save_secure_ram_context
>
> +
IMHO, spurious - just need one EOL, not 2.
> /*
> * ======================
> * == Idle entry point ==
> @@ -163,13 +168,14 @@ ENTRY(save_secure_ram_context_sz)
> * and executes the WFI instruction
> *
> * Notes:
> - * - this code gets copied to internal SRAM at boot.
> + * - this code gets copied to internal SRAM at boot and after wake-up
> + * from OFF mode
> * - when the OMAP wakes up it continues at different execution points
> * depending on the low power mode (non-OFF vs OFF modes),
> * cf. 'Resume path for xxx mode' comments.
> */
> ENTRY(omap34xx_cpu_suspend)
> - stmfd sp!, {r0-r12, lr} @ save registers on stack
> + stmfd sp!, {r0-r12, lr} @ save registers on stack
>
> /*
> * r0 contains restore pointer in sdram
> @@ -276,9 +282,9 @@ clean_l2:
> * - should be faster and will change with kernel
> * - 'might' have to copy address, load and jump to it
> */
> - ldr r1, kernel_flush
> - mov lr, pc
> - bx r1
> + ldr r1, kernel_flush
> + mov lr, pc
> + bx r1
>
> omap3_do_wfi:
> ldr r4, sdrc_power @ read the SDRC_POWER register
> @@ -371,18 +377,18 @@ restore_3630:
> /* Fall thru to common code for the remaining logic */
>
> restore:
> - /*
> + /*
> * Check what was the reason for mpu reset and store the reason in r9:
> * 0 - No context lost
> - * 1 - Only L1 and logic lost
> - * 2 - Only L2 lost - In this case, we wont be here
> - * 3 - Both L1 and L2 lost
> + * 1 - Only L1 and logic lost
> + * 2 - Only L2 lost - In this case, we wont be here
> + * 3 - Both L1 and L2 lost
> */
> - ldr r1, pm_pwstctrl_mpu
> + ldr r1, pm_pwstctrl_mpu
> ldr r2, [r1]
> - and r2, r2, #0x3
> - cmp r2, #0x0 @ Check if target power state was OFF or RET
> - moveq r9, #0x3 @ MPU OFF => L1 and L2 lost
> + and r2, r2, #0x3
> + cmp r2, #0x0 @ Check if target power state was OFF or RET
> + moveq r9, #0x3 @ MPU OFF => L1 and L2 lost
> movne r9, #0x1 @ Only L1 and L2 lost => avoid L2 invalidation
> bne logic_l1_restore
>
> @@ -398,71 +404,74 @@ skipl2dis:
> and r1, #0x700
> cmp r1, #0x300
> beq l2_inv_gp
> - mov r0, #40 @ set service ID for PPA
> - mov r12, r0 @ copy secure Service ID in r12
> - mov r1, #0 @ set task id for ROM code in r1
> - mov r2, #4 @ set some flags in r2, r6
> + mov r0, #40 @ set service ID for PPA
> + mov r12, r0 @ copy secure Service ID in r12
> + mov r1, #0 @ set task id for ROM code in r1
> + mov r2, #4 @ set some flags in r2, r6
> mov r6, #0xff
> adr r3, l2_inv_api_params @ r3 points to dummy parameters
> mcr p15, 0, r0, c7, c10, 4 @ data write barrier
> mcr p15, 0, r0, c7, c10, 5 @ data memory barrier
> .word 0xE1600071 @ call SMI monitor (smi #1)
> /* Write to Aux control register to set some bits */
> - mov r0, #42 @ set service ID for PPA
> - mov r12, r0 @ copy secure Service ID in r12
> - mov r1, #0 @ set task id for ROM code in r1
> - mov r2, #4 @ set some flags in r2, r6
> + mov r0, #42 @ set service ID for PPA
> + mov r12, r0 @ copy secure Service ID in r12
> + mov r1, #0 @ set task id for ROM code in r1
> + mov r2, #4 @ set some flags in r2, r6
> mov r6, #0xff
> ldr r4, scratchpad_base
> - ldr r3, [r4, #0xBC] @ r3 points to parameters
> + ldr r3, [r4, #0xBC] @ r3 points to parameters
> mcr p15, 0, r0, c7, c10, 4 @ data write barrier
> mcr p15, 0, r0, c7, c10, 5 @ data memory barrier
> .word 0xE1600071 @ call SMI monitor (smi #1)
>
> #ifdef CONFIG_OMAP3_L2_AUX_SECURE_SAVE_RESTORE
> /* Restore L2 aux control register */
> - @ set service ID for PPA
> + @ set service ID for PPA
> mov r0, #CONFIG_OMAP3_L2_AUX_SECURE_SERVICE_SET_ID
> - mov r12, r0 @ copy service ID in r12
> - mov r1, #0 @ set task ID for ROM code in r1
> - mov r2, #4 @ set some flags in r2, r6
> + mov r12, r0 @ copy service ID in r12
> + mov r1, #0 @ set task ID for ROM code in r1
> + mov r2, #4 @ set some flags in r2, r6
> mov r6, #0xff
> ldr r4, scratchpad_base
> ldr r3, [r4, #0xBC]
> - adds r3, r3, #8 @ r3 points to parameters
> + adds r3, r3, #8 @ r3 points to parameters
> mcr p15, 0, r0, c7, c10, 4 @ data write barrier
> mcr p15, 0, r0, c7, c10, 5 @ data memory barrier
> .word 0xE1600071 @ call SMI monitor (smi #1)
> #endif
> b logic_l1_restore
> +
> l2_inv_api_params:
> - .word 0x1, 0x00
> + .word 0x1, 0x00
> l2_inv_gp:
> /* Execute smi to invalidate L2 cache */
> - mov r12, #0x1 @ set up to invalide L2
> -smi: .word 0xE1600070 @ Call SMI monitor (smieq)
> + mov r12, #0x1 @ set up to invalidate L2
> + .word 0xE1600070 @ Call SMI monitor (smieq)
> /* Write to Aux control register to set some bits */
> ldr r4, scratchpad_base
> ldr r3, [r4,#0xBC]
> ldr r0, [r3,#4]
> mov r12, #0x3
> - .word 0xE1600070 @ Call SMI monitor (smieq)
> + .word 0xE1600070 @ Call SMI monitor (smieq)
> ldr r4, scratchpad_base
> ldr r3, [r4,#0xBC]
> ldr r0, [r3,#12]
> mov r12, #0x2
> - .word 0xE1600070 @ Call SMI monitor (smieq)
> + .word 0xE1600070 @ Call SMI monitor (smieq)
> logic_l1_restore:
> ldr r1, l2dis_3630
> - cmp r1, #0x1 @ Do we need to re-enable L2 on 3630?
> + cmp r1, #0x1 @ Test if L2 re-enable needed on 3630
> bne skipl2reen
> mrc p15, 0, r1, c1, c0, 1
> - orr r1, r1, #2 @ re-enable L2 cache
> + orr r1, r1, #2 @ re-enable L2 cache
> mcr p15, 0, r1, c1, c0, 1
> skipl2reen:
> mov r1, #0
> - /* Invalidate all instruction caches to PoU
> - * and flush branch target cache */
> + /*
> + * Invalidate all instruction caches to PoU
> + * and flush branch target cache
> + */
> mcr p15, 0, r1, c7, c5, 0
>
> ldr r4, scratchpad_base
> @@ -483,33 +492,33 @@ skipl2reen:
> MCR p15, 0, r6, c2, c0, 1
> /* Translation table base control register */
> MCR p15, 0, r7, c2, c0, 2
> - /*domain access Control Register */
> + /* Domain access Control Register */
> MCR p15, 0, r8, c3, c0, 0
> - /* data fault status Register */
> + /* Data fault status Register */
> MCR p15, 0, r9, c5, c0, 0
>
> - ldmia r3!,{r4-r8}
> - /* instruction fault status Register */
> + ldmia r3!,{r4-r8}
> + /* Instruction fault status Register */
> MCR p15, 0, r4, c5, c0, 1
> - /*Data Auxiliary Fault Status Register */
> + /* Data Auxiliary Fault Status Register */
> MCR p15, 0, r5, c5, c1, 0
> - /*Instruction Auxiliary Fault Status Register*/
> + /* Instruction Auxiliary Fault Status Register*/
> MCR p15, 0, r6, c5, c1, 1
> - /*Data Fault Address Register */
> + /* Data Fault Address Register */
> MCR p15, 0, r7, c6, c0, 0
> - /*Instruction Fault Address Register*/
> + /* Instruction Fault Address Register*/
> MCR p15, 0, r8, c6, c0, 2
> - ldmia r3!,{r4-r7}
> + ldmia r3!,{r4-r7}
>
> - /* user r/w thread and process ID */
> + /* User r/w thread and process ID */
> MCR p15, 0, r4, c13, c0, 2
> - /* user ro thread and process ID */
> + /* User ro thread and process ID */
> MCR p15, 0, r5, c13, c0, 3
> - /*Privileged only thread and process ID */
> + /* Privileged only thread and process ID */
> MCR p15, 0, r6, c13, c0, 4
> - /* cache size selection */
> + /* Cache size selection */
> MCR p15, 2, r7, c0, c0, 0
> - ldmia r3!,{r4-r8}
> + ldmia r3!,{r4-r8}
> /* Data TLB lockdown registers */
> MCR p15, 0, r4, c10, c0, 0
> /* Instruction TLB lockdown registers */
> @@ -521,26 +530,27 @@ skipl2reen:
> /* Context PID */
> MCR p15, 0, r8, c13, c0, 1
>
> - ldmia r3!,{r4-r5}
> - /* primary memory remap register */
> + ldmia r3!,{r4-r5}
> + /* Primary memory remap register */
> MCR p15, 0, r4, c10, c2, 0
> - /*normal memory remap register */
> + /* Normal memory remap register */
> MCR p15, 0, r5, c10, c2, 1
>
> /* Restore cpsr */
> - ldmia r3!,{r4} /*load CPSR from SDRAM*/
> - msr cpsr, r4 /*store cpsr */
> + ldmia r3!,{r4} @ load CPSR from SDRAM
> + msr cpsr, r4 @ store cpsr
>
> /* Enabling MMU here */
> - mrc p15, 0, r7, c2, c0, 2 /* Read TTBRControl */
> - /* Extract N (0:2) bits and decide whether to use TTBR0 or TTBR1*/
> + mrc p15, 0, r7, c2, c0, 2 @ Read TTBRControl
> + /* Extract N (0:2) bits and decide whether to use TTBR0 or TTBR1 */
> and r7, #0x7
> cmp r7, #0x0
> beq usettbr0
> ttbr_error:
> - /* More work needs to be done to support N[0:2] value other than 0
> - * So looping here so that the error can be detected
> - */
> + /*
> + * More work needs to be done to support N[0:2] value other than 0
> + * So looping here so that the error can be detected
> + */
> b ttbr_error
> usettbr0:
> mrc p15, 0, r2, c2, c0, 0
> @@ -548,21 +558,25 @@ usettbr0:
> and r2, r5
> mov r4, pc
> ldr r5, table_index_mask
> - and r4, r5 /* r4 = 31 to 20 bits of pc */
> + and r4, r5 @ r4 = 31 to 20 bits of pc
> /* Extract the value to be written to table entry */
> ldr r1, table_entry
> - add r1, r1, r4 /* r1 has value to be written to table entry*/
> + /* r1 has the value to be written to table entry*/
> + add r1, r1, r4
> /* Getting the address of table entry to modify */
> lsr r4, #18
> - add r2, r4 /* r2 has the location which needs to be modified */
> + /* r2 has the location which needs to be modified */
> + add r2, r4
> /* Storing previous entry of location being modified */
> ldr r5, scratchpad_base
> ldr r4, [r2]
> str r4, [r5, #0xC0]
> /* Modify the table entry */
> str r1, [r2]
> - /* Storing address of entry being modified
> - * - will be restored after enabling MMU */
> + /*
> + * Storing address of entry being modified
> + * - will be restored after enabling MMU
> + */
> ldr r5, scratchpad_base
> str r2, [r5, #0xC4]
>
> @@ -571,8 +585,11 @@ usettbr0:
> mcr p15, 0, r0, c7, c5, 6 @ Invalidate branch predictor array
> mcr p15, 0, r0, c8, c5, 0 @ Invalidate instruction TLB
> mcr p15, 0, r0, c8, c6, 0 @ Invalidate data TLB
> - /* Restore control register but dont enable caches here*/
> - /* Caches will be enabled after restoring MMU table entry */
> + /*
> + * Restore control register. This enables the MMU.
> + * The caches and prediction are not enabled here, they
> + * will be enabled after restoring the MMU table entry.
> + */
> ldmia r3!, {r4}
> /* Store previous value of control register in scratchpad */
> str r4, [r5, #0xC8]
> @@ -585,7 +602,7 @@ usettbr0:
> * == Exit point from OFF mode ==
> * ==============================
> */
> - ldmfd sp!, {r0-r12, pc} @ restore regs and return
> + ldmfd sp!, {r0-r12, pc} @ restore regs and return
>
>
> /*
> @@ -651,55 +668,56 @@ ENTRY(es3_sdrc_fix_sz)
> /* Make sure SDRC accesses are ok */
> wait_sdrc_ok:
>
> -/* DPLL3 must be locked before accessing the SDRC. Maybe the HW ensures this. */
> +/* DPLL3 must be locked before accessing the SDRC. Maybe the HW ensures this */
> ldr r4, cm_idlest_ckgen
> wait_dpll3_lock:
> ldr r5, [r4]
> tst r5, #1
> beq wait_dpll3_lock
>
> - ldr r4, cm_idlest1_core
> + ldr r4, cm_idlest1_core
> wait_sdrc_ready:
> - ldr r5, [r4]
> - tst r5, #0x2
> - bne wait_sdrc_ready
> + ldr r5, [r4]
> + tst r5, #0x2
> + bne wait_sdrc_ready
> /* allow DLL powerdown upon hw idle req */
> - ldr r4, sdrc_power
> - ldr r5, [r4]
> - bic r5, r5, #0x40
> - str r5, [r4]
> -is_dll_in_lock_mode:
> + ldr r4, sdrc_power
> + ldr r5, [r4]
> + bic r5, r5, #0x40
> + str r5, [r4]
>
> - /* Is dll in lock mode? */
> - ldr r4, sdrc_dlla_ctrl
> - ldr r5, [r4]
> - tst r5, #0x4
> - bxne lr
> - /* wait till dll locks */
> +is_dll_in_lock_mode:
> + /* Is dll in lock mode? */
> + ldr r4, sdrc_dlla_ctrl
> + ldr r5, [r4]
> + tst r5, #0x4
> + bxne lr @ Return if locked
> + /* wait till dll locks */
> wait_dll_lock_timed:
> ldr r4, wait_dll_lock_counter
> add r4, r4, #1
> str r4, wait_dll_lock_counter
> ldr r4, sdrc_dlla_status
> - mov r6, #8 /* Wait 20uS for lock */
> + /* Wait 20uS for lock */
> + mov r6, #8
> wait_dll_lock:
> subs r6, r6, #0x1
> beq kick_dll
> - ldr r5, [r4]
> - and r5, r5, #0x4
> - cmp r5, #0x4
> - bne wait_dll_lock
> - bx lr
> + ldr r5, [r4]
> + and r5, r5, #0x4
> + cmp r5, #0x4
> + bne wait_dll_lock
> + bx lr @ Return when locked
>
> /* disable/reenable DLL if not locked */
> kick_dll:
> ldr r4, sdrc_dlla_ctrl
> ldr r5, [r4]
> mov r6, r5
> - bic r6, #(1<<3) /* disable dll */
> + bic r6, #(1<<3) @ disable dll
> str r6, [r4]
> dsb
> - orr r6, r6, #(1<<3) /* enable dll */
> + orr r6, r6, #(1<<3) @ enable dll
> str r6, [r4]
> dsb
> ldr r4, kick_counter
> @@ -724,7 +742,7 @@ scratchpad_base:
> sram_base:
> .word SRAM_BASE_P + 0x8000
> sdrc_power:
> - .word SDRC_POWER_V
> + .word SDRC_POWER_V
> ttbrbit_mask:
> .word 0xFFFFC000
> table_index_mask:
> @@ -738,9 +756,9 @@ control_stat:
> control_mem_rta:
> .word CONTROL_MEM_RTA_CTRL
> kernel_flush:
> - .word v7_flush_dcache_all
> + .word v7_flush_dcache_all
> l2dis_3630:
> - .word 0
> + .word 0
> /* these 2 words need to be at the end !!! */
> kick_counter:
> .word 0
as such, this patch:
Tested-by: Nishanth Menon <nm at ti.com>
Tested on:
SDP3630
SDP3430
Test script:
http://pastebin.mozilla.org/889933
--
Regards,
Nishanth Menon
More information about the linux-arm-kernel
mailing list