On 12/01/2014 07:50 PM, Lina Iyer wrote:
On Thu, Nov 27 2014 at 01:52 -0700, Daniel Lezcano wrote:
On 11/27/2014 06:24 AM, Lina Iyer wrote:
+ static bool cpuidle_drv_init;
^^^^^^^^^
As already said in a previous comment, please find a way to remove that.
I will look into it. Stephen and I wanted the cpuidle driver to be
probed only after any of the SPMs are ready. And possibly, only for that
cpu, for which the SPM has been probed.
To achieve the SPM -> CPUIDLE Device relation, I havent found a good way
to do that. Without using CPUIDLE_MULTIPLE_DRIVERS, initializing each
cpuidle device, separate from the cpuidle driver, requires that I update
the cpuidle_driver->cpumask after probing each SPM device, to allow for
only one driver and cpuidle devices only for the probed cpus. Using the
cpuidle_register_driver(), resets the drv->refcnt in
__cpuidle_driver_init.
I may need something like this in the else clause of
CPUIDLE_MULTIPLE_DRIVERS -
static inline struct cpuidle_driver *__cpuidle_get_cpu_driver(int cpu)
{
struct cpuidle_driver *drv = cpuidle_curr_driver;
if (drv && !cpumask_test_cpu(cpu, drv->cpumask))
drv = NULL;
return drv;
}
You don't have to deal with the drv->cpumask and
CPUIDLE_MULTIPLE_DRIVERS. This field gives the list of the cpus the
driver will have to potentially handle.
Just register the driver at the first place with the platform device by
using cpuidle_register_driver(drv), I suggest somewhere else than the
spm code.
The underlying code will do:
if (!drv->cpumask)
drv->cpumask = (struct cpumask *)cpu_possible_mask;
Until the cpuidle device is not initialized for a specific cpu, the
cpuidle code will have no effect for this cpu. If you register the
driver but without registering the cpuidle devices that will result in
nothing more than a structure initialized but inoperative.
For each SPM being probed:
struct cpuidle_device *dev = &per_cpu(cpuidle_dev, cpu);
dev->cpu = cpu;
cpuidle_register_device(dev);
Note cpuidle_dev is exported via cpuidle.h, so you don't have to
redeclare a per cpu cpuidle device by your own.
So rephrasing all that:
(1) in cpuidle-qcom register the driver
(2) in the spm code register the device for each spm probe
(1) being called before (2).
If after that in the code you still have a "static bool
cpuidle_drv_init", then I guess we have a problem in the init sequence
somewhere else.
void cpuidle_update_cpumask(struct cpumask *mask) {
struct cpuidle_driver *drv;
spin_lock(&cpuidle_driver_lock);
drv = cpuidle_get_driver();
if (drv)
drv->cpumask = mask ?: cpu_possible_mask;
spin_unlock(&cpuidle_driver_lock);
}
With that, I could register cpuidle driver the first time when the mask
changes
from empty and consequent updates would just update the cpumask. (I am not
sure, if I missed anything in this change). It just seemed far too
invasive at
this time, in lieu of the static bool.
+ const struct platform_device_info qcom_cpuidle_info = {
+ .name = "qcom_cpuidle",
+ .id = -1,
+ .data = &lpm_ops,
+ .size_data = sizeof(lpm_ops),
+ };
+
+ drv = spm_get_drv(pdev, &cpu);
+ if (!drv || cpu < 0)
+ return -EINVAL;
As already said in a previous comment, it is not possible to have "cpu
< 0" with "drv != NULL", so except I am missing something the test
should be:
if (!drv)
return -EINVAL;
Sorry, done.
There is something wrong with the init sequence. Don't you find weird
you have to backward search for the cpu belonging to the pdev each
time the probe function is called ?
Well, it was that or the SPM device node pointing to the CPU that it
references. It seems more common to have an iterator than the doubly
linked device nodes. I dont have a strong preference either way, just
chose the way that made device nodes easier.
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ drv->reg_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(drv->reg_base))
+ return PTR_ERR(drv->reg_base);
+
+ match_id = of_match_node(spm_match_table, pdev->dev.of_node);
+ if (!match_id)
+ return -ENODEV;
+
+ drv->reg_data = match_id->data;
+
+ /* Write the SPM sequences first.. */
+ addr = drv->reg_base +
drv->reg_data->reg_offset[SPM_REG_SEQ_ENTRY];
+ __iowrite32_copy(addr, drv->reg_data->seq,
+ ARRAY_SIZE(drv->reg_data->seq) / 4);
+
+ /*
+ * ..and then the control registers.
+ * On some SoC's if the control registers are written first and
if the
+ * CPU was held in reset, the reset signal could trigger the SPM
state
+ * machine, before the sequences are completely written.
+ */
+ spm_register_write(drv, SPM_REG_CFG, drv->reg_data->spm_cfg);
+ spm_register_write(drv, SPM_REG_DLY, drv->reg_data->spm_dly);
+ spm_register_write(drv, SPM_REG_PMIC_DLY, drv->reg_data->pmic_dly);
+
+ spm_register_write(drv, SPM_REG_PMIC_DATA_0,
+ drv->reg_data->pmic_data[0]);
+ spm_register_write(drv, SPM_REG_PMIC_DATA_1,
+ drv->reg_data->pmic_data[1]);
+
+ /*
+ * Ensure all observers see the above register writes before the
+ * cpuidle driver is allowed to use the SPM.
+ */
+ wmb();
+ per_cpu(cpu_spm_drv, cpu) = drv;
+
+ if (!cpuidle_drv_init) {
+ platform_device_register_full(&qcom_cpuidle_info);
+ cpuidle_drv_init = true;
+ }
+
+ return 0;
+}
+
+static struct platform_driver spm_driver = {
+ .probe = spm_dev_probe,
+ .driver = {
+ .name = "saw",
+ .of_match_table = spm_match_table,
+ },
+};
+
+module_platform_driver(spm_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("SAW power controller driver");
+MODULE_ALIAS("platform:saw");
diff --git a/include/soc/qcom/pm.h b/include/soc/qcom/pm.h
new file mode 100644
index 0000000..d9a56d7
--- /dev/null
+++ b/include/soc/qcom/pm.h
@@ -0,0 +1,31 @@
+/*
+ * 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_STBY,
+ PM_SLEEP_MODE_RET,
+ PM_SLEEP_MODE_SPC,
+ PM_SLEEP_MODE_PC,
+ PM_SLEEP_MODE_NR,
+};
+
+struct qcom_cpu_pm_ops {
+ int (*standby)(void *data);
+ int (*spc)(void *data);
+};
+
+#endif /* __QCOM_PM_H */
--
<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
--
<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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
