* [PATCH v2 0/4] Add standby support for the recent mvebu SoCs
@ 2015-06-30 17:18 Gregory CLEMENT
2015-06-30 17:18 ` [PATCH v2 1/4] ARM: mvebu: Use __init for the PM initialization functions Gregory CLEMENT
` (4 more replies)
0 siblings, 5 replies; 20+ messages in thread
From: Gregory CLEMENT @ 2015-06-30 17:18 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v2 1/4] ARM: mvebu: Use __init for the PM initialization functions 2015-06-30 17:18 [PATCH v2 0/4] Add standby support for the recent mvebu SoCs Gregory CLEMENT @ 2015-06-30 17:18 ` Gregory CLEMENT 2015-07-01 15:18 ` Thomas Petazzoni 2015-06-30 17:18 ` [PATCH v2 2/4] ARM: mvebu: Add standby support Gregory CLEMENT ` (3 subsequent siblings) 4 siblings, 1 reply; 20+ messages in thread From: Gregory CLEMENT @ 2015-06-30 17:18 UTC (permalink / raw) To: linux-arm-kernel 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 <gregory.clement@free-electrons.com> --- 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 1/4] ARM: mvebu: Use __init for the PM initialization functions 2015-06-30 17:18 ` [PATCH v2 1/4] ARM: mvebu: Use __init for the PM initialization functions Gregory CLEMENT @ 2015-07-01 15:18 ` Thomas Petazzoni 0 siblings, 0 replies; 20+ messages in thread From: Thomas Petazzoni @ 2015-07-01 15:18 UTC (permalink / raw) To: linux-arm-kernel 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 <gregory.clement@free-electrons.com> > --- > 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 <thomas.petazzoni@free-electrons.com> Thanks, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/4] ARM: mvebu: Add standby support 2015-06-30 17:18 [PATCH v2 0/4] Add standby support for the recent mvebu SoCs Gregory CLEMENT 2015-06-30 17:18 ` [PATCH v2 1/4] ARM: mvebu: Use __init for the PM initialization functions Gregory CLEMENT @ 2015-06-30 17:18 ` Gregory CLEMENT 2015-07-01 15:47 ` Thomas Petazzoni 2015-06-30 17:18 ` [PATCH v2 3/4] ARM: mvebu: Allow using the GIC for wakeup in standby mode Gregory CLEMENT ` (2 subsequent siblings) 4 siblings, 1 reply; 20+ messages in thread From: Gregory CLEMENT @ 2015-06-30 17:18 UTC (permalink / raw) To: linux-arm-kernel 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 <gregory.clement@free-electrons.com> --- 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/4] ARM: mvebu: Add standby support 2015-06-30 17:18 ` [PATCH v2 2/4] ARM: mvebu: Add standby support Gregory CLEMENT @ 2015-07-01 15:47 ` Thomas Petazzoni 2015-07-03 11:39 ` Gregory CLEMENT 0 siblings, 1 reply; 20+ messages in thread From: Thomas Petazzoni @ 2015-07-01 15:47 UTC (permalink / raw) To: linux-arm-kernel 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 <gregory.clement@free-electrons.com> > --- > 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 <gregory.clement@free-electrons.com> 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 <gregory.clement@free-electrons.com> 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/4] ARM: mvebu: Add standby support 2015-07-01 15:47 ` Thomas Petazzoni @ 2015-07-03 11:39 ` Gregory CLEMENT 2015-07-03 12:17 ` Thomas Petazzoni 0 siblings, 1 reply; 20+ messages in thread From: Gregory CLEMENT @ 2015-07-03 11:39 UTC (permalink / raw) To: linux-arm-kernel 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 <gregory.clement@free-electrons.com> >> --- >> 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 <gregory.clement@free-electrons.com> > 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 <gregory.clement@free-electrons.com> > > 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/4] ARM: mvebu: Add standby support 2015-07-03 11:39 ` Gregory CLEMENT @ 2015-07-03 12:17 ` Thomas Petazzoni 2015-07-03 12:21 ` Gregory CLEMENT 0 siblings, 1 reply; 20+ messages in thread From: Thomas Petazzoni @ 2015-07-03 12:17 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/4] ARM: mvebu: Add standby support 2015-07-03 12:17 ` Thomas Petazzoni @ 2015-07-03 12:21 ` Gregory CLEMENT 2015-07-03 12:33 ` Thomas Petazzoni 0 siblings, 1 reply; 20+ messages in thread From: Gregory CLEMENT @ 2015-07-03 12:21 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/4] ARM: mvebu: Add standby support 2015-07-03 12:21 ` Gregory CLEMENT @ 2015-07-03 12:33 ` Thomas Petazzoni 0 siblings, 0 replies; 20+ messages in thread From: Thomas Petazzoni @ 2015-07-03 12:33 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 3/4] ARM: mvebu: Allow using the GIC for wakeup in standby mode 2015-06-30 17:18 [PATCH v2 0/4] Add standby support for the recent mvebu SoCs Gregory CLEMENT 2015-06-30 17:18 ` [PATCH v2 1/4] ARM: mvebu: Use __init for the PM initialization functions Gregory CLEMENT 2015-06-30 17:18 ` [PATCH v2 2/4] ARM: mvebu: Add standby support Gregory CLEMENT @ 2015-06-30 17:18 ` Gregory CLEMENT 2015-07-01 15:54 ` Thomas Petazzoni 2015-07-01 16:05 ` Sudeep Holla 2015-06-30 17:19 ` [PATCH v2 4/4] ARM: mvebu: Warn about the wake-up sources not taken into account in suspend Gregory CLEMENT 2015-07-25 15:20 ` [PATCH v2 0/4] Add standby support for the recent mvebu SoCs Gregory CLEMENT 4 siblings, 2 replies; 20+ messages in thread From: Gregory CLEMENT @ 2015-06-30 17:18 UTC (permalink / raw) To: linux-arm-kernel 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 <gregory.clement@free-electrons.com> --- 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/4] ARM: mvebu: Allow using the GIC for wakeup in standby mode 2015-06-30 17:18 ` [PATCH v2 3/4] ARM: mvebu: Allow using the GIC for wakeup in standby mode Gregory CLEMENT @ 2015-07-01 15:54 ` Thomas Petazzoni 2015-07-03 7:18 ` Gregory CLEMENT 2015-07-27 11:02 ` Sudeep Holla 2015-07-01 16:05 ` Sudeep Holla 1 sibling, 2 replies; 20+ messages in thread From: Thomas Petazzoni @ 2015-07-01 15:54 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 3/4] ARM: mvebu: Allow using the GIC for wakeup in standby mode 2015-07-01 15:54 ` Thomas Petazzoni @ 2015-07-03 7:18 ` Gregory CLEMENT 2015-07-27 11:02 ` Sudeep Holla 1 sibling, 0 replies; 20+ messages in thread From: Gregory CLEMENT @ 2015-07-03 7:18 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 3/4] ARM: mvebu: Allow using the GIC for wakeup in standby mode 2015-07-01 15:54 ` Thomas Petazzoni 2015-07-03 7:18 ` Gregory CLEMENT @ 2015-07-27 11:02 ` Sudeep Holla 2015-07-28 9:42 ` Gregory CLEMENT 1 sibling, 1 reply; 20+ messages in thread From: Sudeep Holla @ 2015-07-27 11:02 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 3/4] ARM: mvebu: Allow using the GIC for wakeup in standby mode 2015-07-27 11:02 ` Sudeep Holla @ 2015-07-28 9:42 ` Gregory CLEMENT 0 siblings, 0 replies; 20+ messages in thread From: Gregory CLEMENT @ 2015-07-28 9:42 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 3/4] ARM: mvebu: Allow using the GIC for wakeup in standby mode 2015-06-30 17:18 ` [PATCH v2 3/4] ARM: mvebu: Allow using the GIC for wakeup in standby mode Gregory CLEMENT 2015-07-01 15:54 ` Thomas Petazzoni @ 2015-07-01 16:05 ` Sudeep Holla 2015-07-01 16:56 ` Geert Uytterhoeven 1 sibling, 1 reply; 20+ messages in thread From: Sudeep Holla @ 2015-07-01 16:05 UTC (permalink / raw) To: linux-arm-kernel 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 <gregory.clement@free-electrons.com> > --- > 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 3/4] ARM: mvebu: Allow using the GIC for wakeup in standby mode 2015-07-01 16:05 ` Sudeep Holla @ 2015-07-01 16:56 ` Geert Uytterhoeven 0 siblings, 0 replies; 20+ messages in thread From: Geert Uytterhoeven @ 2015-07-01 16:56 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jul 1, 2015 at 6:05 PM, Sudeep Holla <sudeep.holla@arm.com> 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 <gregory.clement@free-electrons.com> >> --- >> 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 -- geert at linux-m68k.org 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 4/4] ARM: mvebu: Warn about the wake-up sources not taken into account in suspend 2015-06-30 17:18 [PATCH v2 0/4] Add standby support for the recent mvebu SoCs Gregory CLEMENT ` (2 preceding siblings ...) 2015-06-30 17:18 ` [PATCH v2 3/4] ARM: mvebu: Allow using the GIC for wakeup in standby mode Gregory CLEMENT @ 2015-06-30 17:19 ` Gregory CLEMENT 2015-07-01 16:04 ` Thomas Petazzoni 2015-07-25 15:20 ` [PATCH v2 0/4] Add standby support for the recent mvebu SoCs Gregory CLEMENT 4 siblings, 1 reply; 20+ messages in thread From: Gregory CLEMENT @ 2015-06-30 17:19 UTC (permalink / raw) To: linux-arm-kernel 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 <gregory.clement@free-electrons.com> --- 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 4/4] ARM: mvebu: Warn about the wake-up sources not taken into account in suspend 2015-06-30 17:19 ` [PATCH v2 4/4] ARM: mvebu: Warn about the wake-up sources not taken into account in suspend Gregory CLEMENT @ 2015-07-01 16:04 ` Thomas Petazzoni 2015-07-03 9:55 ` Gregory CLEMENT 0 siblings, 1 reply; 20+ messages in thread From: Thomas Petazzoni @ 2015-07-01 16:04 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 4/4] ARM: mvebu: Warn about the wake-up sources not taken into account in suspend 2015-07-01 16:04 ` Thomas Petazzoni @ 2015-07-03 9:55 ` Gregory CLEMENT 0 siblings, 0 replies; 20+ messages in thread From: Gregory CLEMENT @ 2015-07-03 9:55 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 0/4] Add standby support for the recent mvebu SoCs 2015-06-30 17:18 [PATCH v2 0/4] Add standby support for the recent mvebu SoCs Gregory CLEMENT ` (3 preceding siblings ...) 2015-06-30 17:19 ` [PATCH v2 4/4] ARM: mvebu: Warn about the wake-up sources not taken into account in suspend Gregory CLEMENT @ 2015-07-25 15:20 ` Gregory CLEMENT 4 siblings, 0 replies; 20+ messages in thread From: Gregory CLEMENT @ 2015-07-25 15:20 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2015-07-28 9:42 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-06-30 17:18 [PATCH v2 0/4] Add standby support for the recent mvebu SoCs Gregory CLEMENT 2015-06-30 17:18 ` [PATCH v2 1/4] ARM: mvebu: Use __init for the PM initialization functions Gregory CLEMENT 2015-07-01 15:18 ` Thomas Petazzoni 2015-06-30 17:18 ` [PATCH v2 2/4] ARM: mvebu: Add standby support Gregory CLEMENT 2015-07-01 15:47 ` Thomas Petazzoni 2015-07-03 11:39 ` Gregory CLEMENT 2015-07-03 12:17 ` Thomas Petazzoni 2015-07-03 12:21 ` Gregory CLEMENT 2015-07-03 12:33 ` Thomas Petazzoni 2015-06-30 17:18 ` [PATCH v2 3/4] ARM: mvebu: Allow using the GIC for wakeup in standby mode Gregory CLEMENT 2015-07-01 15:54 ` Thomas Petazzoni 2015-07-03 7:18 ` Gregory CLEMENT 2015-07-27 11:02 ` Sudeep Holla 2015-07-28 9:42 ` Gregory CLEMENT 2015-07-01 16:05 ` Sudeep Holla 2015-07-01 16:56 ` Geert Uytterhoeven 2015-06-30 17:19 ` [PATCH v2 4/4] ARM: mvebu: Warn about the wake-up sources not taken into account in suspend Gregory CLEMENT 2015-07-01 16:04 ` Thomas Petazzoni 2015-07-03 9:55 ` Gregory CLEMENT 2015-07-25 15:20 ` [PATCH v2 0/4] Add standby support for the recent mvebu SoCs Gregory CLEMENT
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).