On Mon, Sep 29 2014 at 17:37 -0600, Stephen Boyd wrote:
On 09/26/14 17:58, Lina Iyer wrote:
Based on work by many authors, available at codeaurora.org
Signed-off-by: Lina Iyer <lina.iyer@xxxxxxxxxx>
[lina: simplify the driver for an initial submission, add commit text
description of idle states]
Maintainer tags don't really make sense unless there is another author.
Hmm.. Since this patch is a derivative work, I wanted to clarify, what
changed seems important. The work was done by many authors. Adding
signed-off from everybody who could have contributed to the patch
downstream is confusing.
I would be okay removing it.
+
[...]
+static int qcom_pm_collapse(unsigned long int unused)
+{
+ int ret;
+ u32 flag;
+
+ ret = set_up_boot_address(cpu_resume, raw_smp_processor_id());
Preemption better be off here, so why are we using raw_smp_processor_id()?
True, so raw_ returns without premeption disable, no?
+ if (ret) {
+ pr_err("Failed to set warm boot address for cpu %d\n",
+ raw_smp_processor_id());
Stuff cpu into a local variable?
Yeah
+ return ret;
+ }
+
+ flag = SCM_L2_ON & SCM_FLUSH_FLAG_MASK;
+ scm_call_atomic1(SCM_SVC_BOOT, SCM_CMD_TERMINATE_PC, flag);
+
+ 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);
Isn't it a one-to-one between PM_SLEEP_MODE_SPC and
SPM_MODE_POWER_COLLAPSE? The enum to enum map seems overly complicated.
Not really. SPM modes, differ when the idle state has to notify RPM. It
does not have 2 enums for those modes, but uses an overloaded enum with
an additional flag.
+ 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();
+ break;
+ }
+
+ local_irq_enable();
+
+ return ret;
+}
+
+static struct platform_device qcom_cpuidle_device = {
+ .name = "qcom_cpuidle",
+ .id = -1,
+ .dev.platform_data = qcom_cpu_pm_enter_sleep,
+};
This doesn't need to be static. Use platform_device_register_full() instead.
Okay
+
+static int __init qcom_pm_device_init(void)
+{
+ platform_device_register(&qcom_cpuidle_device);
+
+ return 0;
+}
+device_initcall(qcom_pm_device_init);
modules?
Why? An earlier initialization helps with power saving
diff --git a/include/soc/qcom/pm.h b/include/soc/qcom/pm.h
new file mode 100644
index 0000000..563b9c3
--- /dev/null
+++ b/include/soc/qcom/pm.h
@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2009-2014, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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.
+ *
+ */
+
+#ifndef __QCOM_PM_H
+#define __QCOM_PM_H
+
+enum pm_sleep_mode {
+ PM_SLEEP_MODE_WFI,
+ PM_SLEEP_MODE_RET,
+ PM_SLEEP_MODE_SPC,
+ PM_SLEEP_MODE_PC,
+ PM_SLEEP_MODE_NR,
+};
+
+#endif /* __QCOM_PM_H */
Hopefully this header is not necessary.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
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
