[PATCH 7/7] OMAP3: ASM sleep code format rework
Jean Pihet
jean.pihet at newoldbits.com
Sat Dec 18 10:56:14 EST 2010
On Fri, Dec 17, 2010 at 4:58 PM, Nishanth Menon <nm at ti.com> wrote:
>
> 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 $
Oops sorry about that. Fixed!
Thanks,
Jean
>
> 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
Removed the new copyright.
>
>> * (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