From patchwork Mon Apr 8 11:00:32 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King - ARM Linux X-Patchwork-Id: 2407731 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) by patchwork1.kernel.org (Postfix) with ESMTP id BCD173FC71 for ; Mon, 8 Apr 2013 11:04:27 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UP9rw-0005ZU-Mv; Mon, 08 Apr 2013 11:04:13 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1UP9rl-0001D6-RO; Mon, 08 Apr 2013 11:04:01 +0000 Received: from caramon.arm.linux.org.uk ([78.32.30.218]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UP9ri-00017y-Jy for linux-arm-kernel@lists.infradead.org; Mon, 08 Apr 2013 11:04:00 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=caramon; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=/0y2QduOzULCKyiRLuu/aEAqUnSykIaNU2awtj/xc5A=; b=TWnCyhhNYLxK/OM6hKqy8MoWXqDtK2V8HrmSSr6uO7tYu1PGefj9Ks4Un7PPKA126ytk3euNiYtmZxadDjSP/9KRq0lQFnjZv4IohLNIn3qSrplbSz5/XdRTHOz3IG27RrzKvO+xRFq+JcnSCxL/xrjQsk+mSdHLeZpA/fx6ggU=; Received: from n2100.arm.linux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:50186) by caramon.arm.linux.org.uk with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.76) (envelope-from ) id 1UP9oQ-00032X-Cu; Mon, 08 Apr 2013 12:00:35 +0100 Received: from linux by n2100.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1UP9oO-00057D-Qm; Mon, 08 Apr 2013 12:00:32 +0100 Date: Mon, 8 Apr 2013 12:00:32 +0100 From: Russell King - ARM Linux To: Sylwester Nawrocki Subject: Re: [PATCH] ARM: Samsung: Select ARM_CPU_SUSPEND when required Message-ID: <20130408110032.GK17995@n2100.arm.linux.org.uk> References: <1365366442-16025-1-git-send-email-s.nawrocki@samsung.com> <2cd601ce343f$7d6aac50$784004f0$%kim@samsung.com> <51629B96.9050504@samsung.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <51629B96.9050504@samsung.com> User-Agent: Mutt/1.5.19 (2009-01-05) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130408_070359_206768_239FEAC1 X-CRM114-Status: GOOD ( 25.66 ) X-Spam-Score: -6.7 (------) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-6.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [78.32.30.218 listed in list.dnswl.org] -2.4 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature Cc: linux-samsung-soc@vger.kernel.org, Kukjin Kim , linux-arm-kernel@lists.infradead.org, stable@vger.kernel.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On Mon, Apr 08, 2013 at 12:27:34PM +0200, Sylwester Nawrocki wrote: > On 04/08/2013 11:57 AM, Kukjin Kim wrote: > Yes, this looks better. However after posting this patch I noticed linker > errors in some builds due to undefined cpu_arm920_do_suspend, > cpu_arm920_do_resume routines. > > It seems it is because various cpu_*_do_suspend routines are selected by > CONFIG_PM_SLEEP. And the PM code in arch/arm/mach-s3c24xx, arch/arm/mach- > s3c64xx is selected by CONFIG_PM. > > $ git grep -1 "ENTRY(cpu_.*_do_suspend" > arch/arm/mm/proc-arm920.S-#ifdef CONFIG_PM_SLEEP > arch/arm/mm/proc-arm920.S:ENTRY(cpu_arm920_do_suspend) > arch/arm/mm/proc-arm920.S- stmfd sp!, {r4 - r6, lr} > -- > arch/arm/mm/proc-arm926.S-#ifdef CONFIG_PM_SLEEP > arch/arm/mm/proc-arm926.S:ENTRY(cpu_arm926_do_suspend) > arch/arm/mm/proc-arm926.S- stmfd sp!, {r4 - r6, lr} > -- > arch/arm/mm/proc-mohawk.S-#ifdef CONFIG_PM_SLEEP > arch/arm/mm/proc-mohawk.S:ENTRY(cpu_mohawk_do_suspend) > arch/arm/mm/proc-mohawk.S- stmfd sp!, {r4 - r9, lr} > -- > arch/arm/mm/proc-sa1100.S-#ifdef CONFIG_PM_SLEEP > arch/arm/mm/proc-sa1100.S:ENTRY(cpu_sa1100_do_suspend) > arch/arm/mm/proc-sa1100.S- stmfd sp!, {r4 - r6, lr} > -- > arch/arm/mm/proc-v6.S-#ifdef CONFIG_PM_SLEEP > arch/arm/mm/proc-v6.S:ENTRY(cpu_v6_do_suspend) > arch/arm/mm/proc-v6.S- stmfd sp!, {r4 - r9, lr} > -- > arch/arm/mm/proc-v7.S-#ifdef CONFIG_ARM_CPU_SUSPEND > arch/arm/mm/proc-v7.S:ENTRY(cpu_v7_do_suspend) > arch/arm/mm/proc-v7.S- stmfd sp!, {r4 - r10, lr} > -- > arch/arm/mm/proc-xsc3.S-#ifdef CONFIG_PM_SLEEP > arch/arm/mm/proc-xsc3.S:ENTRY(cpu_xsc3_do_suspend) > arch/arm/mm/proc-xsc3.S- stmfd sp!, {r4 - r9, lr} > -- > arch/arm/mm/proc-xscale.S-#ifdef CONFIG_PM_SLEEP > arch/arm/mm/proc-xscale.S:ENTRY(cpu_xscale_do_suspend) > arch/arm/mm/proc-xscale.S- stmfd sp!, {r4 - r9, lr} > > However I can't reproduce it now :-/ Anyway the $subject patch fixes > the main issue, which I can easily reproduce here as well. So I'll > prepare another patch if needed when I get back to this later. Sigh. This stuff looks rather screwed up now: $ grep -B1 'ENTRY.*do_suspend' arch/arm/mm/proc*.S arch/arm/mm/proc-arm920.S-#ifdef CONFIG_PM_SLEEP arch/arm/mm/proc-arm920.S:ENTRY(cpu_arm920_do_suspend) --- arch/arm/mm/proc-arm926.S-#ifdef CONFIG_PM_SLEEP arch/arm/mm/proc-arm926.S:ENTRY(cpu_arm926_do_suspend) -- arch/arm/mm/proc-mohawk.S-#ifdef CONFIG_PM_SLEEP arch/arm/mm/proc-mohawk.S:ENTRY(cpu_mohawk_do_suspend) -- arch/arm/mm/proc-sa1100.S-#ifdef CONFIG_PM_SLEEP arch/arm/mm/proc-sa1100.S:ENTRY(cpu_sa1100_do_suspend) -- arch/arm/mm/proc-v6.S-#ifdef CONFIG_PM_SLEEP arch/arm/mm/proc-v6.S:ENTRY(cpu_v6_do_suspend) -- arch/arm/mm/proc-v7.S-#ifdef CONFIG_ARM_CPU_SUSPEND arch/arm/mm/proc-v7.S:ENTRY(cpu_v7_do_suspend) -- arch/arm/mm/proc-xsc3.S-#ifdef CONFIG_PM_SLEEP arch/arm/mm/proc-xsc3.S:ENTRY(cpu_xsc3_do_suspend) -- arch/arm/mm/proc-xscale.S-#ifdef CONFIG_PM_SLEEP arch/arm/mm/proc-xscale.S:ENTRY(cpu_xscale_do_suspend) Now, CONFIG_PM_SLEEP is fine if this stuff only ever gets used when PM_SLEEP is enabled - that's what it was designed for in the first place. However, as we can see from the earlier patches in this thread, the cpu_suspend stuff is being selected when PM is enabled (which is arguably wrong), and also in some cases when CPU_IDLE is enabled. Therefore, making this code depend on ARM_CPU_SUSPEND seems sensible. However, So, how did proc-v7.S and only that file end up doing something different? commit 15e0d9e37c7fe9711b60f47221c394d45553ad8c Author: Arnd Bergmann Date: Sat Oct 1 21:09:39 2011 +0200 ARM: pm: let platforms select cpu_suspend support Support for the cpu_suspend functions is only built-in when CONFIG_PM_SLEEP is enabled, but omap3/4, exynos4 and pxa always call cpu_suspend when CONFIG_PM is enabled. Signed-off-by: Arnd Bergmann ... diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S index a30e785..591accd 100644 --- a/arch/arm/mm/proc-v7.S +++ b/arch/arm/mm/proc-v7.S @@ -217,7 +217,7 @@ ENDPROC(cpu_v7_set_pte_ext) /* Suspend/resume support: derived from arch/arm/mach-s5pv210/sleep.S */ .globl cpu_v7_suspend_size .equ cpu_v7_suspend_size, 4 * 9 -#ifdef CONFIG_PM_SLEEP +#ifdef CONFIG_ARM_CPU_SUSPEND ENTRY(cpu_v7_do_suspend) ... As far as this commit goes, it looks sane at the time that it was written, but as soon as we have *any* other selections of ARM_CPU_SUSPEND, the whole idea becomes extremely fragile - hence the reason for your build errors. Moreover, with the above commit, there is _no_ sense what so ever in not applying the same change to all proc-*.S files, thereby entirely avoiding this fragility. I would argue that the original commit should have made the same change to _all_ proc-*.S files. Let's do the job properly - hence this is now queued for -rc: 8<=== From: Russell King Subject: [PATCH] ARM: Do 15e0d9e37c (ARM: pm: let platforms select cpu_suspend support) properly Let's do the changes properly and fix the same problem everywhere, not just for one case. Cc: # kernels containing 15e0d9e37c7fe or equivalent Signed-off-by: Russell King --- arch/arm/mm/proc-arm920.S | 2 +- arch/arm/mm/proc-arm926.S | 2 +- arch/arm/mm/proc-mohawk.S | 2 +- arch/arm/mm/proc-sa1100.S | 2 +- arch/arm/mm/proc-v6.S | 2 +- arch/arm/mm/proc-xsc3.S | 2 +- arch/arm/mm/proc-xscale.S | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/arch/arm/mm/proc-arm920.S b/arch/arm/mm/proc-arm920.S index 2c3b942..2556cf1 100644 --- a/arch/arm/mm/proc-arm920.S +++ b/arch/arm/mm/proc-arm920.S @@ -387,7 +387,7 @@ ENTRY(cpu_arm920_set_pte_ext) /* Suspend/resume support: taken from arch/arm/plat-s3c24xx/sleep.S */ .globl cpu_arm920_suspend_size .equ cpu_arm920_suspend_size, 4 * 3 -#ifdef CONFIG_PM_SLEEP +#ifdef CONFIG_ARM_CPU_SUSPEND ENTRY(cpu_arm920_do_suspend) stmfd sp!, {r4 - r6, lr} mrc p15, 0, r4, c13, c0, 0 @ PID diff --git a/arch/arm/mm/proc-arm926.S b/arch/arm/mm/proc-arm926.S index f1803f7..344c8a5 100644 --- a/arch/arm/mm/proc-arm926.S +++ b/arch/arm/mm/proc-arm926.S @@ -402,7 +402,7 @@ ENTRY(cpu_arm926_set_pte_ext) /* Suspend/resume support: taken from arch/arm/plat-s3c24xx/sleep.S */ .globl cpu_arm926_suspend_size .equ cpu_arm926_suspend_size, 4 * 3 -#ifdef CONFIG_PM_SLEEP +#ifdef CONFIG_ARM_CPU_SUSPEND ENTRY(cpu_arm926_do_suspend) stmfd sp!, {r4 - r6, lr} mrc p15, 0, r4, c13, c0, 0 @ PID diff --git a/arch/arm/mm/proc-mohawk.S b/arch/arm/mm/proc-mohawk.S index 82f9cdc..0b60dd3 100644 --- a/arch/arm/mm/proc-mohawk.S +++ b/arch/arm/mm/proc-mohawk.S @@ -350,7 +350,7 @@ ENTRY(cpu_mohawk_set_pte_ext) .globl cpu_mohawk_suspend_size .equ cpu_mohawk_suspend_size, 4 * 6 -#ifdef CONFIG_PM_SLEEP +#ifdef CONFIG_ARM_CPU_SUSPEND ENTRY(cpu_mohawk_do_suspend) stmfd sp!, {r4 - r9, lr} mrc p14, 0, r4, c6, c0, 0 @ clock configuration, for turbo mode diff --git a/arch/arm/mm/proc-sa1100.S b/arch/arm/mm/proc-sa1100.S index 3aa0da1..d92dfd0 100644 --- a/arch/arm/mm/proc-sa1100.S +++ b/arch/arm/mm/proc-sa1100.S @@ -172,7 +172,7 @@ ENTRY(cpu_sa1100_set_pte_ext) .globl cpu_sa1100_suspend_size .equ cpu_sa1100_suspend_size, 4 * 3 -#ifdef CONFIG_PM_SLEEP +#ifdef CONFIG_ARM_CPU_SUSPEND ENTRY(cpu_sa1100_do_suspend) stmfd sp!, {r4 - r6, lr} mrc p15, 0, r4, c3, c0, 0 @ domain ID diff --git a/arch/arm/mm/proc-v6.S b/arch/arm/mm/proc-v6.S index bcaaa8d..5c07ee4 100644 --- a/arch/arm/mm/proc-v6.S +++ b/arch/arm/mm/proc-v6.S @@ -138,7 +138,7 @@ ENTRY(cpu_v6_set_pte_ext) /* Suspend/resume support: taken from arch/arm/mach-s3c64xx/sleep.S */ .globl cpu_v6_suspend_size .equ cpu_v6_suspend_size, 4 * 6 -#ifdef CONFIG_PM_SLEEP +#ifdef CONFIG_ARM_CPU_SUSPEND ENTRY(cpu_v6_do_suspend) stmfd sp!, {r4 - r9, lr} mrc p15, 0, r4, c13, c0, 0 @ FCSE/PID diff --git a/arch/arm/mm/proc-xsc3.S b/arch/arm/mm/proc-xsc3.S index eb93d64..e8efd83 100644 --- a/arch/arm/mm/proc-xsc3.S +++ b/arch/arm/mm/proc-xsc3.S @@ -413,7 +413,7 @@ ENTRY(cpu_xsc3_set_pte_ext) .globl cpu_xsc3_suspend_size .equ cpu_xsc3_suspend_size, 4 * 6 -#ifdef CONFIG_PM_SLEEP +#ifdef CONFIG_ARM_CPU_SUSPEND ENTRY(cpu_xsc3_do_suspend) stmfd sp!, {r4 - r9, lr} mrc p14, 0, r4, c6, c0, 0 @ clock configuration, for turbo mode diff --git a/arch/arm/mm/proc-xscale.S b/arch/arm/mm/proc-xscale.S index 2551036..e766f88 100644 --- a/arch/arm/mm/proc-xscale.S +++ b/arch/arm/mm/proc-xscale.S @@ -528,7 +528,7 @@ ENTRY(cpu_xscale_set_pte_ext) .globl cpu_xscale_suspend_size .equ cpu_xscale_suspend_size, 4 * 6 -#ifdef CONFIG_PM_SLEEP +#ifdef CONFIG_ARM_CPU_SUSPEND ENTRY(cpu_xscale_do_suspend) stmfd sp!, {r4 - r9, lr} mrc p14, 0, r4, c6, c0, 0 @ clock configuration, for turbo mode