Skip to contentSkip to conversation

PSCI: system suspend support

PATCH Superseded Sudeep Holla 5 participants 26 messages Jun 16, 2015 – Sep 14, 2015

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover PSCI: system suspend support

Hi,

These couple of patches add generic support for PSCI system suspend.

The first patch aligns the cpu_{suspend,resume} functions so that they have identical prototype both on ARM32 and ARM64. The second patch adds the system suspend support on top of PSCIv1.0 implementation.

The series is built on top of M.Rutland's branch[1] + Lorenzo's PSCI v1.0 support[2]. The wakeup source is not scope of these patches. It needs to addressed separately.

Platforms with PSCI v1.0 supported in the firmware and SYSTEM_SUSPEND feature advertised as available is required for testing this. This was tested in ARM on Juno platform.

Regards,
Sudeep

[1] git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git psci/unification
[2] http://www.spinics.net/lists/devicetree/msg82017.html

Sudeep Holla (2):
arm64: kernel: rename __cpu_suspend to keep it aligned with arm
drivers: firmware: psci: add system suspend support

 arch/arm64/include/asm/cpuidle.h |  8 ++------
 arch/arm64/include/asm/suspend.h |  2 +-
 arch/arm64/kernel/cpuidle.c      |  4 ++--
 arch/arm64/kernel/psci.c         |  2 +-
 arch/arm64/kernel/suspend.c      |  6 +++---
 drivers/firmware/psci.c          | 39 +++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/psci.h        |  2 ++
 7 files changed, 50 insertions(+), 13 deletions(-)
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/2 arm64: kernel: rename __cpu_suspend to keep it aligned with arm

This patch renames __cpu_suspend to cpu_suspend so that it's aligned with ARM32. It also removes the redundant wrapper created.

This is preparation to implement generic PSCI system suspend using the cpu_{suspend,resume} which now has the same interface on both ARM and ARM64.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
 arch/arm64/include/asm/cpuidle.h | 8 ++------
 arch/arm64/include/asm/suspend.h | 2 +-
 arch/arm64/kernel/cpuidle.c      | 4 ++--
 arch/arm64/kernel/psci.c         | 2 +-
 arch/arm64/kernel/suspend.c      | 6 +++---
 5 files changed, 9 insertions(+), 13 deletions(-)
arch/arm64/include/asm/cpuidle.h +2 -6
index 141b2fcabaa6..0f74f05d662a 100644
@@ -5,20 +5,16 @@
 
 #ifdef CONFIG_CPU_IDLE
 extern int arm_cpuidle_init(unsigned int cpu);
-extern int cpu_suspend(unsigned long arg);
+extern int arm_cpuidle_suspend(int index);
 #else
 static inline int arm_cpuidle_init(unsigned int cpu)
 {
 	return -EOPNOTSUPP;
 }
 
-static inline int cpu_suspend(unsigned long arg)
+static inline int arm_cpuidle_suspend(int index)
 {
 	return -EOPNOTSUPP;
 }
 #endif
-static inline int arm_cpuidle_suspend(int index)
-{
-	return cpu_suspend(index);
-}
 #endif
arch/arm64/include/asm/suspend.h +1 -1
index 003802f58963..59a5b0f1e81c 100644
@@ -21,6 +21,6 @@ struct sleep_save_sp {
 	phys_addr_t save_ptr_stash_phys;
 };
 
-extern int __cpu_suspend(unsigned long arg, int (*fn)(unsigned long));
+extern int cpu_suspend(unsigned long arg, int (*fn)(unsigned long));
 extern void cpu_resume(void);
 #endif
arch/arm64/kernel/cpuidle.c +2 -2
index f8aa16973318..7ce589ca54a4 100644
@@ -32,7 +32,7 @@ int arm_cpuidle_init(unsigned int cpu)
  * 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 arm_cpuidle_suspend(int index)
 {
 	int cpu = smp_processor_id();
 
@@ -42,5 +42,5 @@ int cpu_suspend(unsigned long arg)
 	 */
 	if (!cpu_ops[cpu] || !cpu_ops[cpu]->cpu_suspend)
 		return -EOPNOTSUPP;
-	return cpu_ops[cpu]->cpu_suspend(arg);
+	return cpu_ops[cpu]->cpu_suspend(index);
 }
arch/arm64/kernel/psci.c +1 -1
index a0fc99a518b8..4be597228b14 100644
@@ -203,7 +203,7 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index)
 	if (!psci_power_state_loses_context(state[index - 1]))
 		ret = psci_ops.cpu_suspend(state[index - 1], 0);
 	else
-		ret = __cpu_suspend(index, psci_suspend_finisher);
+		ret = cpu_suspend(index, psci_suspend_finisher);
 
 	return ret;
 }
arch/arm64/kernel/suspend.c +3 -3
index d7daf45ae7a2..400e1265e205 100644
@@ -51,13 +51,13 @@ void __init cpu_suspend_set_dbg_restorer(void (*hw_bp_restore)(void *))
 }
 
 /*
- * __cpu_suspend
+ * cpu_suspend
  *
  * arg: argument to pass to the finisher function
  * fn: finisher function pointer
  *
  */
-int __cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
+int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
 {
 	struct mm_struct *mm = current->active_mm;
 	int ret;
@@ -82,7 +82,7 @@ int __cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
 		 * We are resuming from reset with TTBR0_EL1 set to the
 		 * idmap to enable the MMU; restore the active_mm mappings in
 		 * TTBR0_EL1 unless the active_mm == &init_mm, in which case
-		 * the thread entered __cpu_suspend with TTBR0_EL1 set to
+		 * the thread entered cpu_suspend with TTBR0_EL1 set to
 		 * reserved TTBR0 page tables and should be restored as such.
 		 */
 		if (mm == &init_mm)
Signature · 1 line
1.9.1
#2 lore raw

PATCH 1/2 arm64: kernel: rename __cpu_suspend to keep it aligned with arm

Quoted 10 lines · Sudeep Holla

This patch renames __cpu_suspend to cpu_suspend so that it's aligned with ARM32. It also removes the redundant wrapper created.

This is preparation to implement generic PSCI system suspend using the cpu_{suspend,resume} which now has the same interface on both ARM and ARM64.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
Quoted 7 lines
---
 arch/arm64/include/asm/cpuidle.h | 8 ++------
 arch/arm64/include/asm/suspend.h | 2 +-
 arch/arm64/kernel/cpuidle.c      | 4 ++--
 arch/arm64/kernel/psci.c         | 2 +-
 arch/arm64/kernel/suspend.c      | 6 +++---
 5 files changed, 9 insertions(+), 13 deletions(-)
Quoted 104 lines
arch/arm64/include/asm/cpuidle.h +2 -6
index 141b2fcabaa6..0f74f05d662a 100644
@@ -5,20 +5,16 @@
 
 #ifdef CONFIG_CPU_IDLE
 extern int arm_cpuidle_init(unsigned int cpu);
-extern int cpu_suspend(unsigned long arg);
+extern int arm_cpuidle_suspend(int index);
 #else
 static inline int arm_cpuidle_init(unsigned int cpu)
 {
 	return -EOPNOTSUPP;
 }
 
-static inline int cpu_suspend(unsigned long arg)
+static inline int arm_cpuidle_suspend(int index)
 {
 	return -EOPNOTSUPP;
 }
 #endif
-static inline int arm_cpuidle_suspend(int index)
-{
-	return cpu_suspend(index);
-}
 #endif
arch/arm64/include/asm/suspend.h +1 -1
index 003802f58963..59a5b0f1e81c 100644
@@ -21,6 +21,6 @@ struct sleep_save_sp {
 	phys_addr_t save_ptr_stash_phys;
 };
 
-extern int __cpu_suspend(unsigned long arg, int (*fn)(unsigned long));
+extern int cpu_suspend(unsigned long arg, int (*fn)(unsigned long));
 extern void cpu_resume(void);
 #endif
arch/arm64/kernel/cpuidle.c +2 -2
index f8aa16973318..7ce589ca54a4 100644
@@ -32,7 +32,7 @@ int arm_cpuidle_init(unsigned int cpu)
  * 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 arm_cpuidle_suspend(int index)
 {
 	int cpu = smp_processor_id();
 
@@ -42,5 +42,5 @@ int cpu_suspend(unsigned long arg)
 	 */
 	if (!cpu_ops[cpu] || !cpu_ops[cpu]->cpu_suspend)
 		return -EOPNOTSUPP;
-	return cpu_ops[cpu]->cpu_suspend(arg);
+	return cpu_ops[cpu]->cpu_suspend(index);
 }
arch/arm64/kernel/psci.c +1 -1
index a0fc99a518b8..4be597228b14 100644
@@ -203,7 +203,7 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index)
 	if (!psci_power_state_loses_context(state[index - 1]))
 		ret = psci_ops.cpu_suspend(state[index - 1], 0);
 	else
-		ret = __cpu_suspend(index, psci_suspend_finisher);
+		ret = cpu_suspend(index, psci_suspend_finisher);
 
 	return ret;
 }
arch/arm64/kernel/suspend.c +3 -3
index d7daf45ae7a2..400e1265e205 100644
@@ -51,13 +51,13 @@ void __init cpu_suspend_set_dbg_restorer(void (*hw_bp_restore)(void *))
 }
 
 /*
- * __cpu_suspend
+ * cpu_suspend
  *
  * arg: argument to pass to the finisher function
  * fn: finisher function pointer
  *
  */
