From patchwork Fri May 20 12:39:37 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Frank Hofmann X-Patchwork-Id: 96598 Return-Path: X-Original-To: incoming-imx@patchwork.ozlabs.org Delivered-To: patchwork-incoming-imx@bilbo.ozlabs.org Received: from bombadil.infradead.org (bombadil.infradead.org [18.85.46.34]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 83128B719D for ; Fri, 20 May 2011 23:02:09 +1000 (EST) Received: from canuck.infradead.org ([2001:4978:20e::1]) by bombadil.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1QNPLA-00013h-IZ; Fri, 20 May 2011 13:02:04 +0000 Received: from localhost ([127.0.0.1] helo=canuck.infradead.org) by canuck.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1QNPL8-0006VU-Po; Fri, 20 May 2011 13:02:02 +0000 Received: from casper.infradead.org ([2001:770:15f::2]) by canuck.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1QNPL5-0006Ui-A4 for linux-arm-kernel@canuck.infradead.org; Fri, 20 May 2011 13:01:59 +0000 Received: from mxvs2.esa.t-systems.com ([81.7.202.143]) by casper.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1QNOzl-00081h-7O for linux-arm-kernel@lists.infradead.org; Fri, 20 May 2011 12:39:58 +0000 Received: from unknown (HELO nl-exc-01.intra.local) ([82.210.235.24]) by mx.esa.t-systems.com with ESMTP; 20 May 2011 12:39:39 +0000 Received: from magrathea ([10.101.8.37]) by nl-exc-01.intra.local with Microsoft SMTPSVC(6.0.3790.3959); Fri, 20 May 2011 14:39:38 +0200 Date: Fri, 20 May 2011 13:39:37 +0100 (BST) From: Frank Hofmann To: Dave Martin Subject: Re: [RFC PATCH] ARM hibernation / suspend-to-disk support code In-Reply-To: <20110520113758.GA3141@arm.com> Message-ID: References: <3DCE2F529B282E4B8F53D4D8AA406A07014FFE@008-AM1MPN1-022.mgdnok.nokia.com> <20110520113758.GA3141@arm.com> User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) MIME-Version: 1.0 X-OriginalArrivalTime: 20 May 2011 12:39:38.0365 (UTC) FILETIME=[FB7292D0:01CC16EA] X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20110520_133957_504287_F0245D7D X-CRM114-Status: GOOD ( 37.63 ) X-Spam-Score: -2.6 (--) X-Spam-Report: SpamAssassin version 3.3.2-r929478 on casper.infradead.org summary: Content analysis details: (-2.6 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [81.7.202.143 listed in list.dnswl.org] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Frank Hofmann , linux-pm@lists.linux-foundation.org, tuxonice-devel@tuxonice.net, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: frank.hofmann@tomtom.com List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+incoming-imx=patchwork.ozlabs.org@lists.infradead.org List-Id: linux-imx-kernel.lists.patchwork.ozlabs.org On Fri, 20 May 2011, Dave Martin wrote: [ ... ] >> +/* >> + * Save the current CPU state before suspend / poweroff. >> + */ >> +ENTRY(swsusp_arch_suspend) >> + ldr r0, =__swsusp_arch_ctx >> + mrs r1, cpsr >> + str r1, [r0], #4 /* CPSR */ >> +ARM( msr cpsr_c, #SYSTEM_MODE ) >> +THUMB( mov r2, #SYSTEM_MODE ) >> +THUMB( msr cpsr_c, r2 ) > > For Thumb-2 kernels, you can use the cps instruction to change the CPU > mode: > cps #SYSTEM_MODE > > For ARM though, this instruction is only supported for ARMv6 and above, > so it's best to keep the msr form for compatibility for that case. Ah, ok, no problem will make that change, looks good. Do all ARMs have cpsr / spsr as used here ? Or is that code restricted to ARMv5+ ? I don't have the CPU evolution history there, only got involved with ARM when armv6 already was out. I've simply done there what the "setmode" macro from is doing, have chosen not to include that file because it overrides "push" on a global scale for no good reason and that sort of thing makes me cringe. > >> + stm r0!, {r4-r12,lr} /* nonvolatile regs */ > > Since r12 is allowed to be corrupted across a function call, we > probably don't need to save it. ah ok thx for clarification. Earlier iterations of the patch just saved r0-r14 there, "just to have it all", doing it right is best as always. > [ ... ] >> + bl __save_processor_state > > You're right. I've attached the codechanges which implement __save/__restore... for TI OMAP3 and Samsung S5P64xx, to illustrate, again (that's the stuff referred to in the earlier mail I mentioned in first post; beware of code churn in there, those diffs don't readily apply to 'just any' kernel). These hooks are essentially the same as the machine-specific cpu_suspend() except that we substitute "r0" (the save context after the generic part) as target for where-to-save-the-state, and we make the funcs return instead of switching to OFF mode. That's what I meant with "backdoorish". A better way would be to change the cpu_suspend interface so that it returns instead of directly switching to off mode / powering down. Russell has lately been making changes in this area; the current kernels are a bit different here since the caller already supplies the state-save-buffer, while the older ones used to choose in some mach-specific way where to store that state. (one of the reasons why these mach-dependent enablers aren't part of this patch suggestion ...) > >> + pop {lr} >> + b swsusp_save >> +ENDPROC(swsusp_arch_suspend) > > I'm not too familiar with the suspend/resume stuff, so I may be asking > a dumb question here, but: > > Where do we save/restore r8_FIQ..r13_FIQ, r13_IRQ, r13_UND and r13_ABT? > (I'm assuming there's no reason to save/restore r14 or SPSR for any > exception mode, since we presumably aren't going to suspend/resume > from inside an exception handler (?)) > > The exception stack pointers might conceivably be reinitialised to > sane values on resume, without necessarily needing to save/restore > them, providing my assumption in the previous paragraph is correct. > > r8_FIQ..r12_FIQ can store arbitrary state used by the FIQ handler, > if FIQ is in use. Can we expect any driver using FIQ to save/restore > this state itself, rather than doing it globally? This may be > reasonable. We don't need to save/restore those, because at the time the snapshot is taken interrupts are off and we cannot be in any trap handler either. On resume, the kernel that has been loaded has initialized them properly already. If there's really any driver out there which uses FIQ in such an exclusive manner that it expects FIQ register bank contents to remain the same across multiple FIQ events then it's rather that driver's responsibility to perform the save/restore via suspend/resume callbacks. See also Russell's mails about that, for previous attempts a year and half a year ago to get "something for ARM hibernation" in: https://lists.linux-foundation.org/pipermail/linux-pm/2010-July/027571.html https://lists.linux-foundation.org/pipermail/linux-pm/2010-December/029665.html The kernel doesn't do IRQ/FIQ/ABT/UND save / restore for suspend-to-ram either. CPU hotplug support (re)initializes those. I believe we're fine. > > Cheers > ---Dave > > Thanks, FrankH. diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S index ea4e498..19ecd8e 100644 --- a/arch/arm/mach-omap2/sleep34xx.S +++ b/arch/arm/mach-omap2/sleep34xx.S @@ -358,6 +358,7 @@ logic_l1_restore: ldr r4, scratchpad_base ldr r3, [r4,#0xBC] adds r3, r3, #16 +.Llogic_l1_restore_internal: ldmia r3!, {r4-r6} mov sp, r4 msr spsr_cxsf, r5 @@ -433,6 +434,10 @@ ttbr_error: */ b ttbr_error usettbr0: +#ifdef CONFIG_HIBERNATION + cmp r1, #1 + ldmeqfd sp!, { r0 - r12, pc } @ early return from __restore_processor_state +#endif mrc p15, 0, r2, c2, c0, 0 ldr r5, ttbrbit_mask and r2, r5 @@ -471,6 +476,25 @@ usettbr0: mcr p15, 0, r4, c1, c0, 0 ldmfd sp!, {r0-r12, pc} @ restore regs and return + +#ifdef CONFIG_HIBERNATION +ENTRY(__save_processor_state) + stmfd sp!, {r0-r12, lr} + mov r1, #0x4 + mov r8, r0 + b l1_logic_lost +ENDPROC(__save_processor_state) + +ENTRY(__restore_processor_state) + stmfd sp!, { r0 - r12, lr } + str sp, [r0] @ fixup saved stack pointer + str lr, [r0, #8] @ fixup saved link register + mov r3, r0 + mov r1, #1 + b .Llogic_l1_restore_internal +ENDPROC(__restore_processor_state) +#endif + save_context_wfi: /*b save_context_wfi*/ @ enable to debug save code mov r8, r0 /* Store SDRAM address in r8 */ @@ -545,6 +569,10 @@ l1_logic_lost: mrc p15, 0, r4, c1, c0, 0 /* save control register */ stmia r8!, {r4} +#ifdef CONFIG_HIBERNATION + cmp r1, #4 + ldmeqfd sp!, {r0-r12, pc} @ early return from __save_processor_state +#endif clean_caches: /* Clean Data or unified cache to POU*/ /* How to invalidate only L1 cache???? - #FIX_ME# */ diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig index df5ce56..b4713ba 100644 --- a/arch/arm/plat-omap/Kconfig +++ b/arch/arm/plat-omap/Kconfig @@ -23,6 +23,7 @@ config ARCH_OMAP3 select CPU_V7 select COMMON_CLKDEV select OMAP_IOMMU + select ARCH_HIBERNATION_POSSIBLE config ARCH_OMAP4 bool "TI OMAP4"