cpuidle driver for QCOM SoCs: 8064, 8074, 8084

Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt +3 -0

+                       "qcom,apq8064-saw2-v1.1-cpu"
+                       "qcom,msm8974-saw2-v2.1-cpu"
+                       "qcom,apq8084-saw2-v2.1-cpu"
 

-       regulator@2099000 {

+       power-controller@2099000 {
                compatible = "qcom,saw2";
                reg = <0x02099000 0x1000>, <0x02009000 0x1000>;

+ regulator;

Hi,

Quoted 1 line · Lina Iyer

Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt PROPERTIES · line 25

Quoted 5 lines

- compatible:

Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt +0 -0

@@ -14,10 +23,13 @@ PROPERTIES
        Value type: <string>
        Definition: shall contain "qcom,saw2". A more specific value should be
                                                one of:

Which driver is supposed to handle this property?

Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt PROPERTIES · line 29

Quoted 8 lines

Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt +4 -4

-                               "qcom,saw2-v1"
-                               "qcom,saw2-v1.1"
-                               "qcom,saw2-v2"
-                               "qcom,saw2-v2.1"
+                       "qcom,saw2-v1"
+                       "qcom,saw2-v1.1"
+                       "qcom,saw2-v2"
+                       "qcom,saw2-v2.1"

Doesn't seems to be used or supported?

Quoted 4 lines

Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt +3 -0

+                       "qcom,apq8064-saw2-v1.1-cpu"
+                       "qcom,msm8974-saw2-v2.1-cpu"
+                       "qcom,apq8084-saw2-v2.1-cpu"
 

<snip>

Quoted 6 lines

-???????regulator at 2099000 {

+???????power-controller at 2099000 {
                compatible = "qcom,saw2";
                reg = <0x02099000 0x1000>, <0x02009000 0x1000>;

+ regulator;

<snip>

drivers/soc/qcom/spm.c line 244

Quoted 10 lines

drivers/soc/qcom/spm.c new file +10 -0

+static const struct of_device_id spm_match_table[] = {
+       { .compatible = "qcom,msm8974-saw2-v2.1-cpu",
+                       .data = &spm_reg_8974_8084_cpu },
+       { .compatible = "qcom,apq8084-saw2-v2.1-cpu",
+                       .data = &spm_reg_8974_8084_cpu },
+       { .compatible = "qcom,apq8064-saw2-v1.1-cpu",
+                       .data = &spm_reg_8064_cpu },
+       { },
+};
+

Regards,
Ivan

SPM is a hardware block that controls the peripheral logic surrounding the application cores (cpu/l$). When the core executes WFI instruction, the SPM takes over the putting the core in low power state as configured. The wake up for the SPM is an interrupt at the GIC, which then completes the rest of low power mode sequence and brings the core out of low power mode.

The SPM has a set of control registers that configure the SPMs individually based on the type of the core and the runtime conditions. SPM is a finite state machine block to which a sequence is provided and it interprets the bytes and executes them in sequence. Each low power mode that the core can enter into is provided to the SPM as a sequence.

Configure the SPM to set the core (cpu or L2) into its low power mode, the index of the first command in the sequence is set in the SPM_CTL register. When the core executes ARM wfi instruction, it triggers the SPM state machine to start executing from that index. The SPM state machine waits until the interrupt occurs and starts executing the rest of the sequence until it hits the end of the sequence. The end of the sequence jumps the core out of its low power mode.

Add support for an idle driver to set up the SPM to place the core in Standby or Standalone power collapse mode when the core is idle.

Based on work by: Mahesh Sivasubramanian <[email protected]>, Ai Li <[email protected]>, Praveen Chidambaram <[email protected]> Original tree available at -
git://codeaurora.org/quic/la/kernel/msm-3.10.git

Signed-off-by Lina Iyer <[email protected]>

Reviewed-by Kevin Hilman <[email protected]>

drivers/soc/qcom/spm.c new file +45 -0

+static struct spm_driver_data *spm_get_drv(struct platform_device *pdev,
+		int *spm_cpu)
+{
+	struct spm_driver_data *drv = NULL;
+	struct device_node *cpu_node, *saw_node;
+	int cpu;
+	bool found = false;
+
+	for_each_possible_cpu(cpu) {
+		cpu_node = of_cpu_device_node_get(cpu);
+		if (!cpu_node)
+			continue;
+		saw_node = of_parse_phandle(cpu_node, "qcom,saw", 0);
+		if (saw_node) {
+			if (saw_node == pdev->dev.of_node)
+				found = true;
+			of_node_put(saw_node);
+		}
+		of_node_put(cpu_node);
+		if (found)
+			break;
+	}
+
+	if (found) {
+		drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
+		if (drv)
+			*spm_cpu = cpu;
+	}
+
+	return drv;
+}
+
+static const struct of_device_id spm_match_table[] = {
+	{ .compatible = "qcom,msm8974-saw2-v2.1-cpu",
+	  .data = &spm_reg_8974_8084_cpu },
+	{ .compatible = "qcom,apq8084-saw2-v2.1-cpu",
+	  .data = &spm_reg_8974_8084_cpu },
+	{ .compatible = "qcom,apq8064-saw2-v1.1-cpu",
+	  .data = &spm_reg_8064_cpu },
+	{ },
+};
+
+static const struct qcom_cpu_pm_ops lpm_ops = {
+	.standby = qcom_cpu_standby,
+	.spc = qcom_cpu_spc,

drivers/soc/qcom/spm.c new file +60 -0

+
+	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");

include/soc/qcom/pm.h new file +31 -0

new file mode 100644
index 0000000..d9a56d7
@@ -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 */

On 11/27/2014 06:24 AM, Lina Iyer wrote:

(quoted diff) +1 -0

+	static bool cpuidle_drv_init;
 
       ^^^^^^^^^

As already said in a previous comment, please find a way to remove that.

drivers/soc/qcom/spm.c line 267

Quoted 10 lines

drivers/soc/qcom/spm.c new file +10 -0

+	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;

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 ?

Quoted 98 lines

drivers/soc/qcom/spm.c new file +60 -0

+
+	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");

include/soc/qcom/pm.h new file +31 -0

new file mode 100644
index 0000000..d9a56d7
@@ -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 */

Signature · 5 lines

<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

Quoted 8 lines · Daniel Lezcano

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;
}

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.

Quoted 11 lines

drivers/soc/qcom/spm.c line 267

Quoted 10 lines

drivers/soc/qcom/spm.c new file +10 -0

+    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;

Quoted 7 lines

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.

Quoted 5 lines

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.

Quoted 110 lines

Quoted 91 lines

drivers/soc/qcom/spm.c new file +14 -0

+
+    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];

drivers/soc/qcom/spm.c new file +6 -0

+    __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

drivers/soc/qcom/spm.c new file +39 -0

+     * 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");

include/soc/qcom/pm.h new file +21 -0

new file mode 100644
index 0000000..d9a56d7
@@ -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,

include/soc/qcom/pm.h line 31

Quoted 10 lines

include/soc/qcom/pm.h new file +10 -0

+    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

Quoted 25 lines · Lina Iyer

Quoted 8 lines · Daniel Lezcano

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)

Quoted 8 lines

{
       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.

Quoted 43 lines

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.

Quoted 11 lines

drivers/soc/qcom/spm.c line 267

Quoted 10 lines

drivers/soc/qcom/spm.c new file +10 -0

+    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;

Quoted 7 lines

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.

Quoted 5 lines

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 ?

Quoted 5 lines

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.

Quoted 65 lines · Daniel Lezcano

Quoted 26 lines · Lina Iyer

Quoted 9 lines · Daniel Lezcano

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)

Quoted 8 lines

{
       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:

Quoted 9 lines

(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.

Okay, Thanks. Last night I was able to get a patch working with one cpuidle_register_driver and multiple cpuidle_register_device. The issue using an module_init() was -

1. Register a device to probe the cpuidle driver.
2. Register cpuidle devices for every SPM probed.

What used to happen is that the cpuidle driver probe always happens after the cpuidle devices were created.

I have a solution where I register for the cpuidle device to probe a driver, at postcore_initcall() at which point, the cpuidle framework would also be available.

I am not sure how elegant it looks, but thats the latest initcall I could get a consistent driver initialization before the SPM probe (therefore the device registration).

I will send the patch this morning.

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

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 ?

Quoted 5 lines

Quoted 5 lines

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.

This one, I havent addressed. How strongly do you feel about this? I still have to iterate through all the cpus to find the cpu node that matches the handle specified in the SAW node.

drivers/soc/qcom/spm.c new file +10 -0

+static int spm_set_low_power_mode(enum pm_sleep_mode mode)
+{
+       struct spm_driver_data *drv = per_cpu(cpu_spm_drv,
+                                               smp_processor_id());
+       u32 start_index;
+       u32 ctl_val;
+
+       if (!drv)
+               return -ENXIO;
+

drivers/soc/qcom/spm.c new file +12 -0

+
+       return 0;
+}
+
+static int qcom_pm_collapse(unsigned long int unused)
+{
+       int ret;
+       u32 flag;
+       int cpu = smp_processor_id();
+
+       ret = scm_set_warm_boot_addr(cpu_resume, cpu);
+       if (ret)

drivers/soc/qcom/spm.c new file +14 -0

+}
+
+static int qcom_cpu_standby(void *unused)
+{
+       int ret;
+
+       ret = spm_set_low_power_mode(PM_SLEEP_MODE_STBY);
+       if (ret)
+               return ret;
+
+       cpu_do_idle();
+
+       return 0;
+}

drivers/soc/qcom/spm.c new file +8 -0

+       cpu_pm_exit();
+
+       return 0;
+}
+
+static struct spm_driver_data *spm_get_drv(struct platform_device *pdev,
+               int *spm_cpu)
+{

drivers/soc/qcom/spm.c new file +107 -0

+               of_node_put(cpu_node);
+               if (found)
+                       break;
+       }
+
+       if (found) {
+               drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
+               if (drv)
+                       *spm_cpu = cpu;
+       }
+
+       return drv;
+}
+
+static const struct of_device_id spm_match_table[] = {
+       { .compatible = "qcom,msm8974-saw2-v2.1-cpu",
+         .data = &spm_reg_8974_8084_cpu },
+       { .compatible = "qcom,apq8084-saw2-v2.1-cpu",
+         .data = &spm_reg_8974_8084_cpu },
+       { .compatible = "qcom,apq8064-saw2-v1.1-cpu",
+         .data = &spm_reg_8064_cpu },
+       { },
+};
+
+static const struct qcom_cpu_pm_ops lpm_ops = {
+       .standby = qcom_cpu_standby,
+       .spc = qcom_cpu_spc,
+};
+
+static int spm_dev_probe(struct platform_device *pdev)
+{
+       struct spm_driver_data *drv;
+       struct resource *res;
+       const struct of_device_id *match_id;
+       void __iomem *addr;
+       int cpu = -EINVAL;
+       static bool cpuidle_drv_init;
+       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;
+
+       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");

include/soc/qcom/pm.h new file +31 -1

new file mode 100644
index 0000000..d9a56d7
@@ -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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Thu, Nov 27, 2014 at 05:24:07AM +0000, Lina Iyer wrote:

[...]

Quoted 16 lines

drivers/soc/qcom/spm.c new file +16 -0

+static int spm_set_low_power_mode(enum pm_sleep_mode mode)
+{
+       struct spm_driver_data *drv = per_cpu(cpu_spm_drv,
+                                               smp_processor_id());
+       u32 start_index;
+       u32 ctl_val;
+
+       if (!drv)
+               return -ENXIO;
+
+       start_index = drv->reg_data->start_index[mode];
+
+       ctl_val = spm_register_read(drv, SPM_REG_SPM_CTL);
+       ctl_val &= ~(SPM_CTL_INDEX << SPM_CTL_INDEX_SHIFT);
+       ctl_val |= start_index << SPM_CTL_INDEX_SHIFT;
+       ctl_val |= SPM_CTL_EN;

drivers/soc/qcom/spm.c line 151

Quoted 4 lines

drivers/soc/qcom/spm.c new file +4 -0

+       spm_register_write(drv, SPM_REG_SPM_CTL, ctl_val);
+
+       /* Ensure we have written the start address */
+       wmb();

Can you explain please what this wmb is meant to achieve ? If it is there to make sure the write made it to the SPM that's not a proper way to achieve it, the barrier ensures ordering, not write completion.

Quoted 18 lines

drivers/soc/qcom/spm.c new file +18 -0

+
+       return 0;
+}
+
+static int qcom_pm_collapse(unsigned long int unused)
+{
+       int ret;
+       u32 flag;
+       int cpu = smp_processor_id();
+
+       ret = scm_set_warm_boot_addr(cpu_resume, cpu);
+       if (ret)
+               return ret;
+
+       flag = SCM_L2_ON & SCM_FLUSH_FLAG_MASK;
+       scm_call_atomic1(SCM_SVC_BOOT, SCM_CMD_TERMINATE_PC, flag);
+
+       /*

drivers/soc/qcom/spm.c line 173

Quoted 4 lines

drivers/soc/qcom/spm.c new file +4 -0

+        * Returns here only if there was a pending interrupt and we did not
+        * power down as a result.
+        */
+       return 0;

I know the return value is changed by the cpu_suspend implementation to signal error when not returning from cpu_resume, but you should not return 0 anyway.

Quoted 18 lines

drivers/soc/qcom/spm.c new file +18 -0

+}
+
+static int qcom_cpu_standby(void *unused)
+{
+       int ret;
+
+       ret = spm_set_low_power_mode(PM_SLEEP_MODE_STBY);
+       if (ret)
+               return ret;
+
+       cpu_do_idle();
+
+       return 0;
+}
+
+static int qcom_cpu_spc(void *unused)
+{      int ret;
+

drivers/soc/qcom/spm.c line 197

Quoted 6 lines

drivers/soc/qcom/spm.c new file +6 -0

+       ret = spm_set_low_power_mode(PM_SLEEP_MODE_STBY);
+       if (ret)
+               return ret;
+
+       cpu_pm_enter();
+       cpu_suspend(0, qcom_pm_collapse);

You must stash the return value and propagate it.

Quoted 18 lines

drivers/soc/qcom/spm.c new file +18 -0

+       cpu_pm_exit();
+
+       return 0;
+}
+
+static struct spm_driver_data *spm_get_drv(struct platform_device *pdev,
+               int *spm_cpu)
+{
+       struct spm_driver_data *drv = NULL;
+       struct device_node *cpu_node, *saw_node;
+       int cpu;
+       bool found = false;
+
+       for_each_possible_cpu(cpu) {
+               cpu_node = of_cpu_device_node_get(cpu);
+               if (!cpu_node)
+                       continue;
+               saw_node = of_parse_phandle(cpu_node, "qcom,saw", 0);

drivers/soc/qcom/spm.c new file +5 -0

+               if (saw_node) {
+                       if (saw_node == pdev->dev.of_node)
+                               found = true;
+                       of_node_put(saw_node);
+               }

can be:

		found = (saw_node == pdev->dev.of_node);
		of_node_put(saw_node);

Thanks,
Lorenzo

Quoted 148 lines

drivers/soc/qcom/spm.c new file +107 -0

+               of_node_put(cpu_node);
+               if (found)
+                       break;
+       }
+
+       if (found) {
+               drv = devm_kzalloc(&pdev->dev, sizeof(*drv), GFP_KERNEL);
+               if (drv)
+                       *spm_cpu = cpu;
+       }
+
+       return drv;
+}
+
+static const struct of_device_id spm_match_table[] = {
+       { .compatible = "qcom,msm8974-saw2-v2.1-cpu",
+         .data = &spm_reg_8974_8084_cpu },
+       { .compatible = "qcom,apq8084-saw2-v2.1-cpu",
+         .data = &spm_reg_8974_8084_cpu },
+       { .compatible = "qcom,apq8064-saw2-v1.1-cpu",
+         .data = &spm_reg_8064_cpu },
+       { },
+};
+
+static const struct qcom_cpu_pm_ops lpm_ops = {
+       .standby = qcom_cpu_standby,
+       .spc = qcom_cpu_spc,
+};
+
+static int spm_dev_probe(struct platform_device *pdev)
+{
+       struct spm_driver_data *drv;
+       struct resource *res;
+       const struct of_device_id *match_id;
+       void __iomem *addr;
+       int cpu = -EINVAL;
+       static bool cpuidle_drv_init;
+       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;
+
+       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");

include/soc/qcom/pm.h new file +31 -1

new file mode 100644
index 0000000..d9a56d7
@@ -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

Quoted 5 lines

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 20 lines · Lorenzo Pieralisi

On Thu, Nov 27, 2014 at 05:24:07AM +0000, Lina Iyer wrote:

[...]

Quoted 16 lines

drivers/soc/qcom/spm.c new file +16 -0

+static int spm_set_low_power_mode(enum pm_sleep_mode mode)
+{
+       struct spm_driver_data *drv = per_cpu(cpu_spm_drv,
+                                               smp_processor_id());
+       u32 start_index;
+       u32 ctl_val;
+
+       if (!drv)
+               return -ENXIO;
+
+       start_index = drv->reg_data->start_index[mode];
+
+       ctl_val = spm_register_read(drv, SPM_REG_SPM_CTL);
+       ctl_val &= ~(SPM_CTL_INDEX << SPM_CTL_INDEX_SHIFT);
+       ctl_val |= start_index << SPM_CTL_INDEX_SHIFT;
+       ctl_val |= SPM_CTL_EN;

Quoted 8 lines

drivers/soc/qcom/spm.c line 151

Quoted 4 lines

drivers/soc/qcom/spm.c new file +4 -0

+       spm_register_write(drv, SPM_REG_SPM_CTL, ctl_val);
+
+       /* Ensure we have written the start address */
+       wmb();

Can you explain please what this wmb is meant to achieve ? If it is there to make sure the write made it to the SPM that's not a proper way to achieve it, the barrier ensures ordering, not write completion.

A write completion would be great, but assuming that the context switch using SCM or WFI could ensure the flush, then ordering is probably good enough for this.

Quoted 30 lines · Lina Iyer

Quoted 28 lines · Lorenzo Pieralisi

On Thu, Nov 27, 2014 at 05:24:07AM +0000, Lina Iyer wrote:

[...]

Quoted 10 lines

drivers/soc/qcom/spm.c new file +10 -0

+static int spm_set_low_power_mode(enum pm_sleep_mode mode)
+{
+       struct spm_driver_data *drv = per_cpu(cpu_spm_drv,
+                                               smp_processor_id());
+       u32 start_index;
+       u32 ctl_val;
+
+       if (!drv)
+               return -ENXIO;
+

drivers/soc/qcom/spm.c line 151

Quoted 10 lines

drivers/soc/qcom/spm.c new file +10 -0

+       start_index = drv->reg_data->start_index[mode];
+
+       ctl_val = spm_register_read(drv, SPM_REG_SPM_CTL);
+       ctl_val &= ~(SPM_CTL_INDEX << SPM_CTL_INDEX_SHIFT);
+       ctl_val |= start_index << SPM_CTL_INDEX_SHIFT;
+       ctl_val |= SPM_CTL_EN;
+       spm_register_write(drv, SPM_REG_SPM_CTL, ctl_val);
+
+       /* Ensure we have written the start address */
+       wmb();

Can you explain please what this wmb is meant to achieve ? If it is there to make sure the write made it to the SPM that's not a proper way to achieve it, the barrier ensures ordering, not write completion.

Quoted 3 lines

A write completion would be great, but assuming that the context switch using SCM or WFI could ensure the flush, then ordering is probably good enough for this.

I assume you want the write to the device to complete (side-effects are observed) before the WFI (which would raise a StandbyWFI signal). The safer way is to read back from the device (or check/poll for the device state change) rather than a barrier.

Dependent patchsets -

         https://lkml.org/lkml/2014/8/4/767
         http://www.spinics.net/lists/linux-arm-msm/msg10799.html
         http://www.spinics.net/lists/linux-arm-msm/msg10795.html

Changes since v12:
- Minor fixes
- Added Reviewed-by

Changes since v11:
- Address review comments on spm.c
- Commenting style fixes
- Added Reviewed-by

Changes since v10:
[ https://www.mail-archive.com/devicetree at vger.kernel.org/msg51880.html ]
- Address review comments
- Added Acked-by and Reviewed-by

Changes since v9:
[ https://www.mail-archive.com/linux-arm-msm at vger.kernel.org/msg11714.html ]
- Address review comments on v9

Changes since v8:
[ https://www.mail-archive.com/linux-arm-msm at vger.kernel.org/msg11473.html ]
- Flatten out the file structure - merge pm.c into spm.c after discussions
- Add a new function to set warm boot address, in scm-boot.c
- Support for 8064 (New)
- Tested on 8074, 8084. 8064 was tested with a WIP tree
- Address review comments from v8
- Looking into possiblility of initializing the cpuidle device for a cpu, only when the corresponding spm device is probed successfully.

Changes since v7:
[ https://www.mail-archive.com/linux-arm-msm at vger.kernel.org/msg11199.html ]
- Address review comments
- Tested on 8974 but not 8084
- WFI renamed to Standby
- Update commit text with original author and link to the downstream tree

Changes since v6:
[ https://www.mail-archive.com/linux-arm-msm at vger.kernel.org/msg11012.html ]
- SPM device nodes merged with existing SAW DT nodes
- SPM register information is handled within the driver
- Clean up from using 'msm' to 'qcom'
- Shorten some enumerations as well
- Review comments from v6 addressed
- New: Support for 8084 SoC
- Not tested. I do not have a board with this SoC, but the SPM configuration should be identical for WFI and SPC

Changes since v5:
[ https://www.mail-archive.com/linux-arm-msm at vger.kernel.org/msg10559.html ]
- Merge spm-devices.c and spm.c into one file and one patch
- Simplify implementation of the driver.
- Update documentation mapping the DT properties with corresponding SPM register information.
- Removed scm-boot changes for quad core warmboot, its has been pulled in.

Changes since v4:
[ https://www.mail-archive.com/linux-arm-msm at vger.kernel.org/msg10327.html ]
- Update to the v8 of ARM generic idle states patches
- Use platform device model for cpuidle-qcom
- Clean up msm-pm.c to remove unnecessary include files and functions
- Update commit text and documentation for all idle states
- Remove scm-boot relocate patch from this series, submitted earlier [ https://www.mail-archive.com/linux-arm-msm at vger.kernel.org/msg10518.html ]

Changes since v3:
[ https://www.mail-archive.com/linux-arm-msm at vger.kernel.org/msg10288.html ]
- Fix CONFIG_QCOM_PM Kconfig as bool
- More clean ups in spm.c and spm-devices.c
- Removed and re-organized data structures to make initialization simple
- Remove export of sequence flush functions
- Updated commit text
- Comments for use of barriers.
- Rebase on top of 3.17-rc1

Changes since v2:
[ https://www.mail-archive.com/linux-arm-msm at vger.kernel.org/msg10148.html ]
- Prune all the drivers to support basic WFI and power down cpuidle functionality. Remove debug code.
- Integrate KConfig changes into the drivers' patches.
- Use Lorenzo's ARM idle-states patches as the basis for reading cpuidle c-states from DT.
[ http://marc.info/?l=linux-pm&m=140794514812383&w=2 ]
- Incorporate review comments
- Rebase on top of 3.16

Changes since v1/RFC:
[ https://www.mail-archive.com/linux-arm-msm at vger.kernel.org/msg10065.html ]
- Remove hotplug from the patch series. Will submit it separately.
- Fix SPM drivers per the review comments
- Modify patch sequence to compile SPM drivers independent of msm-pm, so as to allow wfi() calls to use SPM even without SoC interface driver.

8074/8084/8064 like any ARM SoC can do architectural clock gating, that helps save on power, but not enough of leakage power. Leakage power of the SoC can be further reduced by turning off power to the core. To aid this, every core (cpu and L2) is accompanied by a Sub-system Power Manager (SPM), that can be configured to indicate the low power mode, the core would be put into and the SPM programs the peripheral h/w accordingly to enter low power and turn off the power rail to the core.

The idle invocation hierarchy -

CPUIDLE

         |
         cpuidle-qcom.c [CPUIdle driver]
         |
         ------> spm.c [SPM driver]
                 |
                 ------> scm-boot.c [SCM interface layer]
                         |
------------------------|--------------------------
(EL)                    Secure Monitor Code
                         |
                         |
                         wfi();
------------------------|--------------------------
(HW)                    [CPU] {clock gate}
                         |
                         -----> [SPM] {statemachine}

The patchset does the following -

- Introduce the SPM driver to control power to the core
- Add device bindings for 8974, 8084, 8064 CPU SPM devices
- Add cpuidle driver for QCOM cpus, using ARM generic idle state definitions.
- Add device bindings for 8974, 8084, 8064 idle-states - WFI and SPC

Thanks,
Lina

Lina Iyer (10):
qcom: scm: Move scm-boot files to drivers/soc/qcom/ and
include/soc/qcom
qcom: scm: Add SCM warmboot support for quad core SoCs
qcom: spm: Add Subsystem Power Manager driver
arm: dts: qcom: Add power-controller device node for 8074 Krait CPUs
arm: dts: qcom: Add power-controller device node for 8084 Krait CPUs
arm: dts: qcom: Update power-controller device node for 8064 Krait
CPUs
qcom: cpuidle: Add cpuidle driver for QCOM cpus
arm: dts: qcom: Add idle states device nodes for 8074
arm: dts: qcom: Add idle states device nodes for 8084
arm: dts: qcom: Add idle state device nodes for 8064

  .../bindings/arm/msm/qcom,idle-state.txt           |  81 +++++
  .../devicetree/bindings/arm/msm/qcom,saw2.txt      |  31 +-
  arch/arm/boot/dts/qcom-apq8064.dtsi                |  36 ++-
  arch/arm/boot/dts/qcom-apq8084.dtsi                |  46 ++-
  arch/arm/boot/dts/qcom-msm8974.dtsi                |  46 ++-
  arch/arm/mach-qcom/Makefile                        |   1 -
  arch/arm/mach-qcom/platsmp.c                       |   2 +-
  drivers/cpuidle/Kconfig.arm                        |   7 +
  drivers/cpuidle/Makefile                           |   1 +
  drivers/cpuidle/cpuidle-qcom.c                     |  78 +++++
  drivers/soc/qcom/Kconfig                           |   8 +
  drivers/soc/qcom/Makefile                          |   3 +-
  .../arm/mach-qcom => drivers/soc/qcom}/scm-boot.c  |  37 ++-
  drivers/soc/qcom/spm.c                             | 327 +++++++++++++++++++++
  include/soc/qcom/pm.h                              |  31 ++
  .../arm/mach-qcom => include/soc/qcom}/scm-boot.h  |   3 +-
  16 files changed, 716 insertions(+), 22 deletions(-)

create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt create mode 100644 drivers/cpuidle/cpuidle-qcom.c
rename {arch/arm/mach-qcom => drivers/soc/qcom}/scm-boot.c (59%) create mode 100644 drivers/soc/qcom/spm.c
create mode 100644 include/soc/qcom/pm.h
rename {arch/arm/mach-qcom => include/soc/qcom}/scm-boot.h (91%)