-int __cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
+int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
 {
 	struct mm_struct *mm = current->active_mm;
 	int ret;
@@ -82,7 +82,7 @@ int __cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
 		 * We are resuming from reset with TTBR0_EL1 set to the
 		 * idmap to enable the MMU; restore the active_mm mappings in
 		 * TTBR0_EL1 unless the active_mm == &init_mm, in which case
-		 * the thread entered __cpu_suspend with TTBR0_EL1 set to
+		 * the thread entered cpu_suspend with TTBR0_EL1 set to
 		 * reserved TTBR0 page tables and should be restored as such.
 		 */
 		if (mm == &init_mm)
Signature · 1 line
1.9.1
#3 lore raw

PATCH 2/2 drivers: firmware: psci: add system suspend support

In reply to Sudeep Holla

PSCI v1.0 introduces a new API called PSCI_SYSTEM_SUSPEND. This API provides the mechanism by which the calling OS can request entry into the deepest possible system sleep state.

It meets all the necessary preconditions for entrying suspend to RAM state in Linux. This patch adds support for PSCI_SYSTEM_SUSPEND in psci firmware and registers a psci system suspend operation to implement the suspend-to-RAM(s2r) in a generic way on all the platforms implementing PSCI.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
 drivers/firmware/psci.c   | 39 +++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/psci.h |  2 ++
 2 files changed, 41 insertions(+)
drivers/firmware/psci.c +39 -0
index 68d0c2f6d004..4f0359516119 100644
@@ -20,11 +20,13 @@
 #include <linux/printk.h>
 #include <linux/psci.h>
 #include <linux/reboot.h>
+#include <linux/suspend.h>
 
 #include <uapi/linux/psci.h>
 
 #include <asm/system_misc.h>
 #include <asm/smp_plat.h>
+#include <asm/suspend.h>
 
 /*
  * While a 64-bit OS can make calls with SMC32 calling conventions, for some
@@ -222,6 +224,41 @@ static int __init psci_features(u32 psci_func_id)
 			      psci_func_id, 0, 0);
 }
 
+static int psci_system_suspend(unsigned long entry_point)
+{
+	return invoke_psci_fn(PSCI_0_2_FN_NATIVE(SYSTEM_SUSPEND),
+			      entry_point, 0, 0);
+}
+
+static int psci_system_suspend_finisher(unsigned long unused)
+{
+	return psci_system_suspend(virt_to_phys(cpu_resume));
+}
+
+static int psci_system_suspend_enter(suspend_state_t state)
+{
+	if (state != PM_SUSPEND_MEM)
+		return 0;
+
+	return cpu_suspend(0, psci_system_suspend_finisher);
+}
+
+static const struct platform_suspend_ops psci_suspend_ops = {
+	.valid          = suspend_valid_only_mem,
+	.enter          = psci_system_suspend_enter,
+};
+
+static void __init psci_init_system_suspend(void)
+{
+	if (!IS_ENABLED(CONFIG_SUSPEND))
+		return;
+
+	if (psci_features(PSCI_0_2_FN_NATIVE(SYSTEM_SUSPEND)))
+		return;
+
+	suspend_set_ops(&psci_suspend_ops);
+}
+
 static void __init psci_init_cpu_suspend(void)
 {
 	int feature = psci_features(psci_function_id[PSCI_FN_CPU_SUSPEND]);
@@ -310,6 +347,8 @@ static int __init psci_probe(void)
 
 	psci_init_cpu_suspend();
 
+	psci_init_system_suspend();
+
 	return 0;
 }
 
include/uapi/linux/psci.h +2 -0
index 2598d7c9bf20..62b054524819 100644
@@ -39,12 +39,14 @@
 #define PSCI_0_2_FN_MIGRATE_INFO_UP_CPU		PSCI_0_2_FN(7)
 #define PSCI_0_2_FN_SYSTEM_OFF			PSCI_0_2_FN(8)
 #define PSCI_0_2_FN_SYSTEM_RESET		PSCI_0_2_FN(9)
+#define PSCI_0_2_FN_SYSTEM_SUSPEND		PSCI_0_2_FN(14)
 
 #define PSCI_0_2_FN64_CPU_SUSPEND		PSCI_0_2_FN64(1)
 #define PSCI_0_2_FN64_CPU_ON			PSCI_0_2_FN64(3)
 #define PSCI_0_2_FN64_AFFINITY_INFO		PSCI_0_2_FN64(4)
 #define PSCI_0_2_FN64_MIGRATE			PSCI_0_2_FN64(5)
 #define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU	PSCI_0_2_FN64(7)
+#define PSCI_0_2_FN64_SYSTEM_SUSPEND		PSCI_0_2_FN64(14)
 
 #define PSCI_1_0_FN_PSCI_FEATURES		PSCI_0_2_FN(10)
 
Signature · 1 line
1.9.1
#4 lore raw

PATCH 2/2 drivers: firmware: psci: add system suspend support

Quoted 5 lines · Sudeep Holla

PSCI v1.0 introduces a new API called PSCI_SYSTEM_SUSPEND. This API provides the mechanism by which the calling OS can request entry into the deepest possible system sleep state.

It meets all the necessary preconditions for entrying suspend to RAM

s/entrying/entering/

Quoted 34 lines

state in Linux. This patch adds support for PSCI_SYSTEM_SUSPEND in psci firmware and registers a psci system suspend operation to implement the suspend-to-RAM(s2r) in a generic way on all the platforms implementing PSCI.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
 drivers/firmware/psci.c   | 39 +++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/psci.h |  2 ++
 2 files changed, 41 insertions(+)
drivers/firmware/psci.c +2 -0
index 68d0c2f6d004..4f0359516119 100644
@@ -20,11 +20,13 @@
 #include <linux/printk.h>
 #include <linux/psci.h>
 #include <linux/reboot.h>
+#include <linux/suspend.h>
 
 #include <uapi/linux/psci.h>
 
 #include <asm/system_misc.h>
 #include <asm/smp_plat.h>
+#include <asm/suspend.h>
 
 /*
  * While a 64-bit OS can make calls with SMC32 calling conventions, for some
@@ -222,6 +224,41 @@ static int __init psci_features(u32 psci_func_id)
 			      psci_func_id, 0, 0);
 }

drivers/firmware/psci.c static int __init psci_features(u32 psci_func_id) · line 235

Quoted 10 lines
drivers/firmware/psci.c +9 -0
 
+static int psci_system_suspend(unsigned long entry_point)
+{
+	return invoke_psci_fn(PSCI_0_2_FN_NATIVE(SYSTEM_SUSPEND),
+			      entry_point, 0, 0);
+}
+
+static int psci_system_suspend_finisher(unsigned long unused)
+{
+	return psci_system_suspend(virt_to_phys(cpu_resume));

Do we really need another level of indirection ? I would invoke the PSCI function from here.

drivers/firmware/psci.c static int __init psci_features(u32 psci_func_id) · line 241

Quoted 6 lines
drivers/firmware/psci.c +6 -0
+}
+
+static int psci_system_suspend_enter(suspend_state_t state)
+{
+	if (state != PM_SUSPEND_MEM)
+		return 0;

suspend_valid_only_mem should check this, am I missing something ?

You should not return 0 on failure anyway.

Quoted 5 lines
drivers/firmware/psci.c +5 -0
+
+	return cpu_suspend(0, psci_system_suspend_finisher);
+}
+
+static const struct platform_suspend_ops psci_suspend_ops = {

drivers/firmware/psci.c static int __init psci_features(u32 psci_func_id) · line 256

Quoted 10 lines
drivers/firmware/psci.c +10 -0
+	.valid          = suspend_valid_only_mem,
+	.enter          = psci_system_suspend_enter,
+};
+
+static void __init psci_init_system_suspend(void)
+{
+	if (!IS_ENABLED(CONFIG_SUSPEND))
+		return;
+
+	if (psci_features(PSCI_0_2_FN_NATIVE(SYSTEM_SUSPEND)))

Nit: I think we should use a 1_0 tag here for SYSTEM_SUSPEND.

Quoted 22 lines
drivers/firmware/psci.c +7 -0
+		return;
+
+	suspend_set_ops(&psci_suspend_ops);
+}
+
 static void __init psci_init_cpu_suspend(void)
 {
 	int feature = psci_features(psci_function_id[PSCI_FN_CPU_SUSPEND]);
@@ -310,6 +347,8 @@ static int __init psci_probe(void)
 
 	psci_init_cpu_suspend();
 
+	psci_init_system_suspend();
+
 	return 0;
 }
 
include/uapi/linux/psci.h +0 -0
index 2598d7c9bf20..62b054524819 100644

include/uapi/linux/psci.h line 42

Quoted 4 lines
include/uapi/linux/psci.h +1 -0
@@ -39,12 +39,14 @@
 #define PSCI_0_2_FN_MIGRATE_INFO_UP_CPU		PSCI_0_2_FN(7)
 #define PSCI_0_2_FN_SYSTEM_OFF			PSCI_0_2_FN(8)
 #define PSCI_0_2_FN_SYSTEM_RESET		PSCI_0_2_FN(9)
+#define PSCI_0_2_FN_SYSTEM_SUSPEND		PSCI_0_2_FN(14)

I think you should keep versioning consistent, namely SYSTEM_SUSPEND is a 1.0 only feature and the tag should reflect that.

include/uapi/linux/psci.h line 49

Quoted 6 lines
include/uapi/linux/psci.h +1 -0
 #define PSCI_0_2_FN64_CPU_SUSPEND		PSCI_0_2_FN64(1)
 #define PSCI_0_2_FN64_CPU_ON			PSCI_0_2_FN64(3)
 #define PSCI_0_2_FN64_AFFINITY_INFO		PSCI_0_2_FN64(4)
 #define PSCI_0_2_FN64_MIGRATE			PSCI_0_2_FN64(5)
 #define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU	PSCI_0_2_FN64(7)
+#define PSCI_0_2_FN64_SYSTEM_SUSPEND		PSCI_0_2_FN64(14)

Ditto.

Thanks,
Lorenzo

Quoted 6 lines
include/uapi/linux/psci.h +0 -0
 #define PSCI_1_0_FN_PSCI_FEATURES		PSCI_0_2_FN(10)
 
Signature · 1 line
1.9.1
#5 lore raw

PATCH 2/2 drivers: firmware: psci: add system suspend support

Quoted 44 lines · Lorenzo Pieralisi
Quoted 5 lines · Sudeep Holla

PSCI v1.0 introduces a new API called PSCI_SYSTEM_SUSPEND. This API provides the mechanism by which the calling OS can request entry into the deepest possible system sleep state.

It meets all the necessary preconditions for entrying suspend to RAM

s/entrying/entering/

Quoted 35 lines

state in Linux. This patch adds support for PSCI_SYSTEM_SUSPEND in psci firmware and registers a psci system suspend operation to implement the suspend-to-RAM(s2r) in a generic way on all the platforms implementing PSCI.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
  drivers/firmware/psci.c   | 39 +++++++++++++++++++++++++++++++++++++++
  include/uapi/linux/psci.h |  2 ++
  2 files changed, 41 insertions(+)
drivers/firmware/psci.c +2 -0
index 68d0c2f6d004..4f0359516119 100644
@@ -20,11 +20,13 @@
  #include <linux/printk.h>
  #include <linux/psci.h>
  #include <linux/reboot.h>
+#include <linux/suspend.h>
 
  #include <uapi/linux/psci.h>
 
  #include <asm/system_misc.h>
  #include <asm/smp_plat.h>
+#include <asm/suspend.h>
 
  /*
   * While a 64-bit OS can make calls with SMC32 calling conventions, for some
@@ -222,6 +224,41 @@ static int __init psci_features(u32 psci_func_id)
  			      psci_func_id, 0, 0);
  }
 
Quoted 13 lines

drivers/firmware/psci.c static int __init psci_features(u32 psci_func_id) · line 235

Quoted 9 lines
drivers/firmware/psci.c +9 -0
+static int psci_system_suspend(unsigned long entry_point)
+{
+	return invoke_psci_fn(PSCI_0_2_FN_NATIVE(SYSTEM_SUSPEND),
+			      entry_point, 0, 0);
+}
+
+static int psci_system_suspend_finisher(unsigned long unused)
+{
+	return psci_system_suspend(virt_to_phys(cpu_resume));

Do we really need another level of indirection ? I would invoke the PSCI function from here.

True not really required, copied from other functions just for consistency. I will merge them into one.

Quoted 9 lines

drivers/firmware/psci.c static int __init psci_features(u32 psci_func_id) · line 241

Quoted 6 lines
drivers/firmware/psci.c +6 -0
+}
+
+static int psci_system_suspend_enter(suspend_state_t state)
+{
+	if (state != PM_SUSPEND_MEM)
+		return 0;

suspend_valid_only_mem should check this, am I missing something ?

Yes it's taken care, I can remove that.

[...]

Quoted 4 lines
Quoted 4 lines
include/uapi/linux/psci.h +0 -0
index 2598d7c9bf20..62b054524819 100644
Quoted 9 lines

include/uapi/linux/psci.h line 42

Quoted 5 lines
include/uapi/linux/psci.h +1 -0
@@ -39,12 +39,14 @@
  #define PSCI_0_2_FN_MIGRATE_INFO_UP_CPU		PSCI_0_2_FN(7)
  #define PSCI_0_2_FN_SYSTEM_OFF			PSCI_0_2_FN(8)
  #define PSCI_0_2_FN_SYSTEM_RESET		PSCI_0_2_FN(9)
+#define PSCI_0_2_FN_SYSTEM_SUSPEND		PSCI_0_2_FN(14)

I think you should keep versioning consistent, namely SYSTEM_SUSPEND is a 1.0 only feature and the tag should reflect that.

Correct, IIRC I kept it to make you use of PSCI_0_2_FN_NATIVE initially and forgot to change. I will update them all to use _1_0 tag.

Regards,
Sudeep

#6 lore raw

v2 cover PSCI: system suspend support

In reply to Sudeep Holla

Hi,

These couple of patches add generic support for PSCI system suspend.

The first patch aligns the cpu_{suspend,resume} functions so that they have identical prototype both on ARM32 and ARM64. The second patch adds the system suspend support on top of PSCIv1.0 implementation.

The series is built on top of M.Rutland's branch[1] + Lorenzo's PSCI v1.0 support[2]. The wakeup source is not scope of these patches. It needs to addressed separately.

Platforms with PSCI v1.0 supported in the firmware and SYSTEM_SUSPEND feature advertised as available is required for testing this. This was tested in ARM on Juno platform.

Changes v1[3]->v2:
- Introduces generic macro for PSCI_FN_NATIVE
- Removed redundant code and renamed new PSCI_FN macros with v1.0 tag as suggested by Lorenzo

Regards,
Sudeep

[1] git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git psci/unification
[2] http://www.spinics.net/lists/devicetree/msg82017.html
[3] http://www.spinics.net/lists/arm-kernel/msg425793.html

Sudeep Holla (3):
arm64: kernel: rename __cpu_suspend to keep it aligned with arm
drivers: firmware: psci: define more generic PSCI_FN_NATIVE macro
drivers: firmware: psci: add system suspend support

 arch/arm64/include/asm/cpuidle.h |  8 ++-----
 arch/arm64/include/asm/suspend.h |  2 +-
 arch/arm64/kernel/cpuidle.c      |  4 ++--
 arch/arm64/kernel/psci.c         |  2 +-
 arch/arm64/kernel/suspend.c      |  6 ++---
 drivers/firmware/psci.c          | 48 +++++++++++++++++++++++++++++++++-------
 include/uapi/linux/psci.h        |  3 +++
 7 files changed, 52 insertions(+), 21 deletions(-)
Signature · 1 line
1.9.1
#7 lore raw

PATCH v2 1/3 arm64: kernel: rename __cpu_suspend to keep it aligned with arm

This patch renames __cpu_suspend to cpu_suspend so that it's aligned with ARM32. It also removes the redundant wrapper created.

This is in preparation to implement generic PSCI system suspend using the cpu_{suspend,resume} which now has the same interface on both ARM and ARM64.

---
 arch/arm64/include/asm/cpuidle.h | 8 ++------
 arch/arm64/include/asm/suspend.h | 2 +-
 arch/arm64/kernel/cpuidle.c      | 4 ++--
 arch/arm64/kernel/psci.c         | 2 +-
 arch/arm64/kernel/suspend.c      | 6 +++---
 5 files changed, 9 insertions(+), 13 deletions(-)
arch/arm64/include/asm/cpuidle.h +2 -6
index 141b2fcabaa6..0f74f05d662a 100644
@@ -5,20 +5,16 @@
 
 #ifdef CONFIG_CPU_IDLE
 extern int arm_cpuidle_init(unsigned int cpu);
-extern int cpu_suspend(unsigned long arg);
+extern int arm_cpuidle_suspend(int index);
 #else
 static inline int arm_cpuidle_init(unsigned int cpu)
 {
 	return -EOPNOTSUPP;
 }
 
-static inline int cpu_suspend(unsigned long arg)
+static inline int arm_cpuidle_suspend(int index)
 {
 	return -EOPNOTSUPP;
 }
 #endif
-static inline int arm_cpuidle_suspend(int index)
-{
-	return cpu_suspend(index);
-}
 #endif
arch/arm64/include/asm/suspend.h +1 -1
index 003802f58963..59a5b0f1e81c 100644
@@ -21,6 +21,6 @@ struct sleep_save_sp {
 	phys_addr_t save_ptr_stash_phys;
 };
 
-extern int __cpu_suspend(unsigned long arg, int (*fn)(unsigned long));
+extern int cpu_suspend(unsigned long arg, int (*fn)(unsigned long));
 extern void cpu_resume(void);
 #endif
arch/arm64/kernel/cpuidle.c +2 -2
index f8aa16973318..7ce589ca54a4 100644
@@ -32,7 +32,7 @@ int arm_cpuidle_init(unsigned int cpu)
  * 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 arm_cpuidle_suspend(int index)
 {
 	int cpu = smp_processor_id();
 
@@ -42,5 +42,5 @@ int cpu_suspend(unsigned long arg)
 	 */
 	if (!cpu_ops[cpu] || !cpu_ops[cpu]->cpu_suspend)
 		return -EOPNOTSUPP;
-	return cpu_ops[cpu]->cpu_suspend(arg);
+	return cpu_ops[cpu]->cpu_suspend(index);
 }
arch/arm64/kernel/psci.c +1 -1
index a0fc99a518b8..4be597228b14 100644
@@ -203,7 +203,7 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index)
 	if (!psci_power_state_loses_context(state[index - 1]))
 		ret = psci_ops.cpu_suspend(state[index - 1], 0);
 	else
-		ret = __cpu_suspend(index, psci_suspend_finisher);
+		ret = cpu_suspend(index, psci_suspend_finisher);
 
 	return ret;
 }
arch/arm64/kernel/suspend.c +3 -3
index d7daf45ae7a2..400e1265e205 100644
@@ -51,13 +51,13 @@ void __init cpu_suspend_set_dbg_restorer(void (*hw_bp_restore)(void *))
 }
 
 /*
- * __cpu_suspend
+ * cpu_suspend
  *
  * arg: argument to pass to the finisher function
  * fn: finisher function pointer
  *
  */
-int __cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
+int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
 {
 	struct mm_struct *mm = current->active_mm;
 	int ret;
@@ -82,7 +82,7 @@ int __cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
 		 * We are resuming from reset with TTBR0_EL1 set to the
 		 * idmap to enable the MMU; restore the active_mm mappings in
 		 * TTBR0_EL1 unless the active_mm == &init_mm, in which case
-		 * the thread entered __cpu_suspend with TTBR0_EL1 set to
+		 * the thread entered cpu_suspend with TTBR0_EL1 set to
 		 * reserved TTBR0 page tables and should be restored as such.
 		 */
 		if (mm == &init_mm)
Signature · 1 line
1.9.1
maintainer
#8 lore raw

PATCH v2 1/3 arm64: kernel: rename __cpu_suspend to keep it aligned with arm

Quoted 10 lines · Sudeep Holla

This patch renames __cpu_suspend to cpu_suspend so that it's aligned with ARM32. It also removes the redundant wrapper created.

This is in preparation to implement generic PSCI system suspend using the cpu_{suspend,resume} which now has the same interface on both ARM and ARM64.

Quoted 7 lines
---
 arch/arm64/include/asm/cpuidle.h | 8 ++------
 arch/arm64/include/asm/suspend.h | 2 +-
 arch/arm64/kernel/cpuidle.c      | 4 ++--
 arch/arm64/kernel/psci.c         | 2 +-
 arch/arm64/kernel/suspend.c      | 6 +++---
 5 files changed, 9 insertions(+), 13 deletions(-)

Does this patch depend on the other patches from Mark and Lorenzo or it can be merged earlier?

#9 lore raw

PATCH v2 1/3 arm64: kernel: rename __cpu_suspend to keep it aligned with arm

Quoted 19 lines · Catalin Marinas
Quoted 10 lines · Sudeep Holla

This patch renames __cpu_suspend to cpu_suspend so that it's aligned with ARM32. It also removes the redundant wrapper created.

This is in preparation to implement generic PSCI system suspend using the cpu_{suspend,resume} which now has the same interface on both ARM and ARM64.

Quoted 7 lines
---
  arch/arm64/include/asm/cpuidle.h | 8 ++------
  arch/arm64/include/asm/suspend.h | 2 +-
  arch/arm64/kernel/cpuidle.c      | 4 ++--
  arch/arm64/kernel/psci.c         | 2 +-
  arch/arm64/kernel/suspend.c      | 6 +++---
  5 files changed, 9 insertions(+), 13 deletions(-)
Quoted 5 lines

Does this patch depend on the other patches from Mark and Lorenzo or it can be merged earlier?

Though it's independent patch, I think it will conflict with Mark's changes. But since all the ARM64 related changes in that series are already in ARM64 for-next/core, it applies cleanly on it and I even gave it a quick testing. So it can be merged earlier if you are fine with that.

Regards,
Sudeep

maintainer
#10 lore raw

PATCH v2 1/3 arm64: kernel: rename __cpu_suspend to keep it aligned with arm

Quoted 22 lines · Sudeep Holla
Quoted 21 lines · Catalin Marinas
Quoted 10 lines · Sudeep Holla

This patch renames __cpu_suspend to cpu_suspend so that it's aligned with ARM32. It also removes the redundant wrapper created.

This is in preparation to implement generic PSCI system suspend using the cpu_{suspend,resume} which now has the same interface on both ARM and ARM64.

Quoted 7 lines
---
 arch/arm64/include/asm/cpuidle.h | 8 ++------
 arch/arm64/include/asm/suspend.h | 2 +-
 arch/arm64/kernel/cpuidle.c      | 4 ++--
 arch/arm64/kernel/psci.c         | 2 +-
 arch/arm64/kernel/suspend.c      | 6 +++---
 5 files changed, 9 insertions(+), 13 deletions(-)
Quoted 7 lines
Quoted 2 lines

Does this patch depend on the other patches from Mark and Lorenzo or it can be merged earlier?

Though it's independent patch, I think it will conflict with Mark's changes. But since all the ARM64 related changes in that series are already in ARM64 for-next/core, it applies cleanly on it and I even gave it a quick testing. So it can be merged earlier if you are fine with that.

I queued it for 4.2 (it was something already discussed with Lorenzo previously).

Thanks.

Signature · 1 line
Catalin
#11 lore raw

PATCH v2 2/3 drivers: firmware: psci: define more generic PSCI_FN_NATIVE macro

In reply to Sudeep Holla

This patch replaces the definition and usage of PSCI_0_2_FN_NATIVE with the new and more generic macro PSCI_FN_NATIVE that can be used with any version. This will be useful for the new features introduced in PSCIv1.0 and for any future revisions.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
 drivers/firmware/psci.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)
drivers/firmware/psci.c +9 -8
index 68d0c2f6d004..752ca7c9eb97 100644
@@ -29,13 +29,13 @@
 /*
  * While a 64-bit OS can make calls with SMC32 calling conventions, for some
  * calls it is necessary to use SMC64 to pass or return 64-bit values. For such
- * calls PSCI_0_2_FN_NATIVE(x) will choose the appropriate (native-width)
+ * calls PSCI_FN_NATIVE(x) will choose the appropriate (native-width)
  * function ID.
  */
 #ifdef CONFIG_64BIT
