Hi,
Until now only few mvebu v7 based board supported suspend to ram. This
suspend to ram mode was unusual because it involved shutting down the
SoC and relied on a PIC to wake up the system.
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. In this mode the SoCs go in idle mode (but they remain
powered up) and the devices enter in suspend mode. The support itself
was added in the patch 2.
In order to wake-up the interrupt controller driver have been
updated. As in standby mode the interrupt controller is not shutdown,
any interrupt can be a wake-up source. So the GIC (patch 3) now used
the flags IRQCHIP_SKIP_SET_WAKE and IRQCHIP_MASK_ON_SUSPEND.
A wake up source is supposed to work in suspend _and_ in standby mode
but for the mvebu SoCs, no interrupt can wake up the system. The last
patch warns the user about it.
The first patch is a clean-up found while working on this series
This series was applied on top of Thomas' series "ARM: mvebu: add
suspend to RAM support for Armada 38x":
http://thread.gmane.org/gmane.linux.ports.arm.kernel/420458
It has been either using rtcwake or by setting the serial line as a
wake-up source through the sysfs interface.
Thanks,
Gregory
Gregory CLEMENT (4):
ARM: mvebu: Use __init for the PM initialization functions
ARM: mvebu: Add standby support
ARM: mvebu: Allow using the GIC for wakeup in standby mode
ARM: mvebu: Warn about the wake-up sources not taken into account in
suspend
arch/arm/mach-mvebu/board-v7.c | 7 +++++
arch/arm/mach-mvebu/common.h | 5 ++--
arch/arm/mach-mvebu/pm-board.c | 19 +++++++++-----
arch/arm/mach-mvebu/pm.c | 58 +++++++++++++++++++++++++++++++++++++-----
4 files changed, 73 insertions(+), 16 deletions(-)
--
2.1.0
mvebu_pm_init and mvebu_armada_pm_init are only called during boot, so
flag them with __init and save some memory.
Signed-off-by: Gregory CLEMENT <[email protected]>
---
arch/arm/mach-mvebu/pm-board.c | 2 +-
arch/arm/mach-mvebu/pm.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-mvebu/pm-board.c b/arch/arm/mach-mvebu/pm-board.c
index 0f471fad214a..acc69e369c8b 100644
--- a/arch/arm/mach-mvebu/pm-board.c
+++ b/arch/arm/mach-mvebu/pm-board.c
@@ -76,7 +76,7 @@ static void mvebu_armada_pm_enter(void __iomem *sdram_reg, u32 srcmd)
[ackcmd] "r" (ackcmd), [gpio_ctrl] "r" (gpio_ctrl) : "r1");
}
-static int mvebu_armada_pm_init(void)
+static int __init mvebu_armada_pm_init(void)
{
struct device_node *np;
struct device_node *gpio_ctrl_np;
diff --git a/arch/arm/mach-mvebu/pm.c b/arch/arm/mach-mvebu/pm.c
index 90d85ef75d54..f249c8e04bd4 100644
--- a/arch/arm/mach-mvebu/pm.c
+++ b/arch/arm/mach-mvebu/pm.c
@@ -235,7 +235,7 @@ static const struct platform_suspend_ops mvebu_pm_ops = {
.valid = suspend_valid_only_mem,
};
-int mvebu_pm_init(void (*board_pm_enter)(void __iomem *sdram_reg, u32 srcmd))
+int __init mvebu_pm_init(void (*board_pm_enter)(void __iomem *sdram_reg, u32 srcmd))
{
struct device_node *np;
struct resource res;
--
2.1.0
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 <[email protected]>
---
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(-)
diff --git a/arch/arm/mach-mvebu/common.h b/arch/arm/mach-mvebu/common.h
index 3e0aca1f288a..341d35d7185a 100644
--- a/arch/arm/mach-mvebu/common.h
+++ b/arch/arm/mach-mvebu/common.h
@@ -25,6 +25,7 @@ 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));
+void mvebu_pm_register_init(int (*board_pm_init)(void));
#endif
diff --git a/arch/arm/mach-mvebu/pm-board.c b/arch/arm/mach-mvebu/pm-board.c
index acc69e369c8b..b471a0421262 100644
--- a/arch/arm/mach-mvebu/pm-board.c
+++ b/arch/arm/mach-mvebu/pm-board.c
@@ -82,10 +82,6 @@ static int __init mvebu_armada_pm_init(void)
struct device_node *gpio_ctrl_np;
int ret = 0, i;
- if (!of_machine_is_compatible("marvell,axp-gp") &&
- !of_machine_is_compatible("marvell,a388-gp"))
- return -ENODEV;
-
np = of_find_node_by_name(NULL, "pm_pic");
if (!np)
return -ENODEV;
@@ -135,11 +131,20 @@ 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);
return ret;
}
-late_initcall(mvebu_armada_pm_init);
+int __init mvebu_armada_pm_register(void)
+{
+ if (of_machine_is_compatible("marvell,axp-gp") ||
+ of_machine_is_compatible("marvell,a388-gp"))
+ mvebu_pm_register_init(mvebu_armada_pm_init);
+
+ return 0;
+}
+
+arch_initcall(mvebu_armada_pm_register);
diff --git a/arch/arm/mach-mvebu/pm.c b/arch/arm/mach-mvebu/pm.c
index f249c8e04bd4..264073a777d8 100644
--- a/arch/arm/mach-mvebu/pm.c
+++ b/arch/arm/mach-mvebu/pm.c
@@ -34,6 +34,8 @@
static void (*mvebu_board_pm_enter)(void __iomem *sdram_reg, u32 srcmd);
static void __iomem *sdram_ctrl;
+static bool is_suspend_mem_available;
+static int (*mvebu_board_pm_init)(void);
static int mvebu_pm_powerdown(unsigned long data)
{
@@ -204,13 +206,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 +225,58 @@ 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)
+{
+ return ((state == PM_SUSPEND_STANDBY) ||
+ (is_suspend_mem_available && (state == PM_SUSPEND_MEM)));
+}
+
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))
+void __init mvebu_pm_register_init(int (*board_pm_init)(void))
+{
+ mvebu_board_pm_init = board_pm_init;
+}
+
+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;
+
+ if (mvebu_board_pm_init)
+ if (mvebu_board_pm_init())
+ pr_warn("Registering suspend init for this board failed\n");
+ suspend_set_ops(&mvebu_pm_ops);
+
+ return 0;
+}
+
+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 +308,9 @@ 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);
+ is_suspend_mem_available = true;
return 0;
}
+
+late_initcall(mvebu_pm_init);
--
2.1.0
On the Armada 375/38x/39x SoCs, in standby mode the SoCs stay powered
and it is possible to wake-up from any interrupt sources. This patch
adds flag to the GIC irqchip driver to let linux know this.
Signed-off-by: Gregory CLEMENT <[email protected]>
---
arch/arm/mach-mvebu/board-v7.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
index e5911defccac..f446230e7416 100644
--- a/arch/arm/mach-mvebu/board-v7.c
+++ b/arch/arm/mach-mvebu/board-v7.c
@@ -18,6 +18,7 @@
#include <linux/of_address.h>
#include <linux/of_fdt.h>
#include <linux/of_platform.h>
+#include <linux/irq.h>
#include <linux/io.h>
#include <linux/clocksource.h>
#include <linux/dma-mapping.h>
@@ -26,6 +27,7 @@
#include <linux/signal.h>
#include <linux/slab.h>
#include <linux/irqchip.h>
+#include <linux/irqchip/arm-gic.h>
#include <asm/hardware/cache-l2x0.h>
#include <asm/mach/arch.h>
#include <asm/mach/map.h>
@@ -129,6 +131,11 @@ static int armada_375_external_abort_wa(unsigned long addr, unsigned int fsr,
static void __init mvebu_init_irq(void)
{
+ if (of_machine_is_compatible("marvell,armada375") ||
+ of_machine_is_compatible("marvell,armada380") ||
+ of_machine_is_compatible("marvell,armada390"))
+ gic_set_irqchip_flags(IRQCHIP_SKIP_SET_WAKE |
+ IRQCHIP_MASK_ON_SUSPEND);
irqchip_init();
mvebu_scu_enable();
coherency_init();
--
2.1.0
On the Armada 370/XP/38x/39x SoCs when the suspend to ram feature is
supported, the SoC is shutdown and will be waken up by an external
micro-controller, so there is no possibility to setup wake-up source
from Linux. However, in standby mode, the SoCs stay powered and it is
possible to wake-up from any interrupt sources. As, when user decide to
setup a wake-up source, there is no way to know if they will be
wake-up source from suspend or from standby, then we chose allowing to
setup all the interrupt as wake-up sources.
The purpose of this patch is to inform the user that in suspend to ram
mode, the wake-up sources won't be taken into consideration.
Signed-off-by: Gregory CLEMENT <[email protected]>
---
arch/arm/mach-mvebu/pm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-mvebu/pm.c b/arch/arm/mach-mvebu/pm.c
index 264073a777d8..4402dcfa7c56 100644
--- a/arch/arm/mach-mvebu/pm.c
+++ b/arch/arm/mach-mvebu/pm.c
@@ -235,6 +235,7 @@ static int mvebu_pm_enter(suspend_state_t state)
cpu_do_idle();
break;
case PM_SUSPEND_MEM:
+ pr_warn("None of the wakeup sources will be taken into account in suspend to ram\n");
return mvebu_enter_suspend();
default:
return -EINVAL;
--
2.1.0
Dear Gregory CLEMENT,
On Tue, 30 Jun 2015 19:18:57 +0200, Gregory CLEMENT wrote:
> mvebu_pm_init and mvebu_armada_pm_init are only called during boot, so
> flag them with __init and save some memory.
>
> Signed-off-by: Gregory CLEMENT <[email protected]>
> ---
> arch/arm/mach-mvebu/pm-board.c | 2 +-
> arch/arm/mach-mvebu/pm.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
Makes sense:
Reviewed-by: Thomas Petazzoni <[email protected]>
Thanks,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
Dear Gregory CLEMENT,
On Tue, 30 Jun 2015 19:18:58 +0200, Gregory CLEMENT wrote:
> 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
> 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
> future patch.
>
> Signed-off-by: Gregory CLEMENT <[email protected]>
> ---
> 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.
Something like the below (only compile tested, not runtime tested) :
commit 0b74c5b2916cb4be216bd2c607faf5c10a482284
Author: Gregory CLEMENT <[email protected]>
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 <[email protected]>
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;
}
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
Dear Gregory CLEMENT,
On Tue, 30 Jun 2015 19:18:59 +0200, Gregory CLEMENT wrote:
> static void __init mvebu_init_irq(void)
> {
> + if (of_machine_is_compatible("marvell,armada375") ||
> + of_machine_is_compatible("marvell,armada380") ||
> + of_machine_is_compatible("marvell,armada390"))
What we really want to know is if we're using the GIC, so what about
instead:
np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic");
if (np)
gic_set_irqchip_flags(IRQCHIP_SKIP_SET_WAKE |
IRQCHIP_MASK_ON_SUSPEND);
of_node_put(np);
(Not even compile-tested).
Thanks,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
Hello,
On Tue, 30 Jun 2015 19:19:00 +0200, Gregory CLEMENT wrote:
> On the Armada 370/XP/38x/39x SoCs when the suspend to ram feature is
> supported, the SoC is shutdown and will be waken up by an external
waken -> woken
> micro-controller, so there is no possibility to setup wake-up source
source -> sources
> from Linux. However, in standby mode, the SoCs stay powered and it is
stay powered -> stays powered on
> possible to wake-up from any interrupt sources. As, when user decide to
> setup a wake-up source, there is no way to know if they will be
> wake-up source from suspend or from standby, then we chose allowing to
> setup all the interrupt as wake-up sources.
Hum. "Since when the users configures the enabled wake-up sources there
is no way to know if the user will be doing suspend to RAM or standby,
we just allow all wake-up sources to be enabled, and only warn when
entering suspend to RAM".
> diff --git a/arch/arm/mach-mvebu/pm.c b/arch/arm/mach-mvebu/pm.c
> index 264073a777d8..4402dcfa7c56 100644
> --- a/arch/arm/mach-mvebu/pm.c
> +++ b/arch/arm/mach-mvebu/pm.c
> @@ -235,6 +235,7 @@ static int mvebu_pm_enter(suspend_state_t state)
> cpu_do_idle();
> break;
> case PM_SUSPEND_MEM:
> + pr_warn("None of the wakeup sources will be taken into account in suspend to ram\n");
We already had some discussion about this, and I believe this continue
to be confusing: there are some wake-up sources taken into account when
in suspend to RAM: the special button used by the micro-controller. It
is indeed not a wake-up source in the Linux sense, but it is still a
wake-up source. With such a message, the user may believe that there is
simply no way of resuming the platform.
Maybe just:
pr_warn("Entering suspend to RAM. Only special wake-up sources will resume the system\n");
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
On 30/06/15 18:18, Gregory CLEMENT wrote:
> On the Armada 375/38x/39x SoCs, in standby mode the SoCs stay powered
> and it is possible to wake-up from any interrupt sources. This patch
> adds flag to the GIC irqchip driver to let linux know this.
>
> Signed-off-by: Gregory CLEMENT <[email protected]>
> ---
> arch/arm/mach-mvebu/board-v7.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
> index e5911defccac..f446230e7416 100644
> --- a/arch/arm/mach-mvebu/board-v7.c
> +++ b/arch/arm/mach-mvebu/board-v7.c
> @@ -18,6 +18,7 @@
> #include <linux/of_address.h>
> #include <linux/of_fdt.h>
> #include <linux/of_platform.h>
> +#include <linux/irq.h>
> #include <linux/io.h>
> #include <linux/clocksource.h>
> #include <linux/dma-mapping.h>
> @@ -26,6 +27,7 @@
> #include <linux/signal.h>
> #include <linux/slab.h>
> #include <linux/irqchip.h>
> +#include <linux/irqchip/arm-gic.h>
> #include <asm/hardware/cache-l2x0.h>
> #include <asm/mach/arch.h>
> #include <asm/mach/map.h>
> @@ -129,6 +131,11 @@ static int armada_375_external_abort_wa(unsigned long addr, unsigned int fsr,
>
> static void __init mvebu_init_irq(void)
> {
> + if (of_machine_is_compatible("marvell,armada375") ||
> + of_machine_is_compatible("marvell,armada380") ||
> + of_machine_is_compatible("marvell,armada390"))
> + gic_set_irqchip_flags(IRQCHIP_SKIP_SET_WAKE |
> + IRQCHIP_MASK_ON_SUSPEND);
I am planning to add these flags in the GIC driver itself and remove
this function. I will post the patch next week once merge window closes
next, so based on how that discussion proceeds you may have drop this
patch, just heads up.
Regards,
Sudeep
On Wed, Jul 1, 2015 at 6:05 PM, Sudeep Holla <[email protected]> wrote:
> On 30/06/15 18:18, Gregory CLEMENT wrote:
>> On the Armada 375/38x/39x SoCs, in standby mode the SoCs stay powered
>> and it is possible to wake-up from any interrupt sources. This patch
>> adds flag to the GIC irqchip driver to let linux know this.
>>
>> Signed-off-by: Gregory CLEMENT <[email protected]>
>> ---
>> arch/arm/mach-mvebu/board-v7.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/mach-mvebu/board-v7.c
>> b/arch/arm/mach-mvebu/board-v7.c
>> index e5911defccac..f446230e7416 100644
>> --- a/arch/arm/mach-mvebu/board-v7.c
>> +++ b/arch/arm/mach-mvebu/board-v7.c
>> @@ -129,6 +131,11 @@ static int armada_375_external_abort_wa(unsigned long
>> addr, unsigned int fsr,
>>
>> static void __init mvebu_init_irq(void)
>> {
>> + if (of_machine_is_compatible("marvell,armada375") ||
>> + of_machine_is_compatible("marvell,armada380") ||
>> + of_machine_is_compatible("marvell,armada390"))
>> + gic_set_irqchip_flags(IRQCHIP_SKIP_SET_WAKE |
>> + IRQCHIP_MASK_ON_SUSPEND);
>
>
> I am planning to add these flags in the GIC driver itself and remove
> this function. I will post the patch next week once merge window closes
> next, so based on how that discussion proceeds you may have drop this
> patch, just heads up.
Tried that before...
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/331813.html
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Thomas,
On 01/07/2015 17:54, Thomas Petazzoni wrote:
> Dear Gregory CLEMENT,
>
> On Tue, 30 Jun 2015 19:18:59 +0200, Gregory CLEMENT wrote:
>
>> static void __init mvebu_init_irq(void)
>> {
>> + if (of_machine_is_compatible("marvell,armada375") ||
>> + of_machine_is_compatible("marvell,armada380") ||
>> + of_machine_is_compatible("marvell,armada390"))
>
> What we really want to know is if we're using the GIC, so what about
> instead:
>
> np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic");
> if (np)
> gic_set_irqchip_flags(IRQCHIP_SKIP_SET_WAKE |
> IRQCHIP_MASK_ON_SUSPEND);
> of_node_put(np);
>
> (Not even compile-tested).
Indeed it is better. I will make the change.
Thanks,
Gregory
>
> Thanks,
>
> Thomas
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
Thanks,
Gregory
On 01/07/2015 18:04, Thomas Petazzoni wrote:
> Hello,
>
> On Tue, 30 Jun 2015 19:19:00 +0200, Gregory CLEMENT wrote:
>> On the Armada 370/XP/38x/39x SoCs when the suspend to ram feature is
>> supported, the SoC is shutdown and will be waken up by an external
>
> waken -> woken
OK
>
>> micro-controller, so there is no possibility to setup wake-up source
>
> source -> sources
OK
>
>> from Linux. However, in standby mode, the SoCs stay powered and it is
>
> stay powered -> stays powered on
SoCs are plural (but maybe there is a special rule for acronym). However
to be more coherent I will also make the previous SoC plural too.
>
>> possible to wake-up from any interrupt sources. As, when user decide to
>> setup a wake-up source, there is no way to know if they will be
>> wake-up source from suspend or from standby, then we chose allowing to
>> setup all the interrupt as wake-up sources.
>
> Hum. "Since when the users configures the enabled wake-up sources there
> is no way to know if the user will be doing suspend to RAM or standby,
> we just allow all wake-up sources to be enabled, and only warn when
> entering suspend to RAM".
OK
>
>> diff --git a/arch/arm/mach-mvebu/pm.c b/arch/arm/mach-mvebu/pm.c
>> index 264073a777d8..4402dcfa7c56 100644
>> --- a/arch/arm/mach-mvebu/pm.c
>> +++ b/arch/arm/mach-mvebu/pm.c
>> @@ -235,6 +235,7 @@ static int mvebu_pm_enter(suspend_state_t state)
>> cpu_do_idle();
>> break;
>> case PM_SUSPEND_MEM:
>> + pr_warn("None of the wakeup sources will be taken into account in suspend to ram\n");
>
> We already had some discussion about this, and I believe this continue
> to be confusing: there are some wake-up sources taken into account when
> in suspend to RAM: the special button used by the micro-controller. It
> is indeed not a wake-up source in the Linux sense, but it is still a
> wake-up source. With such a message, the user may believe that there is
> simply no way of resuming the platform.
Yes you're right, I was speak about the linux wakeup source, but it is
confusing for a "standard" user.
>
> Maybe just:
>
> pr_warn("Entering suspend to RAM. Only special wake-up sources will resume the system\n");
I will take this.
Thanks,
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
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:
>> 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
>
>> 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
>
>> future patch.
>>
>> Signed-off-by: Gregory CLEMENT <[email protected]>
>> ---
>> 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
>
> Something like the below (only compile tested, not runtime tested) :
>
> commit 0b74c5b2916cb4be216bd2c607faf5c10a482284
> Author: Gregory CLEMENT <[email protected]>
> 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 <[email protected]>
>
> 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
Gregory,
On Fri, 03 Jul 2015 13:39:49 +0200, Gregory CLEMENT wrote:
> 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.
And? It will become available soon afterwards.
> 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.
I don't think we care about the order in which the initcalls are called.
If the SoC level init call registering the suspend_ops gets called
first, then at the beginning there is only support for standby. The
support for suspend to RAM will be enabled once the board-level init
call gets called.
If the board level init call is called first, then it will set
mvebu_board_pm_enter. It is not useful at this point. Until the SoC
level init call registers the suspend_ops.
I believe that the ->valid() method of suspend_ops gets called when the
user actually enters the suspend state by writing to /sys/power/state.
And by that time, both init calls will have been called.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
Hi Thomas,
On 03/07/2015 14:17, Thomas Petazzoni wrote:
> Gregory,
>
> On Fri, 03 Jul 2015 13:39:49 +0200, Gregory CLEMENT wrote:
>
>> 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.
>
> And? It will become available soon afterwards.
No it is called during boot and if the method is not there then it is no
more available for the user. I made te test and with a cat on /sys/power/state
I only got "freeze" and "standby" but not "mem".
Thanks,
Gregory
>
>> 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.
>
> I don't think we care about the order in which the initcalls are called.
>
> If the SoC level init call registering the suspend_ops gets called
> first, then at the beginning there is only support for standby. The
> support for suspend to RAM will be enabled once the board-level init
> call gets called.
>
> If the board level init call is called first, then it will set
> mvebu_board_pm_enter. It is not useful at this point. Until the SoC
> level init call registers the suspend_ops.
>
> I believe that the ->valid() method of suspend_ops gets called when the
> user actually enters the suspend state by writing to /sys/power/state.
> And by that time, both init calls will have been called.
>
> Best regards,
>
> Thomas
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
Hello,
On Fri, 03 Jul 2015 14:21:09 +0200, Gregory CLEMENT wrote:
> No it is called during boot and if the method is not there then it is no
> more available for the user. I made te test and with a cat on /sys/power/state
> I only got "freeze" and "standby" but not "mem".
Argh, indeed it's called by suspend_set_ops() itself.
So now, I wonder why the heck ->valid() is a function pointer in this
case, rather than just an argument passed to suspend_set_ops(). Having
a function pointer would precisely be useful to change at runtime which
states are available, and which states are not available. It is being
called again when actually entering the state, but apparently for no
useful reason.
Another option is to have ->valid() always say that suspend to RAM is
available, and just make ->enter() return -EINVAL if we're trying to
enter the PM_SUSPEND_MEM state and the mvebu_pm_board_enter pointer is
NULL. The only drawback is that the kernel will have done all its
suspend procedure (freezing processes, suspend devices) before telling
the user that it is not possible to enter suspend to RAM. But it's
probably not a big deal.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
Hi,
On 30/06/2015 19:18, Gregory CLEMENT wrote:
> Hi,
>
> Until now only few mvebu v7 based board supported suspend to ram. This
> suspend to ram mode was unusual because it involved shutting down the
> SoC and relied on a PIC to wake up the system.
>
> 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. In this mode the SoCs go in idle mode (but they remain
> powered up) and the devices enter in suspend mode. The support itself
> was added in the patch 2.
>
> In order to wake-up the interrupt controller driver have been
> updated. As in standby mode the interrupt controller is not shutdown,
> any interrupt can be a wake-up source. So the GIC (patch 3) now used
> the flags IRQCHIP_SKIP_SET_WAKE and IRQCHIP_MASK_ON_SUSPEND.
>
> A wake up source is supposed to work in suspend _and_ in standby mode
> but for the mvebu SoCs, no interrupt can wake up the system. The last
> patch warns the user about it.
>
> The first patch is a clean-up found while working on this series
>
> This series was applied on top of Thomas' series "ARM: mvebu: add
> suspend to RAM support for Armada 38x":
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/420458
>
> It has been either using rtcwake or by setting the serial line as a
> wake-up source through the sysfs interface.
I applied the series on mvebu/soc
Thanks,
Gregory
>
> Thanks,
>
> Gregory
>
>
> Gregory CLEMENT (4):
> ARM: mvebu: Use __init for the PM initialization functions
> ARM: mvebu: Add standby support
> ARM: mvebu: Allow using the GIC for wakeup in standby mode
> ARM: mvebu: Warn about the wake-up sources not taken into account in
> suspend
>
> arch/arm/mach-mvebu/board-v7.c | 7 +++++
> arch/arm/mach-mvebu/common.h | 5 ++--
> arch/arm/mach-mvebu/pm-board.c | 19 +++++++++-----
> arch/arm/mach-mvebu/pm.c | 58 +++++++++++++++++++++++++++++++++++++-----
> 4 files changed, 73 insertions(+), 16 deletions(-)
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
Hi Thomas/Gregory,
On 01/07/15 16:54, Thomas Petazzoni wrote:
> Dear Gregory CLEMENT,
>
> On Tue, 30 Jun 2015 19:18:59 +0200, Gregory CLEMENT wrote:
>
>> static void __init mvebu_init_irq(void)
>> {
>> + if (of_machine_is_compatible("marvell,armada375") ||
>> + of_machine_is_compatible("marvell,armada380") ||
>> + of_machine_is_compatible("marvell,armada390"))
>
> What we really want to know is if we're using the GIC, so what about
> instead:
>
> np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic");
> if (np)
> gic_set_irqchip_flags(IRQCHIP_SKIP_SET_WAKE |
> IRQCHIP_MASK_ON_SUSPEND);
As I had mentioned before, this function was planned to be removed. tglx
has now pulled in the patch into tip/core[1]. If this patch is already
pulled in, it's better to revert so that it won't break the build in
linux-next. Since the flags are now moved to GIC driver, this patch is
no longer required. Sorry for the trouble.
Regards,
Sudeep
[1]
https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=irq/core&id=0d3f2c92e004c67404fabea19728c1962b777bd6
Hi Sudeep,
On 27/07/2015 13:02, Sudeep Holla wrote:
> Hi Thomas/Gregory,
>
> On 01/07/15 16:54, Thomas Petazzoni wrote:
>> Dear Gregory CLEMENT,
>>
>> On Tue, 30 Jun 2015 19:18:59 +0200, Gregory CLEMENT wrote:
>>
>>> static void __init mvebu_init_irq(void)
>>> {
>>> + if (of_machine_is_compatible("marvell,armada375") ||
>>> + of_machine_is_compatible("marvell,armada380") ||
>>> + of_machine_is_compatible("marvell,armada390"))
>>
>> What we really want to know is if we're using the GIC, so what about
>> instead:
>>
>> np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic");
>> if (np)
>> gic_set_irqchip_flags(IRQCHIP_SKIP_SET_WAKE |
>> IRQCHIP_MASK_ON_SUSPEND);
>
> As I had mentioned before, this function was planned to be removed. tglx
> has now pulled in the patch into tip/core[1]. If this patch is already
> pulled in, it's better to revert so that it won't break the build in
> linux-next. Since the flags are now moved to GIC driver, this patch is
> no longer required. Sorry for the trouble.
Your patch has been merged in th irq branch just a few day after I merged this one.
But fortunately I can remove the commit from my branch without any trouble.
It is done now.
Thanks,
Gregory
>
> Regards,
> Sudeep
>
> [1]
> https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/commit/?h=irq/core&id=0d3f2c92e004c67404fabea19728c1962b777bd6
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com