On Fri, Dec 17, 2010 at 4:58 PM, Nishanth Menon <nm@xxxxxx> wrote: > > jean.pihet@xxxxxxxxxxxxxx had written, on 12/17/2010 04:08 AM, the > following: >> >> From: Jean Pihet <j-pihet@xxxxxx> > > 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@xxxxxx> >> --- >> 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@xxxxxx> >> + * > > 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@xxxxxx> >> @@ -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@xxxxxx> > Tested on: > SDP3630 > SDP3430 > Test script: > http://pastebin.mozilla.org/889933 > > > -- > Regards, > Nishanth Menon > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