-#define PSCI_0_2_FN_NATIVE(name)	PSCI_0_2_FN64_##name
+#define PSCI_FN_NATIVE(version, name)	PSCI_##version##_FN64_##name
 #else
-#define PSCI_0_2_FN_NATIVE(name)	PSCI_0_2_FN_##name
+#define PSCI_FN_NATIVE(version, name)	PSCI_##version##_FN_##name
 #endif
 
 /*
@@ -169,7 +169,7 @@ static int psci_migrate(unsigned long cpuid)
 static int psci_affinity_info(unsigned long target_affinity,
 		unsigned long lowest_affinity_level)
 {
-	return invoke_psci_fn(PSCI_0_2_FN_NATIVE(AFFINITY_INFO),
+	return invoke_psci_fn(PSCI_FN_NATIVE(0_2, AFFINITY_INFO),
 			      target_affinity, lowest_affinity_level, 0);
 }
 
@@ -180,7 +180,7 @@ static int psci_migrate_info_type(void)
 
 static unsigned long psci_migrate_info_up_cpu(void)
 {
-	return invoke_psci_fn(PSCI_0_2_FN_NATIVE(MIGRATE_INFO_UP_CPU),
+	return invoke_psci_fn(PSCI_FN_NATIVE(0_2, MIGRATE_INFO_UP_CPU),
 			      0, 0, 0);
 }
 
@@ -267,16 +267,17 @@ static void __init psci_init_migrate(void)
 static void __init psci_0_2_set_functions(void)
 {
 	pr_info("Using standard PSCI v0.2 function IDs\n");
-	psci_function_id[PSCI_FN_CPU_SUSPEND] = PSCI_0_2_FN_NATIVE(CPU_SUSPEND);
+	psci_function_id[PSCI_FN_CPU_SUSPEND] =
+					PSCI_FN_NATIVE(0_2, CPU_SUSPEND);
 	psci_ops.cpu_suspend = psci_cpu_suspend;
 
 	psci_function_id[PSCI_FN_CPU_OFF] = PSCI_0_2_FN_CPU_OFF;
 	psci_ops.cpu_off = psci_cpu_off;
 
-	psci_function_id[PSCI_FN_CPU_ON] = PSCI_0_2_FN_NATIVE(CPU_ON);
+	psci_function_id[PSCI_FN_CPU_ON] = PSCI_FN_NATIVE(0_2, CPU_ON);
 	psci_ops.cpu_on = psci_cpu_on;
 
-	psci_function_id[PSCI_FN_MIGRATE] = PSCI_0_2_FN_NATIVE(MIGRATE);
+	psci_function_id[PSCI_FN_MIGRATE] = PSCI_FN_NATIVE(0_2, MIGRATE);
 	psci_ops.migrate = psci_migrate;
 
 	psci_ops.affinity_info = psci_affinity_info;
Signature · 1 line
1.9.1
#12 lore raw

PATCH v2 2/3 drivers: firmware: psci: define more generic PSCI_FN_NATIVE macro

Quoted 17 lines · Sudeep Holla

This patch replaces the definition and usage of PSCI_0_2_FN_NATIVE with the new and more generic macro PSCI_FN_NATIVE that can be used with any version. This will be useful for the new features introduced in PSCIv1.0 and for any future revisions.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
 drivers/firmware/psci.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)
drivers/firmware/psci.c +0 -0
index 68d0c2f6d004..752ca7c9eb97 100644

drivers/firmware/psci.c line 32

Quoted 5 lines
drivers/firmware/psci.c +1 -1
@@ -29,13 +29,13 @@
 /*
  * While a 64-bit OS can make calls with SMC32 calling conventions, for some
  * calls it is necessary to use SMC64 to pass or return 64-bit values. For such
- * calls PSCI_0_2_FN_NATIVE(x) will choose the appropriate (native-width)
+ * calls PSCI_FN_NATIVE(x) will choose the appropriate (native-width)

Nit: Comment should be PSCI_FN_NATIVE(version, name), fixed it up in my branch.

Quoted 53 lines
drivers/firmware/psci.c +8 -7
  * function ID.
  */
 #ifdef CONFIG_64BIT
-#define PSCI_0_2_FN_NATIVE(name)	PSCI_0_2_FN64_##name
+#define PSCI_FN_NATIVE(version, name)	PSCI_##version##_FN64_##name
 #else
-#define PSCI_0_2_FN_NATIVE(name)	PSCI_0_2_FN_##name
+#define PSCI_FN_NATIVE(version, name)	PSCI_##version##_FN_##name
 #endif
 
 /*
@@ -169,7 +169,7 @@ static int psci_migrate(unsigned long cpuid)
 static int psci_affinity_info(unsigned long target_affinity,
 		unsigned long lowest_affinity_level)
 {
-	return invoke_psci_fn(PSCI_0_2_FN_NATIVE(AFFINITY_INFO),
+	return invoke_psci_fn(PSCI_FN_NATIVE(0_2, AFFINITY_INFO),
 			      target_affinity, lowest_affinity_level, 0);
 }
 
@@ -180,7 +180,7 @@ static int psci_migrate_info_type(void)
 
 static unsigned long psci_migrate_info_up_cpu(void)
 {
-	return invoke_psci_fn(PSCI_0_2_FN_NATIVE(MIGRATE_INFO_UP_CPU),
+	return invoke_psci_fn(PSCI_FN_NATIVE(0_2, MIGRATE_INFO_UP_CPU),
 			      0, 0, 0);
 }
 
@@ -267,16 +267,17 @@ static void __init psci_init_migrate(void)
 static void __init psci_0_2_set_functions(void)
 {
 	pr_info("Using standard PSCI v0.2 function IDs\n");
-	psci_function_id[PSCI_FN_CPU_SUSPEND] = PSCI_0_2_FN_NATIVE(CPU_SUSPEND);
+	psci_function_id[PSCI_FN_CPU_SUSPEND] =
+					PSCI_FN_NATIVE(0_2, CPU_SUSPEND);
 	psci_ops.cpu_suspend = psci_cpu_suspend;
 
 	psci_function_id[PSCI_FN_CPU_OFF] = PSCI_0_2_FN_CPU_OFF;
 	psci_ops.cpu_off = psci_cpu_off;
 
-	psci_function_id[PSCI_FN_CPU_ON] = PSCI_0_2_FN_NATIVE(CPU_ON);
+	psci_function_id[PSCI_FN_CPU_ON] = PSCI_FN_NATIVE(0_2, CPU_ON);
 	psci_ops.cpu_on = psci_cpu_on;
 
-	psci_function_id[PSCI_FN_MIGRATE] = PSCI_0_2_FN_NATIVE(MIGRATE);
+	psci_function_id[PSCI_FN_MIGRATE] = PSCI_FN_NATIVE(0_2, MIGRATE);
 	psci_ops.migrate = psci_migrate;
 
 	psci_ops.affinity_info = psci_affinity_info;
Signature · 1 line
1.9.1
#13 lore raw

PATCH v2 2/3 drivers: firmware: psci: define more generic PSCI_FN_NATIVE macro

Quoted 13 lines · Lorenzo Pieralisi
Quoted 12 lines · Sudeep Holla

This patch replaces the definition and usage of PSCI_0_2_FN_NATIVE with the new and more generic macro PSCI_FN_NATIVE that can be used with any version. This will be useful for the new features introduced in PSCIv1.0 and for any future revisions.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
  drivers/firmware/psci.c | 17 +++++++++--------
  1 file changed, 9 insertions(+), 8 deletions(-)
Quoted 14 lines

drivers/firmware/psci.c line 32

Quoted 10 lines
drivers/firmware/psci.c +1 -1
index 68d0c2f6d004..752ca7c9eb97 100644
@@ -29,13 +29,13 @@
  /*
   * While a 64-bit OS can make calls with SMC32 calling conventions, for some
   * calls it is necessary to use SMC64 to pass or return 64-bit values. For such
- * calls PSCI_0_2_FN_NATIVE(x) will choose the appropriate (native-width)
+ * calls PSCI_FN_NATIVE(x) will choose the appropriate (native-width)

Nit: Comment should be PSCI_FN_NATIVE(version, name), fixed it up in my branch.

Ah, sorry for missing that.

Quoted 2 lines

Thanks.

Regards,
Sudeep

#14 lore raw

PATCH v2 3/3 drivers: firmware: psci: add system suspend support

In reply to Sudeep Holla

PSCI v1.0 introduces a new API called PSCI_SYSTEM_SUSPEND. This API provides the mechanism by which the calling OS can request entry into the deepest possible system sleep state.

It meets all the necessary preconditions for entering suspend to RAM state in Linux. This patch adds support for PSCI_SYSTEM_SUSPEND in psci firmware and registers a psci system suspend operation to implement the suspend-to-RAM(s2r) in a generic way on all the platforms implementing PSCI.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
 drivers/firmware/psci.c   | 31 +++++++++++++++++++++++++++++++
 include/uapi/linux/psci.h |  3 +++
 2 files changed, 34 insertions(+)
drivers/firmware/psci.c +31 -0
index 752ca7c9eb97..f2b2b3e1c6e4 100644
@@ -20,11 +20,13 @@
 #include <linux/printk.h>
 #include <linux/psci.h>
 #include <linux/reboot.h>
+#include <linux/suspend.h>
 
 #include <uapi/linux/psci.h>
 
 #include <asm/system_misc.h>
 #include <asm/smp_plat.h>
+#include <asm/suspend.h>
 
 /*
  * While a 64-bit OS can make calls with SMC32 calling conventions, for some
@@ -222,6 +224,33 @@ static int __init psci_features(u32 psci_func_id)
 			      psci_func_id, 0, 0);
 }
 
+static int psci_system_suspend(unsigned long unused)
+{
+	return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
+			      virt_to_phys(cpu_resume), 0, 0);
+}
+
+static int psci_system_suspend_enter(suspend_state_t state)
+{
+	return cpu_suspend(0, psci_system_suspend);
+}
+
+static const struct platform_suspend_ops psci_suspend_ops = {
+	.valid          = suspend_valid_only_mem,
+	.enter          = psci_system_suspend_enter,
+};
+
+static void __init psci_init_system_suspend(void)
+{
+	if (!IS_ENABLED(CONFIG_SUSPEND))
+		return;
+
+	if (psci_features(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND)))
+		return;
+
+	suspend_set_ops(&psci_suspend_ops);
+}
+
 static void __init psci_init_cpu_suspend(void)
 {
 	int feature = psci_features(psci_function_id[PSCI_FN_CPU_SUSPEND]);
@@ -311,6 +340,8 @@ static int __init psci_probe(void)
 
 	psci_init_cpu_suspend();
 
+	psci_init_system_suspend();
+
 	return 0;
 }
 
include/uapi/linux/psci.h +3 -0
index 2598d7c9bf20..a1a2d85a91b9 100644
@@ -47,6 +47,9 @@
 #define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU	PSCI_0_2_FN64(7)
 
 #define PSCI_1_0_FN_PSCI_FEATURES		PSCI_0_2_FN(10)
+#define PSCI_1_0_FN_SYSTEM_SUSPEND		PSCI_0_2_FN(14)
+
+#define PSCI_1_0_FN64_SYSTEM_SUSPEND		PSCI_0_2_FN64(14)
 
 /* PSCI v0.2 power state encoding for CPU_SUSPEND function */
 #define PSCI_0_2_POWER_STATE_ID_MASK		0xffff
Signature · 1 line
1.9.1
#15 lore raw

PATCH v2 3/3 drivers: firmware: psci: add system suspend support

Hi Sudeep,

I'm sorry to being late. Just have some comments below.

Quoted 53 lines · On Thu, 18 Jun 2015 15:41:34 +0100 Sudeep Holla <[email protected]> wrote:

PSCI v1.0 introduces a new API called PSCI_SYSTEM_SUSPEND. This API provides the mechanism by which the calling OS can request entry into the deepest possible system sleep state.

It meets all the necessary preconditions for entering suspend to RAM state in Linux. This patch adds support for PSCI_SYSTEM_SUSPEND in psci firmware and registers a psci system suspend operation to implement the suspend-to-RAM(s2r) in a generic way on all the platforms implementing PSCI.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
 drivers/firmware/psci.c   | 31 +++++++++++++++++++++++++++++++
 include/uapi/linux/psci.h |  3 +++
 2 files changed, 34 insertions(+)
drivers/firmware/psci.c +15 -0
index 752ca7c9eb97..f2b2b3e1c6e4 100644
@@ -20,11 +20,13 @@
 #include <linux/printk.h>
 #include <linux/psci.h>
 #include <linux/reboot.h>
+#include <linux/suspend.h>
 
 #include <uapi/linux/psci.h>
 
 #include <asm/system_misc.h>
 #include <asm/smp_plat.h>
+#include <asm/suspend.h>
 
 /*
  * While a 64-bit OS can make calls with SMC32 calling conventions, for some
@@ -222,6 +224,33 @@ static int __init psci_features(u32 psci_func_id)
 			      psci_func_id, 0, 0);
 }
 
+static int psci_system_suspend(unsigned long unused)
+{
+	return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
+			      virt_to_phys(cpu_resume), 0, 0);
+}
+
+static int psci_system_suspend_enter(suspend_state_t state)
+{
+	return cpu_suspend(0, psci_system_suspend);
+}
+
+static const struct platform_suspend_ops psci_suspend_ops = {
+	.valid          = suspend_valid_only_mem,

drivers/firmware/psci.c static int __init psci_features(u32 psci_func_id) · line 249

Quoted 10 lines
drivers/firmware/psci.c +10 -0
+	.enter          = psci_system_suspend_enter,
+};
+
+static void __init psci_init_system_suspend(void)
+{
+	if (!IS_ENABLED(CONFIG_SUSPEND))
+		return;
+
+	if (psci_features(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND)))
+		return;

So this requires the firmware implements SYSTEM_SUSPEND which doesn't exist until PSCI 1.0, and even in PSCI 1.0 SYSTEM_SUSPEND is optional, we also want suspend to ram feature on PSCI 0.2 or PSCI 1.0 w/o SYSTEM_SUSPEND, is there any possibility to bring your previous RFC patch[1] back? Or ARM expect all PSCI users must have SYSTEM_SUSPEND implemented if they want s2ram?

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/318541.html

Thank you,
Jisheng

Quoted 30 lines
drivers/firmware/psci.c +6 -0
+
+	suspend_set_ops(&psci_suspend_ops);
+}
+
 static void __init psci_init_cpu_suspend(void)
 {
 	int feature = psci_features(psci_function_id[PSCI_FN_CPU_SUSPEND]);
@@ -311,6 +340,8 @@ static int __init psci_probe(void)
 
 	psci_init_cpu_suspend();
 
+	psci_init_system_suspend();
+
 	return 0;
 }
 
include/uapi/linux/psci.h +3 -0
index 2598d7c9bf20..a1a2d85a91b9 100644
@@ -47,6 +47,9 @@
 #define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU	PSCI_0_2_FN64(7)
 
 #define PSCI_1_0_FN_PSCI_FEATURES		PSCI_0_2_FN(10)
+#define PSCI_1_0_FN_SYSTEM_SUSPEND		PSCI_0_2_FN(14)
+
+#define PSCI_1_0_FN64_SYSTEM_SUSPEND		PSCI_0_2_FN64(14)
 
 /* PSCI v0.2 power state encoding for CPU_SUSPEND function */
 #define PSCI_0_2_POWER_STATE_ID_MASK		0xffff
#16 lore raw

PATCH v2 3/3 drivers: firmware: psci: add system suspend support

Quoted 65 lines · Jisheng Zhang

Hi Sudeep,

I'm sorry to being late. Just have some comments below.

Quoted 58 lines · On Thu, 18 Jun 2015 15:41:34 +0100 Sudeep Holla <[email protected]> wrote:

PSCI v1.0 introduces a new API called PSCI_SYSTEM_SUSPEND. This API provides the mechanism by which the calling OS can request entry into the deepest possible system sleep state.

It meets all the necessary preconditions for entering suspend to RAM state in Linux. This patch adds support for PSCI_SYSTEM_SUSPEND in psci firmware and registers a psci system suspend operation to implement the suspend-to-RAM(s2r) in a generic way on all the platforms implementing PSCI.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
  drivers/firmware/psci.c   | 31 +++++++++++++++++++++++++++++++
  include/uapi/linux/psci.h |  3 +++
  2 files changed, 34 insertions(+)
drivers/firmware/psci.c +20 -0
index 752ca7c9eb97..f2b2b3e1c6e4 100644
@@ -20,11 +20,13 @@
  #include <linux/printk.h>
  #include <linux/psci.h>
  #include <linux/reboot.h>
+#include <linux/suspend.h>
 
  #include <uapi/linux/psci.h>
 
  #include <asm/system_misc.h>
  #include <asm/smp_plat.h>
