Thread (20 messages) 20 messages, 4 authors, 2015-07-28

[PATCH v2 2/4] ARM: mvebu: Add standby support

From: Gregory CLEMENT <hidden>
Date: 2015-07-03 11:39:59
Also in: linux-pm, lkml

Hi Thomas,

On 01/07/2015 17:47, Thomas Petazzoni wrote:
Dear Gregory CLEMENT,

On Tue, 30 Jun 2015 19:18:58 +0200, Gregory CLEMENT wrote:
quoted
Until now only one Armada XP and one Armada 388 based board supported
suspend to ram. However, most of the recent mvebu SoCs can support the
standby mode. Unlike for the suspend to ram, nothing special have to
have -> has
quoted
be done for these SoCs. This patch allows the system to use the
standby mode on Armada 370, 38x, 39x and XP SoCs. There are issues
with the Armada 375, and the support would be added (if possible) in a
would -> might
quoted
future patch.

Signed-off-by: Gregory CLEMENT <redacted>
---
 arch/arm/mach-mvebu/common.h   |  5 ++--
 arch/arm/mach-mvebu/pm-board.c | 17 ++++++++-----
 arch/arm/mach-mvebu/pm.c       | 57 ++++++++++++++++++++++++++++++++++++------
 3 files changed, 64 insertions(+), 15 deletions(-)
On the implementation side, this is much more complicated that it needs
to be I believe. You don't need this mechanism to register the
board-specific hook. Just make pm.c register the suspend_ops in a
late_initcall(), and the pm-board.c register the board specific hook in
a late_initcall(), and have pm.c say that it supports suspend to RAM
only if the board-specific hook has been registered.
Having 2 initcall does not work because, there is a dependency between these
2 calls. And actually the suspend_ops is registered before the board specific
hook. As soon as the suspend_ops is registered, mvebu_pm_valid() is called but
at this point mvebu_board_pm_enter is NULL so PM_SUSPEND_MEM is not available.

All the complexity of the original patch was to allow registering a handler
without needed to get the resource(gpio device) that are not available when using
arch_initcall(). However the device_initcall_sync comes latter enough to
get all the devices registered but it still happens before the late_initcall,
so I will use this one and I will add a comment around it.

Thanks,

Gregory

quoted hunk ↗ jump to hunk
Something like the below (only compile tested, not runtime tested) :

commit 0b74c5b2916cb4be216bd2c607faf5c10a482284
Author: Gregory CLEMENT [off-list ref]
Date:   Tue Jun 30 19:18:58 2015 +0200

    ARM: mvebu: Add standby support
    
    Until now only one Armada XP and one Armada 388 based board supported
    suspend to ram. However, most of the recent mvebu SoCs can support the
    standby mode. Unlike for the suspend to ram, nothing special have to
    be done for these SoCs. This patch allows the system to use the
    standby mode on Armada 370, 38x, 39x and XP SoCs. There are issues
    with the Armada 375, and the support would be added (if possible) in a
    future patch.
    
    Signed-off-by: Gregory CLEMENT [off-list ref]
diff --git a/arch/arm/mach-mvebu/common.h b/arch/arm/mach-mvebu/common.h
index 3e0aca1..6b77549 100644
--- a/arch/arm/mach-mvebu/common.h
+++ b/arch/arm/mach-mvebu/common.h
@@ -25,6 +25,6 @@ int mvebu_system_controller_get_soc_id(u32 *dev, u32 *rev);
 
 void __iomem *mvebu_get_scu_base(void);
 
-int mvebu_pm_init(void (*board_pm_enter)(void __iomem *sdram_reg, u32 srcmd));
-
+int mvebu_pm_suspend_init(void (*board_pm_enter)(void __iomem *sdram_reg,
+							u32 srcmd));
 #endif
diff --git a/arch/arm/mach-mvebu/pm-board.c b/arch/arm/mach-mvebu/pm-board.c
index acc69e3..4dccc64 100644
--- a/arch/arm/mach-mvebu/pm-board.c
+++ b/arch/arm/mach-mvebu/pm-board.c
@@ -135,7 +135,7 @@ static int __init mvebu_armada_pm_init(void)
 	if (!gpio_ctrl)
 		return -ENOMEM;
 
-	mvebu_pm_init(mvebu_armada_pm_enter);
+	mvebu_pm_suspend_init(mvebu_armada_pm_enter);
 
 out:
 	of_node_put(np);
diff --git a/arch/arm/mach-mvebu/pm.c b/arch/arm/mach-mvebu/pm.c
index f249c8e..db31cbb 100644
--- a/arch/arm/mach-mvebu/pm.c
+++ b/arch/arm/mach-mvebu/pm.c
@@ -204,13 +204,10 @@ static int mvebu_pm_store_bootinfo(void)
 	return 0;
 }
 
-static int mvebu_pm_enter(suspend_state_t state)
+static int mvebu_enter_suspend(void)
 {
 	int ret;
 
-	if (state != PM_SUSPEND_MEM)
-		return -EINVAL;
-
 	ret = mvebu_pm_store_bootinfo();
 	if (ret)
 		return ret;
@@ -226,16 +223,57 @@ static int mvebu_pm_enter(suspend_state_t state)
 	set_cpu_coherent();
 
 	cpu_pm_exit();
+	return 0;
+}
+
+static int mvebu_pm_enter(suspend_state_t state)
+{
+	switch (state) {
+	case PM_SUSPEND_STANDBY:
+		cpu_do_idle();
+		break;
+	case PM_SUSPEND_MEM:
+		return mvebu_enter_suspend();
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int mvebu_pm_valid(suspend_state_t state)
+{
+	if (state == PM_SUSPEND_STANDBY)
+		return 1;
+
+	if (state == PM_SUSPEND_MEM && mvebu_board_pm_enter != NULL)
+		return 1;
 
 	return 0;
 }
 
 static const struct platform_suspend_ops mvebu_pm_ops = {
 	.enter = mvebu_pm_enter,
-	.valid = suspend_valid_only_mem,
+	.valid = mvebu_pm_valid,
 };
 
-int __init mvebu_pm_init(void (*board_pm_enter)(void __iomem *sdram_reg, u32 srcmd))
+static int __init mvebu_pm_init(void)
+{
+	if (!of_machine_is_compatible("marvell,armadaxp") &&
+	    !of_machine_is_compatible("marvell,armada370") &&
+	    !of_machine_is_compatible("marvell,armada380") &&
+	    !of_machine_is_compatible("marvell,armada390"))
+		return -ENODEV;
+
+	suspend_set_ops(&mvebu_pm_ops);
+
+	return 0;
+}
+
+
+late_initcall(mvebu_pm_init);
+
+int __init mvebu_pm_suspend_init(void (*board_pm_enter)(void __iomem *sdram_reg,
+							u32 srcmd))
 {
 	struct device_node *np;
 	struct resource res;
@@ -267,7 +305,5 @@ int __init mvebu_pm_init(void (*board_pm_enter)(void __iomem *sdram_reg, u32 src
 
 	mvebu_board_pm_enter = board_pm_enter;
 
-	suspend_set_ops(&mvebu_pm_ops);
-
 	return 0;
 }

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help