On Sat, Sep 27, 2014 at 01:58:12AM +0100, Lina Iyer wrote: [...] > diff --git a/drivers/soc/qcom/pm.c b/drivers/soc/qcom/pm.c > new file mode 100644 > index 0000000..a2f7d72 > --- /dev/null > +++ b/drivers/soc/qcom/pm.c > @@ -0,0 +1,109 @@ > +/* Copyright (c) 2010-2014, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#include <linux/kernel.h> > +#include <linux/init.h> > +#include <linux/platform_device.h> > + > +#include <asm/proc-fns.h> > +#include <asm/suspend.h> > + > +#include <soc/qcom/pm.h> > +#include <soc/qcom/scm.h> > +#include <soc/qcom/scm-boot.h> > +#include <soc/qcom/spm.h> > + > +#define SCM_CMD_TERMINATE_PC (0x2) > +#define SCM_FLUSH_FLAG_MASK (0x3) > +#define SCM_L2_ON (0x0) > +#define SCM_L2_OFF (0x1) > + > + > +static int set_up_boot_address(void *entry, int cpu) > +{ > + static int flags[NR_CPUS] = { > + SCM_FLAG_WARMBOOT_CPU0, > + SCM_FLAG_WARMBOOT_CPU1, > + SCM_FLAG_WARMBOOT_CPU2, > + SCM_FLAG_WARMBOOT_CPU3, > + }; > + static DEFINE_PER_CPU(void *, last_known_entry); > + > + if (entry == per_cpu(last_known_entry, cpu)) > + return 0; > + > + per_cpu(last_known_entry, cpu) = entry; > + return scm_set_boot_addr(virt_to_phys(entry), flags[cpu]); > +} > + > +static int qcom_pm_collapse(unsigned long int unused) > +{ > + int ret; > + u32 flag; > + > + ret = set_up_boot_address(cpu_resume, raw_smp_processor_id()); > + if (ret) { > + pr_err("Failed to set warm boot address for cpu %d\n", > + raw_smp_processor_id()); > + return ret; > + } > + > + flag = SCM_L2_ON & SCM_FLUSH_FLAG_MASK; > + scm_call_atomic1(SCM_SVC_BOOT, SCM_CMD_TERMINATE_PC, flag); Function call above does not return IIUC (ie it returns through cpu_resume), add a comment to clarify please. > + > + return 0; > +} > + > +/** > + * qcom_cpu_pm_enter_sleep(): Enter a low power mode on current cpu > + * > + * @mode - sleep mode to enter > + * > + * The code should be called with interrupts disabled and on the core on > + * which the low power mode is to be executed. > + * > + */ > +static int qcom_cpu_pm_enter_sleep(enum pm_sleep_mode mode) > +{ > + int ret; > + > + switch (mode) { > + case PM_SLEEP_MODE_SPC: > + qcom_spm_set_low_power_mode(SPM_MODE_POWER_COLLAPSE); > + ret = cpu_suspend(0, qcom_pm_collapse); > + break; > + default: > + case PM_SLEEP_MODE_WFI: > + qcom_spm_set_low_power_mode(SPM_MODE_CLOCK_GATING); > + ret = cpu_do_idle(); Hmmm...are we sure the ret value you get from cpu_do_idle() is correct ? Maybe I am missing something but I do not think that the cpu_do_idle implementation (in assembly files mm/proc-* called through cpu_do_idle() macro) returns a proper value, it seems to me that it "returns" whatever is present in r0 at the time of the call, which might not be what you want. Am I missing something here ? I do not think you should rely on the cpu_do_idle() return value, at least for the moment. > + break; > + } > + > + local_irq_enable(); This looks wrong to me. You end up running the CPU PM notifiers with irqs enabled, among other issues. Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html