+#include <asm/suspend.h>
 
  /*
   * While a 64-bit OS can make calls with SMC32 calling conventions, for some
@@ -222,6 +224,33 @@ static int __init psci_features(u32 psci_func_id)
  			      psci_func_id, 0, 0);
  }
 
+static int psci_system_suspend(unsigned long unused)
+{
+	return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
+			      virt_to_phys(cpu_resume), 0, 0);
+}
+
+static int psci_system_suspend_enter(suspend_state_t state)
+{
+	return cpu_suspend(0, psci_system_suspend);
+}
+
+static const struct platform_suspend_ops psci_suspend_ops = {
+	.valid          = suspend_valid_only_mem,
+	.enter          = psci_system_suspend_enter,
+};
+
+static void __init psci_init_system_suspend(void)
+{
Quoted 8 lines

drivers/firmware/psci.c static int __init psci_features(u32 psci_func_id) · line 249

Quoted 5 lines
drivers/firmware/psci.c +5 -0
+	if (!IS_ENABLED(CONFIG_SUSPEND))
+		return;
+
+	if (psci_features(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND)))
+		return;

So this requires the firmware implements SYSTEM_SUSPEND which doesn't exist until PSCI 1.0,

Correct, if you need System to RAM in Linux on a platform with PSCI firmware, firmware *must implement* SYSTEM_SUSPEND.

Quoted 1 line

and even in PSCI 1.0 SYSTEM_SUSPEND is optional,

Optional here means, that you may chose to implement or not in the firmware. It doesn't mean we can implement S2R in Linux using other PSCI methods. SYSTEM_SUSPEND was added mainly to avoid since workarounds in the kernel.

Quoted 2 lines

we also want
suspend to ram feature on PSCI 0.2 or PSCI 1.0 w/o SYSTEM_SUSPEND,

Why do you choose to have PSCIv1.0 w/o SYSTEM_SUSPEND when you need that feature in Linux. Is it just because we can manage with workarounds ? Sorry, that's not an option.

Quoted 3 lines

is there
any possibility to bring your previous RFC patch[1] back? Or ARM expect all PSCI users must have SYSTEM_SUSPEND implemented if they want s2ram?

OK, we did expect response back then[1] when we clearly asked if the firmware can be upgraded to v1.0 from v0.2 as the delta is not much and helps up to avoid special DT bindings for that case. Since nobody responded for last 6 months, it's assumed that it's no longer required.

So to answer you, no the RFC is dead and it's expected to have PSCI firmware support for SYSTEM_SUSPEND if S2R is desired feature in Linux.

Regards,
Sudeep

[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/322278.html

#17 lore raw

PATCH v2 3/3 drivers: firmware: psci: add system suspend support

Dear Sudeep,

Quoted 87 lines · On Tue, 14 Jul 2015 10:14:25 +0100 Sudeep Holla <[email protected]> wrote:
Quoted 65 lines · Jisheng Zhang

Hi Sudeep,

I'm sorry to being late. Just have some comments below.

Quoted 58 lines · On Thu, 18 Jun 2015 15:41:34 +0100 Sudeep Holla <[email protected]> wrote:

PSCI v1.0 introduces a new API called PSCI_SYSTEM_SUSPEND. This API provides the mechanism by which the calling OS can request entry into the deepest possible system sleep state.

It meets all the necessary preconditions for entering suspend to RAM state in Linux. This patch adds support for PSCI_SYSTEM_SUSPEND in psci firmware and registers a psci system suspend operation to implement the suspend-to-RAM(s2r) in a generic way on all the platforms implementing PSCI.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
  drivers/firmware/psci.c   | 31 +++++++++++++++++++++++++++++++
  include/uapi/linux/psci.h |  3 +++
  2 files changed, 34 insertions(+)
drivers/firmware/psci.c +20 -0
index 752ca7c9eb97..f2b2b3e1c6e4 100644
@@ -20,11 +20,13 @@
  #include <linux/printk.h>
  #include <linux/psci.h>
  #include <linux/reboot.h>
+#include <linux/suspend.h>
 
  #include <uapi/linux/psci.h>
 
  #include <asm/system_misc.h>
  #include <asm/smp_plat.h>
+#include <asm/suspend.h>
 
  /*
   * While a 64-bit OS can make calls with SMC32 calling conventions, for some
@@ -222,6 +224,33 @@ static int __init psci_features(u32 psci_func_id)
  			      psci_func_id, 0, 0);
  }
 
+static int psci_system_suspend(unsigned long unused)
+{
+	return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
+			      virt_to_phys(cpu_resume), 0, 0);
+}
+
+static int psci_system_suspend_enter(suspend_state_t state)
+{
+	return cpu_suspend(0, psci_system_suspend);
+}
+
+static const struct platform_suspend_ops psci_suspend_ops = {
+	.valid          = suspend_valid_only_mem,
+	.enter          = psci_system_suspend_enter,
+};
+
+static void __init psci_init_system_suspend(void)
+{
Quoted 8 lines

drivers/firmware/psci.c static int __init psci_features(u32 psci_func_id) · line 249

Quoted 5 lines
drivers/firmware/psci.c +5 -0
+	if (!IS_ENABLED(CONFIG_SUSPEND))
+		return;
+
+	if (psci_features(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND)))
+		return;

So this requires the firmware implements SYSTEM_SUSPEND which doesn't exist until PSCI 1.0,

Correct, if you need System to RAM in Linux on a platform with PSCI firmware, firmware *must implement* SYSTEM_SUSPEND.

Quoted 1 line

and even in PSCI 1.0 SYSTEM_SUSPEND is optional,

Optional here means, that you may chose to implement or not in the firmware. It doesn't mean we can implement S2R in Linux using other PSCI methods. SYSTEM_SUSPEND was added mainly to avoid since workarounds in the kernel.

Quoted 6 lines
Quoted 2 lines

we also want
suspend to ram feature on PSCI 0.2 or PSCI 1.0 w/o SYSTEM_SUSPEND,

Why do you choose to have PSCIv1.0 w/o SYSTEM_SUSPEND when you need that feature in Linux. Is it just because we can manage with workarounds ? Sorry, that's not an option.

The problem is the PSCI 1.0 SYSTEM_SUSPEND definition: we can't pass something like stateid as in CPU_SUSPEND to firmware. The stateid is used to tell firmware to go to different Sleep state, for example S2 or S3.

With your RFC patch, it's easy to extend to support this feature, but if we are limited to SYSTEM_SUSPEND in PSCI 1.0, we have to use a different code path for non-S3 sleep state.

Is there any elegant solution?

Thanks,
Jisheng

Quoted 18 lines
Quoted 4 lines

is there
any possibility to bring your previous RFC patch[1] back? Or ARM expect all PSCI users must have SYSTEM_SUSPEND implemented if they want s2ram?

OK, we did expect response back then[1] when we clearly asked if the firmware can be upgraded to v1.0 from v0.2 as the delta is not much and helps up to avoid special DT bindings for that case. Since nobody responded for last 6 months, it's assumed that it's no longer required.

So to answer you, no the RFC is dead and it's expected to have PSCI firmware support for SYSTEM_SUSPEND if S2R is desired feature in Linux.

Regards,
Sudeep

[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/322278.html

#18 lore raw

PATCH v2 3/3 drivers: firmware: psci: add system suspend support

Quoted 95 lines · Jisheng Zhang

Dear Sudeep,

Quoted 90 lines · On Tue, 14 Jul 2015 10:14:25 +0100 Sudeep Holla <[email protected]> wrote:
Quoted 65 lines · Jisheng Zhang

Hi Sudeep,

I'm sorry to being late. Just have some comments below.

Quoted 58 lines · On Thu, 18 Jun 2015 15:41:34 +0100 Sudeep Holla <[email protected]> wrote:

PSCI v1.0 introduces a new API called PSCI_SYSTEM_SUSPEND. This API provides the mechanism by which the calling OS can request entry into the deepest possible system sleep state.

It meets all the necessary preconditions for entering suspend to RAM state in Linux. This patch adds support for PSCI_SYSTEM_SUSPEND in psci firmware and registers a psci system suspend operation to implement the suspend-to-RAM(s2r) in a generic way on all the platforms implementing PSCI.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
   drivers/firmware/psci.c   | 31 +++++++++++++++++++++++++++++++
   include/uapi/linux/psci.h |  3 +++
   2 files changed, 34 insertions(+)
drivers/firmware/psci.c +20 -0
index 752ca7c9eb97..f2b2b3e1c6e4 100644
@@ -20,11 +20,13 @@
   #include <linux/printk.h>
   #include <linux/psci.h>
   #include <linux/reboot.h>
+#include <linux/suspend.h>
 
   #include <uapi/linux/psci.h>
 
   #include <asm/system_misc.h>
   #include <asm/smp_plat.h>
+#include <asm/suspend.h>
 
   /*
    * While a 64-bit OS can make calls with SMC32 calling conventions, for some
@@ -222,6 +224,33 @@ static int __init psci_features(u32 psci_func_id)
   			      psci_func_id, 0, 0);
   }
 
+static int psci_system_suspend(unsigned long unused)
+{
+	return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
+			      virt_to_phys(cpu_resume), 0, 0);
+}
+
+static int psci_system_suspend_enter(suspend_state_t state)
+{
+	return cpu_suspend(0, psci_system_suspend);
+}
+
+static const struct platform_suspend_ops psci_suspend_ops = {
+	.valid          = suspend_valid_only_mem,
+	.enter          = psci_system_suspend_enter,
+};
+
+static void __init psci_init_system_suspend(void)
+{
Quoted 8 lines

drivers/firmware/psci.c static int __init psci_features(u32 psci_func_id) · line 249

Quoted 5 lines
drivers/firmware/psci.c +5 -0
+	if (!IS_ENABLED(CONFIG_SUSPEND))
+		return;
+
+	if (psci_features(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND)))
+		return;

So this requires the firmware implements SYSTEM_SUSPEND which doesn't exist until PSCI 1.0,

Correct, if you need System to RAM in Linux on a platform with PSCI firmware, firmware *must implement* SYSTEM_SUSPEND.

Quoted 1 line

and even in PSCI 1.0 SYSTEM_SUSPEND is optional,

Optional here means, that you may chose to implement or not in the firmware. It doesn't mean we can implement S2R in Linux using other PSCI methods. SYSTEM_SUSPEND was added mainly to avoid since workarounds in the kernel.

Quoted 2 lines

we also want
suspend to ram feature on PSCI 0.2 or PSCI 1.0 w/o SYSTEM_SUSPEND,

Quoted 8 lines
Quoted 3 lines

Why do you choose to have PSCIv1.0 w/o SYSTEM_SUSPEND when you need that feature in Linux. Is it just because we can manage with workarounds ? Sorry, that's not an option.

The problem is the PSCI 1.0 SYSTEM_SUSPEND definition: we can't pass something like stateid as in CPU_SUSPEND to firmware. The stateid is used to tell firmware to go to different Sleep state, for example S2 or S3.

Can you elaborate on S2 here with an example ? You are using ACPI Sleep State definitions here which should not be mixed with PSCI.

*Let's refrain ourselves from using ACPI S-states here* until it's well defined on ARM systems. The current definition is x86 based and doesn't suit ARM at all.

It's difficult to distinguish between the intermediate system suspend sleep states you are referring and the composite cpu idle states. That's the reason I am asking for the example to see if the sleep states you referring can be considered as composite cpu idle states.
I strongly believe that's the case.

I would consider any other states as composite CPU idle states. In order to utilize Linux PM features, we would use PSCI functions as below.

/-----------------------------------\

| PSCI Feature   | Linux PM feature |
|-----------------------------------|
| SYSTEM_SUSPEND |  Suspend-to-RAM  |
|-----------------------------------|
| SYSTEM_SHUTDOWN|  Suspend-to-Disk |

\-----------------------------------/

Quoted 4 lines

With your RFC patch, it's easy to extend to support this feature, but if we are limited to SYSTEM_SUSPEND in PSCI 1.0, we have to use a different code path for non-S3 sleep state.

As I said above can you provide examples for such states.

Regards,
Sudeep

#19 lore raw

PATCH v2 3/3 drivers: firmware: psci: add system suspend support

Dear Sudeep,

Quoted 102 lines · On Tue, 14 Jul 2015 12:02:15 +0100 Sudeep Holla <[email protected]> wrote:
Quoted 99 lines · Jisheng Zhang

Dear Sudeep,

Quoted 93 lines · On Tue, 14 Jul 2015 10:14:25 +0100 Sudeep Holla <[email protected]> wrote:
Quoted 65 lines · Jisheng Zhang

Hi Sudeep,

I'm sorry to being late. Just have some comments below.

Quoted 58 lines · On Thu, 18 Jun 2015 15:41:34 +0100 Sudeep Holla <[email protected]> wrote:

PSCI v1.0 introduces a new API called PSCI_SYSTEM_SUSPEND. This API provides the mechanism by which the calling OS can request entry into the deepest possible system sleep state.

It meets all the necessary preconditions for entering suspend to RAM state in Linux. This patch adds support for PSCI_SYSTEM_SUSPEND in psci firmware and registers a psci system suspend operation to implement the suspend-to-RAM(s2r) in a generic way on all the platforms implementing PSCI.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
   drivers/firmware/psci.c   | 31 +++++++++++++++++++++++++++++++
   include/uapi/linux/psci.h |  3 +++
   2 files changed, 34 insertions(+)
drivers/firmware/psci.c +20 -0
index 752ca7c9eb97..f2b2b3e1c6e4 100644
@@ -20,11 +20,13 @@
   #include <linux/printk.h>
   #include <linux/psci.h>
   #include <linux/reboot.h>
+#include <linux/suspend.h>
 
   #include <uapi/linux/psci.h>
 
   #include <asm/system_misc.h>
   #include <asm/smp_plat.h>
+#include <asm/suspend.h>
 
   /*
    * While a 64-bit OS can make calls with SMC32 calling conventions, for some
@@ -222,6 +224,33 @@ static int __init psci_features(u32 psci_func_id)
   			      psci_func_id, 0, 0);
   }
 
+static int psci_system_suspend(unsigned long unused)
+{
+	return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
+			      virt_to_phys(cpu_resume), 0, 0);
+}
+
+static int psci_system_suspend_enter(suspend_state_t state)
+{
+	return cpu_suspend(0, psci_system_suspend);
+}
+
+static const struct platform_suspend_ops psci_suspend_ops = {
+	.valid          = suspend_valid_only_mem,
+	.enter          = psci_system_suspend_enter,
+};
+
+static void __init psci_init_system_suspend(void)
+{
Quoted 8 lines

drivers/firmware/psci.c static int __init psci_features(u32 psci_func_id) · line 249

Quoted 5 lines
drivers/firmware/psci.c +5 -0
+	if (!IS_ENABLED(CONFIG_SUSPEND))
+		return;
+
+	if (psci_features(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND)))
+		return;

So this requires the firmware implements SYSTEM_SUSPEND which doesn't exist until PSCI 1.0,

Correct, if you need System to RAM in Linux on a platform with PSCI firmware, firmware *must implement* SYSTEM_SUSPEND.

Quoted 1 line

and even in PSCI 1.0 SYSTEM_SUSPEND is optional,

Optional here means, that you may chose to implement or not in the firmware. It doesn't mean we can implement S2R in Linux using other PSCI methods. SYSTEM_SUSPEND was added mainly to avoid since workarounds in the kernel.

Quoted 2 lines

we also want
suspend to ram feature on PSCI 0.2 or PSCI 1.0 w/o SYSTEM_SUSPEND,

Why do you choose to have PSCIv1.0 w/o SYSTEM_SUSPEND when you need that feature in Linux. Is it just because we can manage with workarounds ? Sorry, that's not an option.

Quoted 7 lines
Quoted 4 lines

The problem is the PSCI 1.0 SYSTEM_SUSPEND definition: we can't pass something like stateid as in CPU_SUSPEND to firmware. The stateid is used to tell firmware to go to different Sleep state, for example S2 or S3.

Can you elaborate on S2 here with an example ? You are using ACPI Sleep State definitions here which should not be mixed with PSCI.

Yes, it's ACPI concept. Here "S2" can be the "S2" mentioned in PSCI 1.0 spec.

PSCI 1.0 SPEC says:

"While ACPI distinguishes between two system level suspend to RAM states, S2 and S3, PSCI provides a single API. In practice, operating systems use only one suspend to RAM state, so this is not seen as a limitation. Note that this specification is using the state definitions of S2 and S3 provided by ACPI, but does not limit use of this PSCI function to ACPI based systems. Semantics such as those described for sleep states S2 and S3 are used in non-ACPI based systems."

Here is an example:

S3 is the state where All devices are suspended and power down, memory is placed in self-refresh mode. Usually this is the suspend to ram state.

S2 is the state where devices except cpus are suspended and put in a low-power state(not power off), if the deivce doesn't support lower power mode, the device is left on. memory is placed in self-refresh mode. secondary CPUs are powered off via. PSCI_CPU_OFF, boot cpu is put into WFI state and runs at low freq.

Thanks,
Jisheng

Quoted 32 lines

*Let's refrain ourselves from using ACPI S-states here* until it's well defined on ARM systems. The current definition is x86 based and doesn't suit ARM at all.

It's difficult to distinguish between the intermediate system suspend sleep states you are referring and the composite cpu idle states. That's the reason I am asking for the example to see if the sleep states you referring can be considered as composite cpu idle states.
I strongly believe that's the case.

I would consider any other states as composite CPU idle states. In order to utilize Linux PM features, we would use PSCI functions as below.

/-----------------------------------\

| PSCI Feature   | Linux PM feature |
|-----------------------------------|
| SYSTEM_SUSPEND |  Suspend-to-RAM  |
|-----------------------------------|
| SYSTEM_SHUTDOWN|  Suspend-to-Disk |

\-----------------------------------/

Quoted 4 lines

With your RFC patch, it's easy to extend to support this feature, but if we are limited to SYSTEM_SUSPEND in PSCI 1.0, we have to use a different code path for non-S3 sleep state.

As I said above can you provide examples for such states.

Regards,
Sudeep

#20 lore raw

PATCH v2 3/3 drivers: firmware: psci: add system suspend support

Quoted 119 lines · Jisheng Zhang

Dear Sudeep,

Quoted 102 lines · On Tue, 14 Jul 2015 12:02:15 +0100 Sudeep Holla <[email protected]> wrote:
Quoted 99 lines · Jisheng Zhang

Dear Sudeep,

Quoted 93 lines · On Tue, 14 Jul 2015 10:14:25 +0100 Sudeep Holla <[email protected]> wrote:
Quoted 65 lines · Jisheng Zhang

Hi Sudeep,

I'm sorry to being late. Just have some comments below.

Quoted 58 lines · On Thu, 18 Jun 2015 15:41:34 +0100 Sudeep Holla <[email protected]> wrote:

PSCI v1.0 introduces a new API called PSCI_SYSTEM_SUSPEND. This API provides the mechanism by which the calling OS can request entry into the deepest possible system sleep state.

It meets all the necessary preconditions for entering suspend to RAM state in Linux. This patch adds support for PSCI_SYSTEM_SUSPEND in psci firmware and registers a psci system suspend operation to implement the suspend-to-RAM(s2r) in a generic way on all the platforms implementing PSCI.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
    drivers/firmware/psci.c   | 31 +++++++++++++++++++++++++++++++
    include/uapi/linux/psci.h |  3 +++
    2 files changed, 34 insertions(+)
drivers/firmware/psci.c +20 -0
index 752ca7c9eb97..f2b2b3e1c6e4 100644
@@ -20,11 +20,13 @@
    #include <linux/printk.h>
    #include <linux/psci.h>
    #include <linux/reboot.h>
+#include <linux/suspend.h>
 
    #include <uapi/linux/psci.h>
 
    #include <asm/system_misc.h>
    #include <asm/smp_plat.h>
+#include <asm/suspend.h>
 
    /*
     * While a 64-bit OS can make calls with SMC32 calling conventions, for some
@@ -222,6 +224,33 @@ static int __init psci_features(u32 psci_func_id)
    			      psci_func_id, 0, 0);
    }
 
+static int psci_system_suspend(unsigned long unused)
+{
+	return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
+			      virt_to_phys(cpu_resume), 0, 0);
+}
+
+static int psci_system_suspend_enter(suspend_state_t state)
+{
+	return cpu_suspend(0, psci_system_suspend);
+}
+
+static const struct platform_suspend_ops psci_suspend_ops = {
+	.valid          = suspend_valid_only_mem,
+	.enter          = psci_system_suspend_enter,
+};
+
+static void __init psci_init_system_suspend(void)
+{
Quoted 8 lines

drivers/firmware/psci.c static int __init psci_features(u32 psci_func_id) · line 249

Quoted 5 lines
drivers/firmware/psci.c +5 -0
+	if (!IS_ENABLED(CONFIG_SUSPEND))
+		return;
+
+	if (psci_features(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND)))
+		return;

So this requires the firmware implements SYSTEM_SUSPEND which doesn't exist until PSCI 1.0,

Correct, if you need System to RAM in Linux on a platform with PSCI firmware, firmware *must implement* SYSTEM_SUSPEND.

Quoted 1 line

and even in PSCI 1.0 SYSTEM_SUSPEND is optional,

Optional here means, that you may chose to implement or not in the firmware. It doesn't mean we can implement S2R in Linux using other PSCI methods. SYSTEM_SUSPEND was added mainly to avoid since workarounds in the kernel.

Quoted 2 lines

we also want
suspend to ram feature on PSCI 0.2 or PSCI 1.0 w/o SYSTEM_SUSPEND,

Why do you choose to have PSCIv1.0 w/o SYSTEM_SUSPEND when you need that feature in Linux. Is it just because we can manage with workarounds ? Sorry, that's not an option.

Quoted 7 lines
Quoted 4 lines

The problem is the PSCI 1.0 SYSTEM_SUSPEND definition: we can't pass something like stateid as in CPU_SUSPEND to firmware. The stateid is used to tell firmware to go to different Sleep state, for example S2 or S3.

Can you elaborate on S2 here with an example ? You are using ACPI Sleep State definitions here which should not be mixed with PSCI.

Yes, it's ACPI concept. Here "S2" can be the "S2" mentioned in PSCI 1.0 spec.

PSCI 1.0 SPEC says:

Quoted 8 lines

"While ACPI distinguishes between two system level suspend to RAM states, S2 and S3, PSCI provides a single API. In practice, operating systems use only one suspend to RAM state, so this is not seen as a limitation. Note that this specification is using the state definitions of S2 and S3 provided by ACPI, but does not limit use of this PSCI function to ACPI based systems. Semantics such as those described for sleep states S2 and S3 are used in non-ACPI based systems."

OK, though similar semantics may be used else where, we need to define the state similar to the way ACPI does it for x86 systems.

Quoted 5 lines

Here is an example:

S3 is the state where All devices are suspended and power down, memory is placed in self-refresh mode. Usually this is the suspend to ram state.

Agreed.

Quoted 6 lines

S2 is the state where devices except cpus are suspended and put in a low-power state(not power off), if the deivce doesn't support lower power mode, the device is left on. memory is placed in self-refresh mode. secondary CPUs are powered off via. PSCI_CPU_OFF, boot cpu is put into WFI state and runs at low freq.

Is this a standard state or custom tailored for some power optimization on particular system ? Anyway I am not sure if I quite understand the exact definition of this state. When and how often do you use this use this state ?

Have you looked at PM_SUSPEND_FREEZE state implemented in Linux ? For me this S2 you explained sounds similar to PM_SUSPEND_FREEZE state. In short, PM_SUSPEND_FREEZE = all the processes are frozen + all the devices are suspended + all the processors enter deepest idle state possible.

Though I don't understand why you need to power-off secondary cpus and stay in WFI on boot cpu as that would have increased latency defeating the purpose of S2 state. Also why you are mixing DVFS here ? PSCI deals with just low power states and has nothing to do with DVFS.

Further IMO we can also achieve a low power state almost close to PM_SUSPEND_FREEZE having run-time PM for all the devices and cpuidle.

Regards,
Sudeep

#21 lore raw

PATCH v2 3/3 drivers: firmware: psci: add system suspend support

Dear Sudeep,

Quoted 68 lines · On Tue, 14 Jul 2015 14:18:10 +0100 Sudeep Holla wrote:
Quoted 41 lines
Quoted 29 lines
Quoted 29 lines
Quoted 28 lines
Quoted 11 lines

Review

Quoted 8 lines
(quoted diff) +8 -0
+
+static void __init psci_init_system_suspend(void)
+{
+	if (!IS_ENABLED(CONFIG_SUSPEND))
+		return;
+
+	if (psci_features(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND)))
+		return;

So this requires the firmware implements SYSTEM_SUSPEND which doesn't exist until PSCI 1.0,

Correct, if you need System to RAM in Linux on a platform with PSCI firmware, firmware *must implement* SYSTEM_SUSPEND.

Quoted 1 line

and even in PSCI 1.0 SYSTEM_SUSPEND is optional,

Optional here means, that you may chose to implement or not in the firmware. It doesn't mean we can implement S2R in Linux using other PSCI methods. SYSTEM_SUSPEND was added mainly to avoid since workarounds in the kernel.

Quoted 2 lines

we also want
suspend to ram feature on PSCI 0.2 or PSCI 1.0 w/o SYSTEM_SUSPEND,

Why do you choose to have PSCIv1.0 w/o SYSTEM_SUSPEND when you need that feature in Linux. Is it just because we can manage with workarounds ? Sorry, that's not an option.

Quoted 7 lines
Quoted 4 lines

The problem is the PSCI 1.0 SYSTEM_SUSPEND definition: we can't pass something like stateid as in CPU_SUSPEND to firmware. The stateid is used to tell firmware to go to different Sleep state, for example S2 or S3.

Can you elaborate on S2 here with an example ? You are using ACPI Sleep State definitions here which should not be mixed with PSCI.

Yes, it's ACPI concept. Here "S2" can be the "S2" mentioned in PSCI 1.0 spec.

PSCI 1.0 SPEC says:

Quoted 8 lines

"While ACPI distinguishes between two system level suspend to RAM states, S2 and S3, PSCI provides a single API. In practice, operating systems use only one suspend to RAM state, so this is not seen as a limitation. Note that this specification is using the state definitions of S2 and S3 provided by ACPI, but does not limit use of this PSCI function to ACPI based systems. Semantics such as those described for sleep states S2 and S3 are used in non-ACPI based systems."

OK, though similar semantics may be used else where, we need to define the state similar to the way ACPI does it for x86 systems.

Quoted 5 lines

Here is an example:

S3 is the state where All devices are suspended and power down, memory is placed in self-refresh mode. Usually this is the suspend to ram state.

Agreed.

Quoted 6 lines

S2 is the state where devices except cpus are suspended and put in a low-power state(not power off), if the deivce doesn't support lower power mode, the device is left on. memory is placed in self-refresh mode. secondary CPUs are powered off via. PSCI_CPU_OFF, boot cpu is put into WFI state and runs at low freq.

Quoted 2 lines

Is this a standard state or custom tailored for some power optimization on particular system ? Anyway I am not sure if I quite understand the

This is customized state.

Quoted 2 lines

exact definition of this state. When and how often do you use this use this state ?

Very often. When we want s2ram similar state but low-latency back transition, we want to use this state.

Quoted 6 lines

Have you looked at PM_SUSPEND_FREEZE state implemented in Linux ? For me this S2 you explained sounds similar to PM_SUSPEND_FREEZE state. In short, PM_SUSPEND_FREEZE = all the processes are frozen + all the devices are suspended + all the processors enter deepest idle state possible.

The S2 in my example is a bit different with freeze. It's looks more like the S1 or standby in Documentation/power/states.txt. The biggest difference with freeze is whether putting memory in self-refresh state or not.

Quoted 3 lines

Though I don't understand why you need to power-off secondary cpus and stay in WFI on boot cpu as that would have increased latency

we want save more power, the increased latency is affordable.

Quoted 2 lines

defeating the purpose of S2 state. Also why you are mixing DVFS here ? PSCI deals with just low power states and has nothing to do with DVFS.

It's not DVFS related, but to put the boot cpu in a low power state while still keeping its power.

Quoted 3 lines

Further IMO we can also achieve a low power state almost close to PM_SUSPEND_FREEZE having run-time PM for all the devices and cpuidle.

we want one more sleep state which saves more power than freeze.

Or let's change the question: how to implement "standby" or "s1" in Documentation/power/states.txt with the help of PSCI? Seems the PSCI spec expects "operating systems use only one suspend to RAM state", but this is a limitation in the long run.

Thanks,
Jisheng

#22 lore raw

PATCH v2 3/3 drivers: firmware: psci: add system suspend support

Hi Jisheng,

Quoted 4 lines · Jisheng Zhang

Dear Sudeep,

On Tue, 14 Jul 2015 14:18:10 +0100
Sudeep Holla wrote:

[...]

Quoted 19 lines
Quoted 19 lines

OK, though similar semantics may be used else where, we need to define the state similar to the way ACPI does it for x86 systems.

Quoted 5 lines

Here is an example:

S3 is the state where All devices are suspended and power down, memory is placed in self-refresh mode. Usually this is the suspend to ram state.

Agreed.

Quoted 6 lines

S2 is the state where devices except cpus are suspended and put in a low-power state(not power off), if the deivce doesn't support lower power mode, the device is left on. memory is placed in self-refresh mode. secondary CPUs are powered off via. PSCI_CPU_OFF, boot cpu is put into WFI state and runs at low freq.

Quoted 5 lines
Quoted 2 lines

Is this a standard state or custom tailored for some power optimization on particular system ? Anyway I am not sure if I quite understand the

This is customized state.

OK, but I do have concern as you mention that you leave certain device on, what if that device initiate some DMA operation which might not be possible when DDR is in self-refresh mode. IMO this is highly customized state.

Quoted 6 lines
Quoted 2 lines

exact definition of this state. When and how often do you use this use this state ?

Very often. When we want s2ram similar state but low-latency back transition, we want to use this state.

I don't understand this. You want secondaries hot-plugged out which is quite expensive yet you mention this is low-latency. That could be possible as you leave last CPU in WFI but again highly customized state.

Quoted 11 lines
Quoted 6 lines

Have you looked at PM_SUSPEND_FREEZE state implemented in Linux ? For me this S2 you explained sounds similar to PM_SUSPEND_FREEZE state. In short, PM_SUSPEND_FREEZE = all the processes are frozen + all the devices are suspended + all the processors enter deepest idle state possible.

The S2 in my example is a bit different with freeze. It's looks more like the S1 or standby in Documentation/power/states.txt. The biggest difference with freeze is whether putting memory in self-refresh state or not.

OK I am still struggling to map these(S1/S2) to ARM systems as they designed and map well for x86 systems. Since the idle states on include state where CPU power is lost, I wonder if the state you are explaining provides any more power save compared to system freeze.

Quoted 6 lines
Quoted 3 lines

Though I don't understand why you need to power-off secondary cpus and stay in WFI on boot cpu as that would have increased latency

we want save more power, the increased latency is affordable.

Which clearly contradicts what you said above.

Quoted 6 lines
Quoted 2 lines

defeating the purpose of S2 state. Also why you are mixing DVFS here ? PSCI deals with just low power states and has nothing to do with DVFS.

It's not DVFS related, but to put the boot cpu in a low power state while still keeping its power.

If there's nothing for CPU to do, some CPUFreq governors(e.g. interactive) will ensure lowest OPP is chosen so you need not do anything extra I believe.

Quoted 6 lines
Quoted 3 lines

Further IMO we can also achieve a low power state almost close to PM_SUSPEND_FREEZE having run-time PM for all the devices and cpuidle.

we want one more sleep state which saves more power than freeze.

Do you have any power/latency number that indicates freeze is not that power efficient ? Also I imagine you S2 state has higher latency as you are doing CPU hot-plug.

Quoted 5 lines

Or let's change the question: how to implement "standby" or "s1" in Documentation/power/states.txt with the help of PSCI? Seems the PSCI spec expects "operating systems use only one suspend to RAM state", but this is a limitation in the long run.

OK, this one I leave it for you to take up discussion in PSCI forum with justification(power/latency numbers to prove an extra system state is really worthy)

Regards,
Sudeep

#23 lore raw

PATCH v2 3/3 drivers: firmware: psci: add system suspend support

In reply to Sudeep Holla
Quoted 55 lines · Sudeep Holla

PSCI v1.0 introduces a new API called PSCI_SYSTEM_SUSPEND. This API provides the mechanism by which the calling OS can request entry into the deepest possible system sleep state.

It meets all the necessary preconditions for entering suspend to RAM state in Linux. This patch adds support for PSCI_SYSTEM_SUSPEND in psci firmware and registers a psci system suspend operation to implement the suspend-to-RAM(s2r) in a generic way on all the platforms implementing PSCI.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
 drivers/firmware/psci.c   | 31 +++++++++++++++++++++++++++++++
 include/uapi/linux/psci.h |  3 +++
 2 files changed, 34 insertions(+)
drivers/firmware/psci.c +17 -0
index 752ca7c9eb97..f2b2b3e1c6e4 100644
@@ -20,11 +20,13 @@
 #include <linux/printk.h>
 #include <linux/psci.h>
 #include <linux/reboot.h>
+#include <linux/suspend.h>
 
 #include <uapi/linux/psci.h>
 
 #include <asm/system_misc.h>
 #include <asm/smp_plat.h>
+#include <asm/suspend.h>
 
 /*
  * While a 64-bit OS can make calls with SMC32 calling conventions, for some
@@ -222,6 +224,33 @@ static int __init psci_features(u32 psci_func_id)
 			      psci_func_id, 0, 0);
 }
 
+static int psci_system_suspend(unsigned long unused)
+{
+	return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
+			      virt_to_phys(cpu_resume), 0, 0);
+}
+
+static int psci_system_suspend_enter(suspend_state_t state)
+{
+	return cpu_suspend(0, psci_system_suspend);
+}
+
+static const struct platform_suspend_ops psci_suspend_ops = {
+	.valid          = suspend_valid_only_mem,
+	.enter          = psci_system_suspend_enter,
+};

drivers/firmware/psci.c static int __init psci_features(u32 psci_func_id) · line 251

Quoted 10 lines
drivers/firmware/psci.c +10 -0
+
+static void __init psci_init_system_suspend(void)
+{
+	if (!IS_ENABLED(CONFIG_SUSPEND))
+		return;
+
+	if (psci_features(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND)))
+		return;
+
+	suspend_set_ops(&psci_suspend_ops);

For consistency with psci_init_cpu_suspend I rewrote it like this:

static void __init psci_init_system_suspend(void)

{
	int ret;

	if (!IS_ENABLED(CONFIG_SUSPEND))
		return;

	ret = psci_features(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND));

	if (ret != PSCI_RET_NOT_SUPPORTED)
		suspend_set_ops(&psci_suspend_ops);
}

Let me know if it is ok with you. Other than that:

#24 lore raw

PATCH v2 3/3 drivers: firmware: psci: add system suspend support

Quoted 82 lines · Lorenzo Pieralisi
Quoted 55 lines · Sudeep Holla

PSCI v1.0 introduces a new API called PSCI_SYSTEM_SUSPEND. This API provides the mechanism by which the calling OS can request entry into the deepest possible system sleep state.

It meets all the necessary preconditions for entering suspend to RAM state in Linux. This patch adds support for PSCI_SYSTEM_SUSPEND in psci firmware and registers a psci system suspend operation to implement the suspend-to-RAM(s2r) in a generic way on all the platforms implementing PSCI.

Cc Mark Rutland <[email protected]>
Cc Lorenzo Pieralisi <[email protected]>
---
  drivers/firmware/psci.c   | 31 +++++++++++++++++++++++++++++++
  include/uapi/linux/psci.h |  3 +++
  2 files changed, 34 insertions(+)
drivers/firmware/psci.c +17 -0
index 752ca7c9eb97..f2b2b3e1c6e4 100644
@@ -20,11 +20,13 @@
  #include <linux/printk.h>
  #include <linux/psci.h>
  #include <linux/reboot.h>
+#include <linux/suspend.h>
 
  #include <uapi/linux/psci.h>
 
  #include <asm/system_misc.h>
  #include <asm/smp_plat.h>
+#include <asm/suspend.h>
 
  /*
   * While a 64-bit OS can make calls with SMC32 calling conventions, for some
@@ -222,6 +224,33 @@ static int __init psci_features(u32 psci_func_id)
  			      psci_func_id, 0, 0);
  }
 
+static int psci_system_suspend(unsigned long unused)
+{
+	return invoke_psci_fn(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND),
+			      virt_to_phys(cpu_resume), 0, 0);
+}
+
+static int psci_system_suspend_enter(suspend_state_t state)
+{
+	return cpu_suspend(0, psci_system_suspend);
+}
+
+static const struct platform_suspend_ops psci_suspend_ops = {
+	.valid          = suspend_valid_only_mem,
+	.enter          = psci_system_suspend_enter,
+};

drivers/firmware/psci.c static int __init psci_features(u32 psci_func_id) · line 251

Quoted 10 lines
drivers/firmware/psci.c +10 -0
+
+static void __init psci_init_system_suspend(void)
+{
+	if (!IS_ENABLED(CONFIG_SUSPEND))
+		return;
+
+	if (psci_features(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND)))
+		return;
+
+	suspend_set_ops(&psci_suspend_ops);

For consistency with psci_init_cpu_suspend I rewrote it like this:

static void __init psci_init_system_suspend(void)

{
	int ret;

	if (!IS_ENABLED(CONFIG_SUSPEND))
		return;

	ret = psci_features(PSCI_FN_NATIVE(1_0, SYSTEM_SUSPEND));

	if (ret != PSCI_RET_NOT_SUPPORTED)
		suspend_set_ops(&psci_suspend_ops);
}
Quoted 2 lines

Let me know if it is ok with you. Other than that:

Looks fine to me. Thanks for the fixup.

Quoted 2 lines

Thanks.

Regards,
Sudeep

#25 lore raw

v2 cover PSCI: system suspend support

In reply to Sudeep Holla
Quoted 29 lines · Sudeep Holla

Hi,

These couple of patches add generic support for PSCI system suspend.

The first patch aligns the cpu_{suspend,resume} functions so that they have identical prototype both on ARM32 and ARM64. The second patch adds the system suspend support on top of PSCIv1.0 implementation.

The series is built on top of M.Rutland's branch[1] + Lorenzo's PSCI v1.0 support[2]. The wakeup source is not scope of these patches. It needs to addressed separately.

Platforms with PSCI v1.0 supported in the firmware and SYSTEM_SUSPEND feature advertised as available is required for testing this. This was tested in ARM on Juno platform.

Changes v1[3]->v2:
- Introduces generic macro for PSCI_FN_NATIVE
- Removed redundant code and renamed new PSCI_FN macros with v1.0 tag as suggested by Lorenzo

Regards,
Sudeep

[1] git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git psci/unification
[2] http://www.spinics.net/lists/devicetree/msg82017.html
[3] http://www.spinics.net/lists/arm-kernel/msg425793.html

Quoted 4 lines

Sudeep Holla (3):
arm64: kernel: rename __cpu_suspend to keep it aligned with arm
drivers: firmware: psci: define more generic PSCI_FN_NATIVE macro
drivers: firmware: psci: add system suspend support

For all 3..

Thanks,
Ashwin.