* [PATCH] ARM: Samsung: Select ARM_CPU_SUSPEND when required
@ 2013-04-07 20:27 Sylwester Nawrocki
2013-04-08 9:57 ` Kukjin Kim
0 siblings, 1 reply; 5+ messages in thread
From: Sylwester Nawrocki @ 2013-04-07 20:27 UTC (permalink / raw)
To: linux-arm-kernel
The power management code of S3C24XX, S3C64XX, S5PV210 platform in
arch/arm/plat-samsung/, arch/arm/mach-s3c24xx/, arch/arm/mach-s3c64xx/
directories uses generic cpu_suspend routine. Make sure it is compiled
in when building with power management support. Without this patch
compilation fails with errors as below. It can be reproduced by using
default config files with CONFIG_SUSPEND disabled and CONFIG_PM_RUNTIME
enabled.
- s5pv210_defconfig
arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
arch/arm/plat-samsung/pm.c:304: undefined reference to `cpu_suspend'
arch/arm/plat-samsung/built-in.o: In function `s3c_cpu_resume':
arch/arm/plat-samsung/s5p-sleep.S:74: undefined reference to `cpu_resume'
- s3c24xx_defconfig
arch/arm/mach-s3c24xx/built-in.o: In function `s3c_cpu_resume':
arch/arm/mach-s3c24xx/sleep.S:83: undefined reference to `cpu_resume'
arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
arch/arm/plat-samsung/pm.c:304: undefined reference to `cpu_suspend'
- s3c64xx_defconfig
arch/arm/mach-s3c64xx/built-in.o: In function `s3c_cpu_resume':
arch/arm/mach-s3c64xx/sleep.S:72: undefined reference to `cpu_resume'
arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
arch/arm/plat-samsung/pm.c:304: undefined reference to `cpu_suspend'
To fix this issue select ARM_CPU_SUSPEND for PLAT_SAMSUNG when PM is set.
The build break occurs for kernels at least back to v3.0, however this
patch applies without conflicts only back to v3.7.
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung•com>
Cc: stable at vger.kernel.org
---
arch/arm/plat-samsung/Kconfig | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
index 54d1861..02355ba 100644
--- a/arch/arm/plat-samsung/Kconfig
+++ b/arch/arm/plat-samsung/Kconfig
@@ -10,6 +10,7 @@ config PLAT_SAMSUNG
default y
select GENERIC_IRQ_CHIP
select NO_IOPORT
+ select ARM_CPU_SUSPEND if PM
help
Base platform code for all Samsung SoC based systems
--
1.7.4.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] ARM: Samsung: Select ARM_CPU_SUSPEND when required
2013-04-07 20:27 [PATCH] ARM: Samsung: Select ARM_CPU_SUSPEND when required Sylwester Nawrocki
@ 2013-04-08 9:57 ` Kukjin Kim
2013-04-08 10:27 ` Sylwester Nawrocki
0 siblings, 1 reply; 5+ messages in thread
From: Kukjin Kim @ 2013-04-08 9:57 UTC (permalink / raw)
To: linux-arm-kernel
Sylwester Nawrocki wrote:
>
> The power management code of S3C24XX, S3C64XX, S5PV210 platform in
> arch/arm/plat-samsung/, arch/arm/mach-s3c24xx/, arch/arm/mach-s3c64xx/
> directories uses generic cpu_suspend routine. Make sure it is compiled
> in when building with power management support. Without this patch
> compilation fails with errors as below. It can be reproduced by using
> default config files with CONFIG_SUSPEND disabled and CONFIG_PM_RUNTIME
> enabled.
>
> - s5pv210_defconfig
> arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
> arch/arm/plat-samsung/pm.c:304: undefined reference to `cpu_suspend'
> arch/arm/plat-samsung/built-in.o: In function `s3c_cpu_resume':
> arch/arm/plat-samsung/s5p-sleep.S:74: undefined reference to `cpu_resume'
>
> - s3c24xx_defconfig
> arch/arm/mach-s3c24xx/built-in.o: In function `s3c_cpu_resume':
> arch/arm/mach-s3c24xx/sleep.S:83: undefined reference to `cpu_resume'
> arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
> arch/arm/plat-samsung/pm.c:304: undefined reference to `cpu_suspend'
>
> - s3c64xx_defconfig
> arch/arm/mach-s3c64xx/built-in.o: In function `s3c_cpu_resume':
> arch/arm/mach-s3c64xx/sleep.S:72: undefined reference to `cpu_resume'
> arch/arm/plat-samsung/built-in.o: In function `s3c_pm_enter':
> arch/arm/plat-samsung/pm.c:304: undefined reference to `cpu_suspend'
>
> To fix this issue select ARM_CPU_SUSPEND for PLAT_SAMSUNG when PM is set.
>
> The build break occurs for kernels at least back to v3.0, however this
> patch applies without conflicts only back to v3.7.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung•com>
> Cc: stable at vger.kernel.org
> ---
> arch/arm/plat-samsung/Kconfig | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
> index 54d1861..02355ba 100644
> --- a/arch/arm/plat-samsung/Kconfig
> +++ b/arch/arm/plat-samsung/Kconfig
> @@ -10,6 +10,7 @@ config PLAT_SAMSUNG
> default y
> select GENERIC_IRQ_CHIP
> select NO_IOPORT
> + select ARM_CPU_SUSPEND if PM
> help
> Base platform code for all Samsung SoC based systems
>
> --
> 1.7.4.1
Yes, right. The pm.c in plat-samsung should be built with
arch/arm/kernel/sleep.S and suspend.c.
BTW it should be shown in alphabetical order and we don't need more
following in mach-exynos.
--------8<----------------8<--------
diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index 2f45906..bc0a8b2 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -31,7 +31,6 @@ config CPU_EXYNOS4210
bool "SAMSUNG EXYNOS4210"
default y
depends on ARCH_EXYNOS4
- select ARM_CPU_SUSPEND if PM
select PM_GENERIC_DOMAINS
select S5P_PM if PM
select S5P_SLEEP if PM
diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
index b708b3e..30a976d 100644
--- a/arch/arm/plat-samsung/Kconfig
+++ b/arch/arm/plat-samsung/Kconfig
@@ -8,6 +8,7 @@ config PLAT_SAMSUNG
Bool
depends on PLAT_S3C24XX || ARCH_S3C64XX || PLAT_S5P
default y
+ select ARM_CPU_SUSPEND if PM
select GENERIC_IRQ_CHIP
select NO_IOPORT
help
--------8<----------------8<--------
If you have any objections, let me know.
Thanks.
- Kukjin
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] ARM: Samsung: Select ARM_CPU_SUSPEND when required
2013-04-08 9:57 ` Kukjin Kim
@ 2013-04-08 10:27 ` Sylwester Nawrocki
2013-04-08 11:00 ` Russell King - ARM Linux
0 siblings, 1 reply; 5+ messages in thread
From: Sylwester Nawrocki @ 2013-04-08 10:27 UTC (permalink / raw)
To: linux-arm-kernel
On 04/08/2013 11:57 AM, Kukjin Kim wrote:
> Sylwester Nawrocki wrote:
[...]
> Yes, right. The pm.c in plat-samsung should be built with
> arch/arm/kernel/sleep.S and suspend.c.
>
> BTW it should be shown in alphabetical order and we don't need more
> following in mach-exynos.
>
> --------8<----------------8<--------
> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
> index 2f45906..bc0a8b2 100644
> --- a/arch/arm/mach-exynos/Kconfig
> +++ b/arch/arm/mach-exynos/Kconfig
> @@ -31,7 +31,6 @@ config CPU_EXYNOS4210
> bool "SAMSUNG EXYNOS4210"
> default y
> depends on ARCH_EXYNOS4
> - select ARM_CPU_SUSPEND if PM
> select PM_GENERIC_DOMAINS
> select S5P_PM if PM
> select S5P_SLEEP if PM
> diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
> index b708b3e..30a976d 100644
> --- a/arch/arm/plat-samsung/Kconfig
> +++ b/arch/arm/plat-samsung/Kconfig
> @@ -8,6 +8,7 @@ config PLAT_SAMSUNG
> Bool
> depends on PLAT_S3C24XX || ARCH_S3C64XX || PLAT_S5P
> default y
> + select ARM_CPU_SUSPEND if PM
> select GENERIC_IRQ_CHIP
> select NO_IOPORT
> help
> --------8<----------------8<--------
>
> If you have any objections, let me know.
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.
Thanks,
Sylwester
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] ARM: Samsung: Select ARM_CPU_SUSPEND when required
2013-04-08 10:27 ` Sylwester Nawrocki
@ 2013-04-08 11:00 ` Russell King - ARM Linux
2013-04-08 15:05 ` Sylwester Nawrocki
0 siblings, 1 reply; 5+ messages in thread
From: Russell King - ARM Linux @ 2013-04-08 11:00 UTC (permalink / raw)
To: linux-arm-kernel
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 <arnd@arndb•de>
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 <arnd@arndb•de>
...
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 <rmk+kernel@arm•linux.org.uk>
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: <stable@vger•kernel.org> # kernels containing 15e0d9e37c7fe or equivalent
Signed-off-by: Russell King <rmk+kernel@arm•linux.org.uk>
---
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
--
1.7.4.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] ARM: Samsung: Select ARM_CPU_SUSPEND when required
2013-04-08 11:00 ` Russell King - ARM Linux
@ 2013-04-08 15:05 ` Sylwester Nawrocki
0 siblings, 0 replies; 5+ messages in thread
From: Sylwester Nawrocki @ 2013-04-08 15:05 UTC (permalink / raw)
To: linux-arm-kernel
On 04/08/2013 01:00 PM, Russell King - ARM Linux wrote:
> On Mon, Apr 08, 2013 at 12:27:34PM +0200, Sylwester Nawrocki wrote:
>> On 04/08/2013 11:57 AM, Kukjin Kim wrote:
[...]
>
> 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
Yes, presumably this needs to be cleaned up. I was considering replacing
CONFIG_PM with CONFIG_PM_SLEEP, but that might involve some not trivial
changes. I'm going to have a look at it eventually.
> 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 <arnd@arndb•de>
> 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 <arnd@arndb•de>
>
> ...
> 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:
Thanks, that seems a most reasonable fix. I was considering something like
that, just was afraid a bit to do this wide change and that it might cause
some other issues. Anyway that looks fine.
> 8<===
> From: Russell King <rmk+kernel@arm•linux.org.uk>
> 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: <stable@vger•kernel.org> # kernels containing 15e0d9e37c7fe or equivalent
> Signed-off-by: Russell King <rmk+kernel@arm•linux.org.uk>
> ---
> 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
[...]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-04-08 15:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-07 20:27 [PATCH] ARM: Samsung: Select ARM_CPU_SUSPEND when required Sylwester Nawrocki
2013-04-08 9:57 ` Kukjin Kim
2013-04-08 10:27 ` Sylwester Nawrocki
2013-04-08 11:00 ` Russell King - ARM Linux
2013-04-08 15:05 ` Sylwester Nawrocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox