Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver

[Lina Iyer <lina.iyer@linaro.org>]

On Wed, Dec 03 2014 at 07:31 -0700, Lina Iyer wrote:
>On Wed, Dec 03 2014 at 02:11 -0700, Daniel Lezcano wrote:
>>On 12/03/2014 12:05 AM, Lina Iyer wrote:
>>>On Tue, Dec 02 2014 at 10:40 -0700, Lina Iyer wrote:
>>>+
>>>[...]
>>
>>[ ... ]
>>
>>>>+static int __init qcom_spm_init(void)
>>>>+{
>>>>+    int ret;
>>>>+
>>>>+    /*
>>>>+     * cpuidle driver need to registered before the cpuidle device
>>>>+     * for any cpu. Register the device for the the cpuidle driver.
>>>>+     */
>>>>+    ret = platform_device_register(&qcom_cpuidle_drv);
>>>>+    if (ret)
>>>>+        return ret;
>>>Stephen pointed out that we would have the platform device lying around
>>>on a non-QCOM device when using multi_v7_defconfig.
>>
>>Perhaps I am missing the point, but this is not supposed to happen, no ?
>>
>This would happen, since the file would compile on multi_v7 and we would
>initialize and register this device regardless. The cpuidle-qcom.c
>driver probe would bail out looking for a matching compatible property.
>So we would not register a cpuidle driver but the device would lay
>around.
>
Not the best thing to do, but I noticed that ACPI driver does reference
count to register cpuidle driver for the first device and remove them as
the reference count decreases. I am not sure, if we need to reference
count here, since the SPM devices themselves are not removable.

>>>So instead of doing this here, we could do this in the probe..
>>>
>>>if (!cpuidle_get_driver()) {
>>>        int ret = platform_device_register(&qcom_cpuidle_drv);
>>>        if (ret)
>>>                return ret;
>>>}
>>>
>>>Would that be okay?
>>>
>>>The successful probe indicates that we are on a QCOM SoC, and we have not
>>>registered a cpuidle_driver before this.
>>>
>>>Thanks,
>>>Lina
>>>
>>>>+
>>>>+    return platform_driver_register(&spm_driver);
>>>>+}
>>>>+module_init(qcom_spm_init);
>>>>+
>>>>+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 */
>>>>--
>>>>2.1.0
>>>>
>>
>>
>>-- 
>><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
>>