On Thu, Aug 21, 2014 at 03:24:44AM +0200, Daniel Lezcano wrote:
On 08/20/2014 12:15 AM, Lina Iyer wrote:
Add cpuidle driver interface to allow cpus to go into C-States.
Use the cpuidle DT interfacecommon across ARM architectures to provide
^^^^^^^^
the C-State information to the cpuidle framework.
Signed-off-by: Lina Iyer <lina.iyer@xxxxxxxxxx>
---
drivers/cpuidle/Kconfig.arm | 7 +++
drivers/cpuidle/Makefile | 1 +
drivers/cpuidle/cpuidle-qcom.c | 119 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 127 insertions(+)
create mode 100644 drivers/cpuidle/cpuidle-qcom.c
diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index e339c7f..26e31bd 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -63,3 +63,10 @@ config ARM_MVEBU_V7_CPUIDLE
depends on ARCH_MVEBU
help
Select this to enable cpuidle on Armada 370, 38x and XP processors.
+
+config ARM_QCOM_CPUIDLE
+ bool "CPU Idle drivers for Qualcomm processors"
+ depends on QCOM_PM
+ select DT_IDLE_STATES
+ help
+ Select this to enable cpuidle for QCOM processors
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index 4d177b9..6c222d5 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_ARM_ZYNQ_CPUIDLE) += cpuidle-zynq.o
obj-$(CONFIG_ARM_U8500_CPUIDLE) += cpuidle-ux500.o
obj-$(CONFIG_ARM_AT91_CPUIDLE) += cpuidle-at91.o
obj-$(CONFIG_ARM_EXYNOS_CPUIDLE) += cpuidle-exynos.o
+obj-$(CONFIG_ARM_QCOM_CPUIDLE) += cpuidle-qcom.o
###############################################################################
# MIPS drivers
diff --git a/drivers/cpuidle/cpuidle-qcom.c b/drivers/cpuidle/cpuidle-qcom.c
new file mode 100644
index 0000000..4aae672
--- /dev/null
+++ b/drivers/cpuidle/cpuidle-qcom.c
@@ -0,0 +1,119 @@
+/* Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2014, Linaro Limited.
+ *
+ * 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/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/cpuidle.h>
+#include <linux/cpumask.h>
+#include <linux/slab.h>
See below, cpumask is not needed, you can remove slab.h and cpumask.h.
+#include <linux/of_device.h>
+
+#include <soc/qcom/pm.h>
+#include "dt_idle_states.h"
+
+static enum msm_pm_sleep_mode modes[CPUIDLE_STATE_MAX];
+
+static int qcom_lpm_enter(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int index)
+{
+ return msm_cpu_pm_enter_sleep(modes[index], true);
You should return the index here.
+}
+
+static struct cpuidle_driver qcom_cpuidle_driver = {
+ .name = "qcom_cpuidle",
+ .owner = THIS_MODULE,
+};
+
+static void parse_state_translations(struct cpuidle_driver *drv)
+{
+ struct device_node *state_node, *cpu_node;
+ const char *mode_name;
+ int i, j;
+
+ struct name_map {
+ enum msm_pm_sleep_mode mode;
+ char *name;
+ };
+ static struct name_map c_states[] = {
+ { MSM_PM_SLEEP_MODE_POWER_COLLAPSE_STANDALONE,
+ "standalone-pc" },
What is standalone-pc and collapse ? Core power down ?
+ { MSM_PM_SLEEP_MODE_WAIT_FOR_INTERRUPT, "wfi" },
+ };
+
+ cpu_node = of_cpu_device_node_get(cpumask_first(drv->cpumask));
+ if (!cpu_node)
+ return;
+
+ /**
+ * Get the state description from idle-state node entry-method
+ * First state is always WFI, per spec.
+ */
+ modes[0] = MSM_PM_SLEEP_MODE_WAIT_FOR_INTERRUPT;
+ for (i = 1; i < drv->state_count; i++) {
+ mode_name = NULL;
+ state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
+ of_property_read_string(state_node, "entry-method",
+ &mode_name);
+ for (j = 0; mode_name && (j < ARRAY_SIZE(c_states)); j++) {
+ if (!strcmp(mode_name, c_states[j].name)) {
+ modes[i] = c_states[j].mode;
+ break;
+ }
+ }
+ }
+}
For the function above, I believe we can do better with the
idle_dt_init function to prevent to have to reparse the infos.
I had the opportunity to discuss with Lorenzo privately and we found a
solution he will submit to solve that.
+static int qcom_cpuidle_init(void)
+{
+ struct cpuidle_driver *drv = &qcom_cpuidle_driver;
+ int ret;
+ int i;
+
+ drv->cpumask = kzalloc(cpumask_size(), GFP_KERNEL);
+ if (!drv->cpumask)
+ return -ENOMEM;
+ cpumask_copy(drv->cpumask, cpu_possible_mask);
You can get rid of this because the driver does not rely on the
multiple driver support. If the drv->cpumask is NULL it will be
automatically set to cpu_possible_mask by the cpuidle framework.
Lorenzo: Seems like the dt_init_idle_driver expects to see this mask
set.
May be you can check if its NULL and revert to cpu_possible_mask in that
case. That would avoid drivers creating memory and copying
cpu_possible_mask in their init functions.
+
+ /**
+ * Default to standard for WFI (index = 0)
+ * Probe only for other states
+ */
+ ret = dt_init_idle_driver(drv, 1);
So IIUC, if you specify the index 1, that means the state[0] will be
the default WFI. But you override the callback below in the loop.
I recommend you use the default arm wfi callback but you implement the
cpu_do_idle for your platform.
+ if (ret < 0) {
+ pr_err("%s: failed to initialize idle states\n", __func__);
+ goto failed;
+ }
+
+ /* Parse the idle states for C-States on this cpu */
+ parse_state_translations(drv);
+
+ /* Register entry point for all low states */
+ for (i = 0; i < drv->state_count; i++)
+ drv->states[i].enter = qcom_lpm_enter;
+ drv->safe_state_index = 0;
The safe_state index is only used for the couple c-states and as the
driver is statically defined, it is pointless to initialize this
variable.
+
+ ret = cpuidle_register(drv, NULL);
+ if (ret) {
+ pr_err("%s: failed to register cpuidle driver\n", __func__);
+ goto failed;
+ }
+
+ return 0;
+
+failed:
+ kfree(drv->cpumask);
+ return ret;
+}
+module_init(qcom_cpuidle_init);
Can you stick to the platform_driver paradigm like the other drivers ?
Thanks
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
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
