| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <55967485.2010607@free-electrons.com>
Date: Fri, 03 Jul 2015 13:39:49 +0200
From: Gregory CLEMENT <gregory.clement@...e-electrons.com>
To: Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
CC: Jason Cooper <jason@...edaemon.net>, Andrew Lunn <andrew@...n.ch>,
Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>,
linux-arm-kernel@...ts.infradead.org,
Maxime Ripard <maxime.ripard@...e-electrons.com>,
Boris BREZILLON <boris.brezillon@...e-electrons.com>,
Lior Amsalem <alior@...vell.com>,
Tawfik Bayouk <tawfik@...vell.com>,
Nadav Haklai <nadavh@...vell.com>, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/4] ARM: mvebu: Add standby support
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@...e-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@...e-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@...e-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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists