| Message ID | 1403875377-940-9-git-send-email-gregory.clement@free-electrons.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Dear Gregory CLEMENT, On Fri, 27 Jun 2014 15:22:49 +0200, Gregory CLEMENT wrote: > +static void *mvebu_cpu_resume; > + > static struct platform_device mvebu_v7_cpuidle_device = { > .name = "cpuidle-armada-370-xp", > }; > @@ -281,7 +283,7 @@ static int mvebu_v7_cpu_pm_notify(struct notifier_block *self, > { > if (action == CPU_PM_ENTER) { > unsigned int hw_cpu = cpu_logical_map(smp_processor_id()); > - mvebu_pmsu_set_cpu_boot_addr(hw_cpu, armada_370_xp_cpu_resume); > + mvebu_pmsu_set_cpu_boot_addr(hw_cpu, mvebu_cpu_resume); Instead of doing this in the CPU_PM_ENTER notifier, do we have a reason to not do it in the do_armada_370_xp_cpu_suspend() function, and then do_armada_38x_suspend() function respectively? Those functions are already SoC-specific, so surely, they already know whether the resume path should go through armada_370_xp_cpu_resume() or armada_38x_cpu_resume(), no? Thanks, Thomas
Hi Thomas, On 30/06/2014 17:09, Thomas Petazzoni wrote: > Dear Gregory CLEMENT, > > On Fri, 27 Jun 2014 15:22:49 +0200, Gregory CLEMENT wrote: > >> +static void *mvebu_cpu_resume; >> + >> static struct platform_device mvebu_v7_cpuidle_device = { >> .name = "cpuidle-armada-370-xp", >> }; >> @@ -281,7 +283,7 @@ static int mvebu_v7_cpu_pm_notify(struct notifier_block *self, >> { >> if (action == CPU_PM_ENTER) { >> unsigned int hw_cpu = cpu_logical_map(smp_processor_id()); >> - mvebu_pmsu_set_cpu_boot_addr(hw_cpu, armada_370_xp_cpu_resume); >> + mvebu_pmsu_set_cpu_boot_addr(hw_cpu, mvebu_cpu_resume); > > Instead of doing this in the CPU_PM_ENTER notifier, do we have a reason > to not do it in the do_armada_370_xp_cpu_suspend() function, and then > do_armada_38x_suspend() function respectively? Those functions are > already SoC-specific, so surely, they already know whether the resume > path should go through armada_370_xp_cpu_resume() or > armada_38x_cpu_resume(), no? Now that all this code is also part of pmsu and no more in the cpu dile driver. It should be possible. Let me see what it implies about the path code. Thanks, Gregory
diff --git a/arch/arm/mach-mvebu/pmsu.c b/arch/arm/mach-mvebu/pmsu.c index 454f0f9ede6b..8bf737fb3aac 100644 --- a/arch/arm/mach-mvebu/pmsu.c +++ b/arch/arm/mach-mvebu/pmsu.c @@ -79,6 +79,8 @@ extern void armada_370_xp_cpu_resume(void); static unsigned long pmsu_mp_phys_base; static void __iomem *pmsu_mp_base; +static void *mvebu_cpu_resume; + static struct platform_device mvebu_v7_cpuidle_device = { .name = "cpuidle-armada-370-xp", }; @@ -281,7 +283,7 @@ static int mvebu_v7_cpu_pm_notify(struct notifier_block *self, { if (action == CPU_PM_ENTER) { unsigned int hw_cpu = cpu_logical_map(smp_processor_id()); - mvebu_pmsu_set_cpu_boot_addr(hw_cpu, armada_370_xp_cpu_resume); + mvebu_pmsu_set_cpu_boot_addr(hw_cpu, mvebu_cpu_resume); } else if (action == CPU_PM_EXIT) { mvebu_v7_pmsu_idle_restore(); } @@ -304,6 +306,7 @@ static __init bool armada_xp_cpuidle_init(void) return false; of_node_put(np); + mvebu_cpu_resume = armada_370_xp_cpu_resume; mvebu_v7_cpuidle_device.dev.platform_data = armada_xp_370_cpu_suspend; return true; }
The resume address used by the cpu idle code will not always be the same. Using a local variable to store the resume address allows to keep the same function for the pm notifier but with a different address. This address will be set during the initialization. Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> --- arch/arm/mach-mvebu/pmsu.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)