arm64: kernel: ARM64_CPU_SUSPEND clean-up
v3 cover arm64: kernel: ARM64_CPU_SUSPEND clean-up
v2 => v3
- Fixed defconfig compilation issue by moving the cpu_suspend interface to arch/arm64/kernel/cpuidle.c
- Added cover letter description/reason for the clean-up
- Rebased against 3.19-rc6
v2:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/311468.html
v1 => v2
- Rebased on top of
commit e5e62d475274 ("arm64: psci: Fix build breakage without PM_SLEEP")
The arm64 config option ARM64_CPU_SUSPEND was introduced so that code required to save/restore CPU context could be selectively compiled in the kernel.
Since ARM64_CPU_SUSPEND selected code is needed only if CONFIG_SUSPEND or CONFIG_CPU_IDLE are selected in turn, and the CPU_PM config option already represents those config cases, this patch removes the ARM64_CPU_SUSPEND config option and reshuffles the code so that it is compiled by the kernel subsystem only when it is needed.
On arm64 systems, the current arch API used by the CPUidle driver is:
cpu_suspend(index)
where index corresponds to the idle state index that should be entered upon cpu_suspend() call. Since the cpu_suspend() function is the CPUidle interface to arch code, it should be compiled in only if CPU_IDLE is enabled, so the respective code is moved to the CPUidle specific compilation unit. Since the ARM64_CPU_SUSPEND is removed, and it was used to selectively compile CPU operations like cpu_suspend() (which in turn represents the glue code used by the cpu_suspend() interface), the preprocessor macros should be changed and updated to the current usage, which means that the cpu_suspend CPU operation is compiled in only if CPU_IDLE is enabled in the kernel.
A subsequent patch series will rename the cpu_suspend function, the cpu_suspend CPU operations hook and the functions implementing save/restore code so that the naming scheme would become more coherent and the CPUidle arch interfaces for arm and arm64 kernel could follow the same naming scheme, when it is settled in stone.
Lorenzo Pieralisi (1):
arm64: kernel: remove ARM64_CPU_SUSPEND config option
arch/arm64/Kconfig | 3 --- arch/arm64/include/asm/cpu_ops.h | 8 ++++---- arch/arm64/include/asm/cpuidle.h | 6 ++++++ arch/arm64/include/asm/suspend.h | 2 -- arch/arm64/kernel/Makefile | 2 +- arch/arm64/kernel/asm-offsets.c | 2 +- arch/arm64/kernel/cpuidle.c | 20 ++++++++++++++++++++ arch/arm64/kernel/hw_breakpoint.c | 2 +- arch/arm64/kernel/psci.c | 2 -- arch/arm64/kernel/suspend.c | 21 --------------------- arch/arm64/mm/proc.S | 2 +- drivers/cpuidle/Kconfig.arm64 | 1 - drivers/cpuidle/cpuidle-arm64.c | 1 - 13 files changed, 34 insertions(+), 38 deletions(-)
Signature · 1 line
2.2.1
PATCH v3 1/1 arm64: kernel: remove ARM64_CPU_SUSPEND config option
ARM64_CPU_SUSPEND config option was introduced to make code providing context save/restore selectable only on platforms requiring power management capabilities.
Currently ARM64_CPU_SUSPEND depends on the PM_SLEEP config option which in turn is set by the SUSPEND config option.
The introduction of CPU_IDLE for arm64 requires that code configured by ARM64_CPU_SUSPEND (context save/restore) should be compiled in in order to enable the CPU idle driver to rely on CPU operations carrying out context save/restore.
The ARM64_CPUIDLE config option (ARM64 generic idle driver) is therefore forced to select ARM64_CPU_SUSPEND, even if there may be (ie PM_SLEEP) failed dependencies, which is not a clean way of handling the kernel configuration option.
For these reasons, this patch removes the ARM64_CPU_SUSPEND config option and makes the context save/restore dependent on CPU_PM, which is selected whenever either SUSPEND or CPU_IDLE are configured, cleaning up dependencies in the process.
This way, code previously configured through ARM64_CPU_SUSPEND is compiled in whenever a power management subsystem requires it to be present in the kernel (SUSPEND || CPU_IDLE), which is the behaviour expected on ARM64 kernels.
The cpu_suspend and cpu_init_idle CPU operations are added only if CPU_IDLE is selected, since they are CPU_IDLE specific methods and should be grouped and defined accordingly.
PSCI CPU operations are updated to reflect the introduced changes.
--- arch/arm64/Kconfig | 3 --- arch/arm64/include/asm/cpu_ops.h | 8 ++++---- arch/arm64/include/asm/cpuidle.h | 6 ++++++ arch/arm64/include/asm/suspend.h | 2 -- arch/arm64/kernel/Makefile | 2 +- arch/arm64/kernel/asm-offsets.c | 2 +- arch/arm64/kernel/cpuidle.c | 20 ++++++++++++++++++++ arch/arm64/kernel/hw_breakpoint.c | 2 +- arch/arm64/kernel/psci.c | 2 -- arch/arm64/kernel/suspend.c | 21 --------------------- arch/arm64/mm/proc.S | 2 +- drivers/cpuidle/Kconfig.arm64 | 1 - drivers/cpuidle/cpuidle-arm64.c | 1 - 13 files changed, 34 insertions(+), 38 deletions(-)
arch/arm64/Kconfig +0 -3
@@ -627,9 +627,6 @@ source "kernel/power/Kconfig" config ARCH_SUSPEND_POSSIBLE def_bool y -config ARM64_CPU_SUSPEND - def_bool PM_SLEEP - endmenu menu "CPU Power Management"
arch/arm64/include/asm/cpu_ops.h +4 -4
@@ -28,8 +28,6 @@ struct device_node;* enable-method property. * @cpu_init: Reads any data necessary for a specific enable-method from the * devicetree, for a given cpu node and proposed logical id. -* @cpu_init_idle: Reads any data necessary to initialize CPU idle states from -* devicetree, for a given cpu node and proposed logical id. * @cpu_prepare: Early one-time preparation step for a cpu. If there is a * mechanism for doing so, tests whether it is possible to boot * the given CPU.
@@ -42,6 +40,8 @@ struct device_node;* @cpu_die: Makes a cpu leave the kernel. Must not fail. Called from the * cpu being killed. * @cpu_kill: Ensures a cpu has left the kernel. Called from another cpu. +* @cpu_init_idle: Reads any data necessary to initialize CPU idle states from +* devicetree, for a given cpu node and proposed logical id. * @cpu_suspend: Suspends a cpu and saves the required context. May fail owing * to wrong parameters or error conditions. Called from the * CPU being suspended. Must be called with IRQs disabled.
@@ -49,7 +49,6 @@ struct device_node;struct cpu_operations {const char *name;int (*cpu_init)(struct device_node *,unsigned int ); -int (*cpu_init_idle)(struct device_node *,unsigned int );int (*cpu_prepare)(unsigned int );int (*cpu_boot)(unsigned int );void (*cpu_postboot)(void );
@@ -58,7 +57,8 @@ struct cpu_operations {void (*cpu_die)(unsigned int cpu);int (*cpu_kill)(unsigned int cpu);#endif -#ifdef CONFIG_ARM64_CPU_SUSPEND +#ifdef CONFIG_CPU_IDLE +int (*cpu_init_idle)(struct device_node *,unsigned int );int (*cpu_suspend)(unsigned long );#endif };
arch/arm64/include/asm/cpuidle.h +6 -0
@@ -3,11 +3,17 @@#ifdef CONFIG_CPU_IDLE extern int cpu_init_idle (unsigned int cpu); +extern int cpu_suspend (unsigned long arg);#else static inline int cpu_init_idle (unsigned int cpu) {return -EOPNOTSUPP ; } + +static inline int cpu_suspend (unsigned long arg) +{ +return -EOPNOTSUPP ; +}#endif #endif
arch/arm64/include/asm/suspend.h +0 -2
@@ -23,6 +23,4 @@ struct sleep_save_sp {extern int __cpu_suspend (unsigned long arg,int (*fn)(unsigned long ));extern void cpu_resume (void ); -extern int cpu_suspend (unsigned long ); -#endif
arch/arm64/kernel/Makefile +1 -1
@@ -27,7 +27,7 @@ arm64-obj-$(CONFIG_SMP) += smp.o smp_spin_table.o topology.o arm64-obj-$(CONFIG_PERF_EVENTS) += perf_regs.o arm64-obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o arm64-obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o -arm64-obj-$(CONFIG_ARM64_CPU_SUSPEND) += sleep.o suspend.o +arm64-obj-$(CONFIG_CPU_PM) += sleep.o suspend.o arm64-obj-$(CONFIG_CPU_IDLE) += cpuidle.o arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o arm64-obj-$(CONFIG_KGDB) += kgdb.o
arch/arm64/kernel/asm-offsets.c +1 -1
@@ -152,7 +152,7 @@ int main(void) DEFINE(KVM_VTTBR, offsetof(struct kvm, arch.vttbr)); DEFINE(KVM_VGIC_VCTRL, offsetof(struct kvm, arch.vgic.vctrl_base));#endif -#ifdef CONFIG_ARM64_CPU_SUSPEND +#ifdef CONFIG_CPU_PM DEFINE (CPU_SUSPEND_SZ ,sizeof (struct cpu_suspend_ctx ));DEFINE (CPU_CTX_SP , offsetof(struct cpu_suspend_ctx , sp));DEFINE (MPIDR_HASH_MASK , offsetof(struct mpidr_hash , mask));
arch/arm64/kernel/cpuidle.c +20 -0
@@ -29,3 +29,23 @@ int cpu_init_idle(unsigned int cpu)of_node_put (cpu_node);return ret; } + +/** +* cpu_suspend() - function to enter a low-power idle state +* @arg: argument to pass to CPU suspend operations +* +* Return: 0 on success, -EOPNOTSUPP if CPU suspend hook not initialized, CPU +* operations back-end error code otherwise. +*/ +int cpu_suspend (unsigned long arg) +{ +int cpu =smp_processor_id (); + +/* +* If cpu_ops have not been registered or suspend +* has not been initialized, cpu_suspend call fails early. +*/ +if (!cpu_ops[cpu] || !cpu_ops[cpu]->cpu_suspend) +return -EOPNOTSUPP ; +return cpu_ops[cpu]->cpu_suspend (arg); +}
arch/arm64/kernel/hw_breakpoint.c +1 -1
@@ -894,7 +894,7 @@ static struct notifier_block hw_breakpoint_reset_nb = { .notifier_call = hw_breakpoint_reset_notify, }; -#ifdef CONFIG_ARM64_CPU_SUSPEND +#ifdef CONFIG_CPU_PM extern void cpu_suspend_set_dbg_restorer (void (*hw_bp_restore)(void *));#else static inline void cpu_suspend_set_dbg_restorer (void (*hw_bp_restore)(void *))
arch/arm64/kernel/psci.c +0 -2
@@ -540,8 +540,6 @@ const struct cpu_operations cpu_psci_ops = { .name = "psci",#ifdef CONFIG_CPU_IDLE .cpu_init_idle = cpu_psci_cpu_init_idle, -#endif -#ifdef CONFIG_ARM64_CPU_SUSPEND .cpu_suspend = cpu_psci_cpu_suspend,#endif #ifdef CONFIG_SMP
arch/arm64/kernel/suspend.c +0 -21
@@ -1,7 +1,6 @@#include <linux/percpu.h> #include <linux/slab.h> #include <asm/cacheflush.h> -#include <asm/cpu_ops.h> #include <asm/debug-monitors.h> #include <asm/pgtable.h> #include <asm/memory.h>
@@ -51,26 +50,6 @@ void __init cpu_suspend_set_dbg_restorer(void (*hw_bp_restore)(void *)) hw_breakpoint_restore = hw_bp_restore; } -/** -* cpu_suspend() - function to enter a low-power state -* @arg: argument to pass to CPU suspend operations -* -* Return: 0 on success, -EOPNOTSUPP if CPU suspend hook not initialized, CPU -* operations back-end error code otherwise. -*/ -int cpu_suspend (unsigned long arg) -{ -int cpu =smp_processor_id (); - -/* -* If cpu_ops have not been registered or suspend -* has not been initialized, cpu_suspend call fails early. -*/ -if (!cpu_ops[cpu] || !cpu_ops[cpu]->cpu_suspend) -return -EOPNOTSUPP ; -return cpu_ops[cpu]->cpu_suspend (arg); -} -/* * __cpu_suspend *
arch/arm64/mm/proc.S +1 -1
@@ -102,7 +102,7 @@ ENTRY(cpu_do_idle) ret ENDPROC(cpu_do_idle) -#ifdef CONFIG_ARM64_CPU_SUSPEND +#ifdef CONFIG_CPU_PM /** * cpu_do_suspend - save CPU registers context *
drivers/cpuidle/Kconfig.arm64 +0 -1
@@ -4,7 +4,6 @@ config ARM64_CPUIDLE bool "Generic ARM64 CPU idle Driver" - select ARM64_CPU_SUSPEND select DT_IDLE_STATES help Select this to enable generic cpuidle driver for ARM64.
drivers/cpuidle/cpuidle-arm64.c +0 -1
@@ -19,7 +19,6 @@#include <linux/of.h> #include <asm/cpuidle.h> -#include <asm/suspend.h> #include "dt_idle_states.h"
Signature · 1 line
2.2.1
PATCH v3 1/1 arm64: kernel: remove ARM64_CPU_SUSPEND config option
Hi,
Quoted 27 lines · Lorenzo Pieralisi
ARM64_CPU_SUSPENDconfig option was introduced to make code providing context save/restore selectable only on platforms requiring power management capabilities.Currently
ARM64_CPU_SUSPENDdepends on thePM_SLEEPconfig option which in turn is set by the SUSPEND config option.The introduction of
CPU_IDLEfor arm64 requires that code configured byARM64_CPU_SUSPEND(context save/restore) should be compiled in in order to enable the CPU idle driver to rely on CPU operations carrying out context save/restore.The
ARM64_CPUIDLEconfig option (ARM64 generic idle driver) is therefore forced to selectARM64_CPU_SUSPEND, even if there may be (iePM_SLEEP) failed dependencies, which is not a clean way of handling the kernel configuration option.For these reasons, this patch removes the
ARM64_CPU_SUSPENDconfig option and makes the context save/restore dependent onCPU_PM, which is selected whenever either SUSPEND orCPU_IDLEare configured, cleaning up dependencies in the process.This way, code previously configured through
ARM64_CPU_SUSPENDis compiled in whenever a power management subsystem requires it to be present in the kernel (SUSPEND ||CPU_IDLE), which is the behaviour expected on ARM64 kernels.
Quoted 5 lines
The
cpu_suspendandcpu_init_idleCPU operations are added only ifCPU_IDLEis selected, since they areCPU_IDLEspecific methods and should be grouped and defined accordingly.PSCI CPU operations are updated to reflect the introduced changes.
cpu_suspend CPU operation is currently CPU_IDLE specific but I wonder whether this is correct in the context of PSCI?
[ On 32-bit ARM PSCI cpu_suspend operation can be used in suspend code-path
(please take a look at arch/arm/mach-highbank/pm.c), on 64-bit ARM PSCI this is not possible currently. ]
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Quoted 265 lines
Signed-off-by Lorenzo Pieralisi <[email protected]>Cc Arnd Bergmann <[email protected]>Cc Will Deacon <[email protected]>Cc Krzysztof Kozlowski <[email protected]>Cc Daniel Lezcano <[email protected]>Cc Catalin Marinas <[email protected]>Cc Mark Rutland <[email protected]>--- arch/arm64/Kconfig | 3 --- arch/arm64/include/asm/cpu_ops.h | 8 ++++---- arch/arm64/include/asm/cpuidle.h | 6 ++++++ arch/arm64/include/asm/suspend.h | 2 -- arch/arm64/kernel/Makefile | 2 +- arch/arm64/kernel/asm-offsets.c | 2 +- arch/arm64/kernel/cpuidle.c | 20 ++++++++++++++++++++ arch/arm64/kernel/hw_breakpoint.c | 2 +- arch/arm64/kernel/psci.c | 2 -- arch/arm64/kernel/suspend.c | 21 --------------------- arch/arm64/mm/proc.S | 2 +- drivers/cpuidle/Kconfig.arm64 | 1 - drivers/cpuidle/cpuidle-arm64.c | 1 - 13 files changed, 34 insertions(+), 38 deletions(-)arch/arm64/Kconfig +0
-3@@ -627,9 +627,6 @@ source "kernel/power/Kconfig" config ARCH_SUSPEND_POSSIBLE def_bool y -config ARM64_CPU_SUSPEND - def_bool PM_SLEEP - endmenu menu "CPU Power Management"arch/arm64/include/asm/cpu_ops.h +4
-4@@ -28,8 +28,6 @@ struct device_node;* enable-method property. * @cpu_init: Reads any data necessary for a specific enable-method from the * devicetree, for a given cpu node and proposed logical id. -* @cpu_init_idle: Reads any data necessary to initialize CPU idle states from -* devicetree, for a given cpu node and proposed logical id. * @cpu_prepare: Early one-time preparation step for a cpu. If there is a * mechanism for doing so, tests whether it is possible to boot * the given CPU. @@ -42,6 +40,8 @@ struct device_node;* @cpu_die: Makes a cpu leave the kernel. Must not fail. Called from the * cpu being killed. * @cpu_kill: Ensures a cpu has left the kernel. Called from another cpu. +* @cpu_init_idle: Reads any data necessary to initialize CPU idle states from +* devicetree, for a given cpu node and proposed logical id. * @cpu_suspend: Suspends a cpu and saves the required context. May fail owing * to wrong parameters or error conditions. Called from the * CPU being suspended. Must be called with IRQs disabled. @@ -49,7 +49,6 @@ struct device_node;struct cpu_operations {const char *name;int (*cpu_init)(struct device_node *,unsigned int ); -int (*cpu_init_idle)(struct device_node *,unsigned int );int (*cpu_prepare)(unsigned int );int (*cpu_boot)(unsigned int );void (*cpu_postboot)(void );@@ -58,7 +57,8 @@ struct cpu_operations {void (*cpu_die)(unsigned int cpu);int (*cpu_kill)(unsigned int cpu);#endif -#ifdef CONFIG_ARM64_CPU_SUSPEND +#ifdef CONFIG_CPU_IDLE +int (*cpu_init_idle)(struct device_node *,unsigned int );int (*cpu_suspend)(unsigned long );#endif };arch/arm64/include/asm/cpuidle.h +6
-0@@ -3,11 +3,17 @@#ifdef CONFIG_CPU_IDLE extern int cpu_init_idle (unsigned int cpu); +extern int cpu_suspend (unsigned long arg);#else static inline int cpu_init_idle (unsigned int cpu) {return -EOPNOTSUPP ; } + +static inline int cpu_suspend (unsigned long arg) +{ +return -EOPNOTSUPP ; +}#endif #endif arch/arm64/include/asm/suspend.h +0
-2@@ -23,6 +23,4 @@ struct sleep_save_sp {extern int __cpu_suspend (unsigned long arg,int (*fn)(unsigned long ));extern void cpu_resume (void ); -extern int cpu_suspend (unsigned long ); -#endif arch/arm64/kernel/Makefile +1
-1@@ -27,7 +27,7 @@ arm64-obj-$(CONFIG_SMP) += smp.o smp_spin_table.o topology.o arm64-obj-$(CONFIG_PERF_EVENTS) += perf_regs.o arm64-obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o arm64-obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o -arm64-obj-$(CONFIG_ARM64_CPU_SUSPEND) += sleep.o suspend.o +arm64-obj-$(CONFIG_CPU_PM) += sleep.o suspend.o arm64-obj-$(CONFIG_CPU_IDLE) += cpuidle.o arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o arm64-obj-$(CONFIG_KGDB) += kgdb.oarch/arm64/kernel/asm-offsets.c +1
-1@@ -152,7 +152,7 @@ int main(void) DEFINE(KVM_VTTBR, offsetof(struct kvm, arch.vttbr)); DEFINE(KVM_VGIC_VCTRL, offsetof(struct kvm, arch.vgic.vctrl_base));#endif -#ifdef CONFIG_ARM64_CPU_SUSPEND +#ifdef CONFIG_CPU_PM DEFINE (CPU_SUSPEND_SZ ,sizeof (struct cpu_suspend_ctx ));DEFINE (CPU_CTX_SP , offsetof(struct cpu_suspend_ctx , sp));DEFINE (MPIDR_HASH_MASK , offsetof(struct mpidr_hash , mask));arch/arm64/kernel/cpuidle.c +20
-0@@ -29,3 +29,23 @@ int cpu_init_idle(unsigned int cpu)of_node_put (cpu_node);return ret; } + +/** +* cpu_suspend() - function to enter a low-power idle state +* @arg: argument to pass to CPU suspend operations +* +* Return: 0 on success, -EOPNOTSUPP if CPU suspend hook not initialized, CPU +* operations back-end error code otherwise. +*/ +int cpu_suspend (unsigned long arg) +{ +int cpu =smp_processor_id (); + +/* +* If cpu_ops have not been registered or suspend +* has not been initialized, cpu_suspend call fails early. +*/ +if (!cpu_ops[cpu] || !cpu_ops[cpu]->cpu_suspend) +return -EOPNOTSUPP ; +return cpu_ops[cpu]->cpu_suspend (arg); +}arch/arm64/kernel/hw_breakpoint.c +1
-1@@ -894,7 +894,7 @@ static struct notifier_block hw_breakpoint_reset_nb = { .notifier_call = hw_breakpoint_reset_notify, }; -#ifdef CONFIG_ARM64_CPU_SUSPEND +#ifdef CONFIG_CPU_PM extern void cpu_suspend_set_dbg_restorer (void (*hw_bp_restore)(void *));#else static inline void cpu_suspend_set_dbg_restorer (void (*hw_bp_restore)(void *))arch/arm64/kernel/psci.c +0
-2@@ -540,8 +540,6 @@ const struct cpu_operations cpu_psci_ops = { .name = "psci",#ifdef CONFIG_CPU_IDLE .cpu_init_idle = cpu_psci_cpu_init_idle, -#endif -#ifdef CONFIG_ARM64_CPU_SUSPEND .cpu_suspend = cpu_psci_cpu_suspend,#endif #ifdef CONFIG_SMP arch/arm64/kernel/suspend.c +0
-21@@ -1,7 +1,6 @@#include <linux/percpu.h> #include <linux/slab.h> #include <asm/cacheflush.h> -#include <asm/cpu_ops.h> #include <asm/debug-monitors.h> #include <asm/pgtable.h> #include <asm/memory.h> @@ -51,26 +50,6 @@ void __init cpu_suspend_set_dbg_restorer(void (*hw_bp_restore)(void *)) hw_breakpoint_restore = hw_bp_restore; } -/** -* cpu_suspend() - function to enter a low-power state -* @arg: argument to pass to CPU suspend operations -* -* Return: 0 on success, -EOPNOTSUPP if CPU suspend hook not initialized, CPU -* operations back-end error code otherwise. -*/ -int cpu_suspend (unsigned long arg) -{ -int cpu =smp_processor_id (); - -/* -* If cpu_ops have not been registered or suspend -* has not been initialized, cpu_suspend call fails early. -*/ -if (!cpu_ops[cpu] || !cpu_ops[cpu]->cpu_suspend) -return -EOPNOTSUPP ; -return cpu_ops[cpu]->cpu_suspend (arg); -} -/* * __cpu_suspend * arch/arm64/mm/proc.S +1
-1@@ -102,7 +102,7 @@ ENTRY(cpu_do_idle) ret ENDPROC(cpu_do_idle) -#ifdef CONFIG_ARM64_CPU_SUSPEND +#ifdef CONFIG_CPU_PM /** * cpu_do_suspend - save CPU registers context *drivers/cpuidle/Kconfig.arm64 +0
-1@@ -4,7 +4,6 @@ config ARM64_CPUIDLE bool "Generic ARM64 CPU idle Driver" - select ARM64_CPU_SUSPEND select DT_IDLE_STATES help Select this to enable generic cpuidle driver for ARM64.drivers/cpuidle/cpuidle-arm64.c +0
-1@@ -19,7 +19,6 @@#include <linux/of.h> #include <asm/cpuidle.h> -#include <asm/suspend.h> #include "dt_idle_states.h"
PATCH v3 1/1 arm64: kernel: remove ARM64_CPU_SUSPEND config option
Quoted 31 lines · Bartlomiej Zolnierkiewicz
Hi,
Quoted 27 lines · Lorenzo Pieralisi
ARM64_CPU_SUSPENDconfig option was introduced to make code providing context save/restore selectable only on platforms requiring power management capabilities.Currently
ARM64_CPU_SUSPENDdepends on thePM_SLEEPconfig option which in turn is set by the SUSPEND config option.The introduction of
CPU_IDLEfor arm64 requires that code configured byARM64_CPU_SUSPEND(context save/restore) should be compiled in in order to enable the CPU idle driver to rely on CPU operations carrying out context save/restore.The
ARM64_CPUIDLEconfig option (ARM64 generic idle driver) is therefore forced to selectARM64_CPU_SUSPEND, even if there may be (iePM_SLEEP) failed dependencies, which is not a clean way of handling the kernel configuration option.For these reasons, this patch removes the
ARM64_CPU_SUSPENDconfig option and makes the context save/restore dependent onCPU_PM, which is selected whenever either SUSPEND orCPU_IDLEare configured, cleaning up dependencies in the process.This way, code previously configured through
ARM64_CPU_SUSPENDis compiled in whenever a power management subsystem requires it to be present in the kernel (SUSPEND ||CPU_IDLE), which is the behaviour expected on ARM64 kernels.
Quoted 8 lines
Quoted 5 lines
The
cpu_suspendandcpu_init_idleCPU operations are added only ifCPU_IDLEis selected, since they areCPU_IDLEspecific methods and should be grouped and defined accordingly.PSCI CPU operations are updated to reflect the introduced changes.
cpu_suspendCPU operation is currentlyCPU_IDLEspecific but I wonder whether this is correct in the context of PSCI?
On arm64 cpu_suspend() is the function interfacing the idle driver to the arch code, I will rename it in a subsequent clean-up that will include cpu_ops.cpu_suspend too (ie cpu_enter_idle).
PSCI implements the cpu_ops.cpu_suspend operation through the PSCI CPU suspend call.
Quoted 4 lines
[ On 32-bit ARM PSCI
cpu_suspendoperation can be used in suspend code-path
(please take a look atarch/arm/mach-highbank/pm.c), on 64-bit ARM PSCI this is not possible currently. ]
I did not merge that code, and in that context PSCI CPU suspend has been used to enter what I guess what the deepest idle state, it slipped my review, it is a valid usage, should have been documented though before implementing it that way.
On arm64 we are defining a specific system suspend interface to give system suspend a clearer semantics:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/318540.html
Does this answer your question ?
Thanks,
Lorenzo
Quoted 275 lines
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung ElectronicsQuoted 265 lines
Signed-off-by Lorenzo Pieralisi <[email protected]>Cc Arnd Bergmann <[email protected]>Cc Will Deacon <[email protected]>Cc Krzysztof Kozlowski <[email protected]>Cc Daniel Lezcano <[email protected]>Cc Catalin Marinas <[email protected]>Cc Mark Rutland <[email protected]>--- arch/arm64/Kconfig | 3 --- arch/arm64/include/asm/cpu_ops.h | 8 ++++---- arch/arm64/include/asm/cpuidle.h | 6 ++++++ arch/arm64/include/asm/suspend.h | 2 -- arch/arm64/kernel/Makefile | 2 +- arch/arm64/kernel/asm-offsets.c | 2 +- arch/arm64/kernel/cpuidle.c | 20 ++++++++++++++++++++ arch/arm64/kernel/hw_breakpoint.c | 2 +- arch/arm64/kernel/psci.c | 2 -- arch/arm64/kernel/suspend.c | 21 --------------------- arch/arm64/mm/proc.S | 2 +- drivers/cpuidle/Kconfig.arm64 | 1 - drivers/cpuidle/cpuidle-arm64.c | 1 - 13 files changed, 34 insertions(+), 38 deletions(-)arch/arm64/Kconfig +0
-3@@ -627,9 +627,6 @@ source "kernel/power/Kconfig" config ARCH_SUSPEND_POSSIBLE def_bool y -config ARM64_CPU_SUSPEND - def_bool PM_SLEEP - endmenu menu "CPU Power Management"arch/arm64/include/asm/cpu_ops.h +4
-4@@ -28,8 +28,6 @@ struct device_node;* enable-method property. * @cpu_init: Reads any data necessary for a specific enable-method from the * devicetree, for a given cpu node and proposed logical id. -* @cpu_init_idle: Reads any data necessary to initialize CPU idle states from -* devicetree, for a given cpu node and proposed logical id. * @cpu_prepare: Early one-time preparation step for a cpu. If there is a * mechanism for doing so, tests whether it is possible to boot * the given CPU. @@ -42,6 +40,8 @@ struct device_node;* @cpu_die: Makes a cpu leave the kernel. Must not fail. Called from the * cpu being killed. * @cpu_kill: Ensures a cpu has left the kernel. Called from another cpu. +* @cpu_init_idle: Reads any data necessary to initialize CPU idle states from +* devicetree, for a given cpu node and proposed logical id. * @cpu_suspend: Suspends a cpu and saves the required context. May fail owing * to wrong parameters or error conditions. Called from the * CPU being suspended. Must be called with IRQs disabled. @@ -49,7 +49,6 @@ struct device_node;struct cpu_operations {const char *name;int (*cpu_init)(struct device_node *,unsigned int ); -int (*cpu_init_idle)(struct device_node *,unsigned int );int (*cpu_prepare)(unsigned int );int (*cpu_boot)(unsigned int );void (*cpu_postboot)(void );@@ -58,7 +57,8 @@ struct cpu_operations {void (*cpu_die)(unsigned int cpu);int (*cpu_kill)(unsigned int cpu);#endif -#ifdef CONFIG_ARM64_CPU_SUSPEND +#ifdef CONFIG_CPU_IDLE +int (*cpu_init_idle)(struct device_node *,unsigned int );int (*cpu_suspend)(unsigned long );#endif };arch/arm64/include/asm/cpuidle.h +6
-0@@ -3,11 +3,17 @@#ifdef CONFIG_CPU_IDLE extern int cpu_init_idle (unsigned int cpu); +extern int cpu_suspend (unsigned long arg);#else static inline int cpu_init_idle (unsigned int cpu) {return -EOPNOTSUPP ; } + +static inline int cpu_suspend (unsigned long arg) +{ +return -EOPNOTSUPP ; +}#endif #endif arch/arm64/include/asm/suspend.h +0
-2@@ -23,6 +23,4 @@ struct sleep_save_sp {extern int __cpu_suspend (unsigned long arg,int (*fn)(unsigned long ));extern void cpu_resume (void ); -extern int cpu_suspend (unsigned long ); -#endif arch/arm64/kernel/Makefile +1
-1@@ -27,7 +27,7 @@ arm64-obj-$(CONFIG_SMP) += smp.o smp_spin_table.o topology.o arm64-obj-$(CONFIG_PERF_EVENTS) += perf_regs.o arm64-obj-$(CONFIG_HW_PERF_EVENTS) += perf_event.o arm64-obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o -arm64-obj-$(CONFIG_ARM64_CPU_SUSPEND) += sleep.o suspend.o +arm64-obj-$(CONFIG_CPU_PM) += sleep.o suspend.o arm64-obj-$(CONFIG_CPU_IDLE) += cpuidle.o arm64-obj-$(CONFIG_JUMP_LABEL) += jump_label.o arm64-obj-$(CONFIG_KGDB) += kgdb.oarch/arm64/kernel/asm-offsets.c +1
-1@@ -152,7 +152,7 @@ int main(void) DEFINE(KVM_VTTBR, offsetof(struct kvm, arch.vttbr)); DEFINE(KVM_VGIC_VCTRL, offsetof(struct kvm, arch.vgic.vctrl_base));#endif -#ifdef CONFIG_ARM64_CPU_SUSPEND +#ifdef CONFIG_CPU_PM DEFINE (CPU_SUSPEND_SZ ,sizeof (struct cpu_suspend_ctx ));DEFINE (CPU_CTX_SP , offsetof(struct cpu_suspend_ctx , sp));DEFINE (MPIDR_HASH_MASK , offsetof(struct mpidr_hash , mask));arch/arm64/kernel/cpuidle.c +20
-0@@ -29,3 +29,23 @@ int cpu_init_idle(unsigned int cpu)of_node_put (cpu_node);return ret; } + +/** +* cpu_suspend() - function to enter a low-power idle state +* @arg: argument to pass to CPU suspend operations +* +* Return: 0 on success, -EOPNOTSUPP if CPU suspend hook not initialized, CPU +* operations back-end error code otherwise. +*/ +int cpu_suspend (unsigned long arg) +{ +int cpu =smp_processor_id (); + +/* +* If cpu_ops have not been registered or suspend +* has not been initialized, cpu_suspend call fails early. +*/ +if (!cpu_ops[cpu] || !cpu_ops[cpu]->cpu_suspend) +return -EOPNOTSUPP ; +return cpu_ops[cpu]->cpu_suspend (arg); +}arch/arm64/kernel/hw_breakpoint.c +1
-1@@ -894,7 +894,7 @@ static struct notifier_block hw_breakpoint_reset_nb = { .notifier_call = hw_breakpoint_reset_notify, }; -#ifdef CONFIG_ARM64_CPU_SUSPEND +#ifdef CONFIG_CPU_PM extern void cpu_suspend_set_dbg_restorer (void (*hw_bp_restore)(void *));#else static inline void cpu_suspend_set_dbg_restorer (void (*hw_bp_restore)(void *))arch/arm64/kernel/psci.c +0
-2@@ -540,8 +540,6 @@ const struct cpu_operations cpu_psci_ops = { .name = "psci",#ifdef CONFIG_CPU_IDLE .cpu_init_idle = cpu_psci_cpu_init_idle, -#endif -#ifdef CONFIG_ARM64_CPU_SUSPEND .cpu_suspend = cpu_psci_cpu_suspend,#endif #ifdef CONFIG_SMP arch/arm64/kernel/suspend.c +0
-21@@ -1,7 +1,6 @@#include <linux/percpu.h> #include <linux/slab.h> #include <asm/cacheflush.h> -#include <asm/cpu_ops.h> #include <asm/debug-monitors.h> #include <asm/pgtable.h> #include <asm/memory.h> @@ -51,26 +50,6 @@ void __init cpu_suspend_set_dbg_restorer(void (*hw_bp_restore)(void *)) hw_breakpoint_restore = hw_bp_restore; } -/** -* cpu_suspend() - function to enter a low-power state -* @arg: argument to pass to CPU suspend operations -* -* Return: 0 on success, -EOPNOTSUPP if CPU suspend hook not initialized, CPU -* operations back-end error code otherwise. -*/ -int cpu_suspend (unsigned long arg) -{ -int cpu =smp_processor_id (); - -/* -* If cpu_ops have not been registered or suspend -* has not been initialized, cpu_suspend call fails early. -*/ -if (!cpu_ops[cpu] || !cpu_ops[cpu]->cpu_suspend) -return -EOPNOTSUPP ; -return cpu_ops[cpu]->cpu_suspend (arg); -} -/* * __cpu_suspend * arch/arm64/mm/proc.S +1
-1@@ -102,7 +102,7 @@ ENTRY(cpu_do_idle) ret ENDPROC(cpu_do_idle) -#ifdef CONFIG_ARM64_CPU_SUSPEND +#ifdef CONFIG_CPU_PM /** * cpu_do_suspend - save CPU registers context *drivers/cpuidle/Kconfig.arm64 +0
-1@@ -4,7 +4,6 @@ config ARM64_CPUIDLE bool "Generic ARM64 CPU idle Driver" - select ARM64_CPU_SUSPEND select DT_IDLE_STATES help Select this to enable generic cpuidle driver for ARM64.drivers/cpuidle/cpuidle-arm64.c +0
-1@@ -19,7 +19,6 @@#include <linux/of.h> #include <asm/cpuidle.h> -#include <asm/suspend.h> #include "dt_idle_states.h"
PATCH v3 1/1 arm64: kernel: remove ARM64_CPU_SUSPEND config option
Quoted 58 lines · Lorenzo Pieralisi
Quoted 31 lines · Bartlomiej Zolnierkiewicz
Hi,
Quoted 27 lines · Lorenzo Pieralisi
ARM64_CPU_SUSPENDconfig option was introduced to make code providing context save/restore selectable only on platforms requiring power management capabilities.Currently
ARM64_CPU_SUSPENDdepends on thePM_SLEEPconfig option which in turn is set by the SUSPEND config option.The introduction of
CPU_IDLEfor arm64 requires that code configured byARM64_CPU_SUSPEND(context save/restore) should be compiled in in order to enable the CPU idle driver to rely on CPU operations carrying out context save/restore.The
ARM64_CPUIDLEconfig option (ARM64 generic idle driver) is therefore forced to selectARM64_CPU_SUSPEND, even if there may be (iePM_SLEEP) failed dependencies, which is not a clean way of handling the kernel configuration option.For these reasons, this patch removes the
ARM64_CPU_SUSPENDconfig option and makes the context save/restore dependent onCPU_PM, which is selected whenever either SUSPEND orCPU_IDLEare configured, cleaning up dependencies in the process.This way, code previously configured through
ARM64_CPU_SUSPENDis compiled in whenever a power management subsystem requires it to be present in the kernel (SUSPEND ||CPU_IDLE), which is the behaviour expected on ARM64 kernels.Quoted 8 lines
Quoted 5 lines
The
cpu_suspendandcpu_init_idleCPU operations are added only ifCPU_IDLEis selected, since they areCPU_IDLEspecific methods and should be grouped and defined accordingly.PSCI CPU operations are updated to reflect the introduced changes.
cpu_suspendCPU operation is currentlyCPU_IDLEspecific but I wonder whether this is correct in the context of PSCI?On arm64
cpu_suspend()is the function interfacing the idle driver to the arch code, I will rename it in a subsequent clean-up that will include cpu_ops.cpu_suspend too (iecpu_enter_idle).PSCI implements the cpu_ops.cpu_suspend operation through the PSCI CPU suspend call.
Quoted 4 lines
[ On 32-bit ARM PSCI
cpu_suspendoperation can be used in suspend code-path
(please take a look atarch/arm/mach-highbank/pm.c), on 64-bit ARM PSCI this is not possible currently. ]I did not merge that code, and in that context PSCI CPU suspend has been used to enter what I guess what the deepest idle state, it slipped my review, it is a valid usage, should have been documented though before implementing it that way.
Quoted 6 lines
On arm64 we are defining a specific system suspend interface to give system suspend a clearer semantics:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/318540.html
Does this answer your question ?
Yes. Thank you!
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics