cpuidle driver for QCOM SoCs: 8064, 8074, 8084

Set the warmboot address using an SCM call, only if the new address is different than the old one.

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

---
  drivers/soc/qcom/scm-boot.c | 22 ++++++++++++++++++++++
  include/soc/qcom/scm-boot.h |  1 +
  2 files changed, 23 insertions(+)

drivers/soc/qcom/scm-boot.c +0 -0

index 60ff7b4..5710967 100644

include/soc/qcom/scm-boot.h +1 -0

  int scm_set_boot_addr(phys_addr_t addr, int flags);
+int scm_set_warm_boot_addr(void *entry, int cpu);
 
  #endif
 

Quoted 2 lines · Lina Iyer

Set the warmboot address using an SCM call, only if the new address is different than the old one.

Please could you elaborate why a new address can be changed ?

Quoted 11 lines

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

---
 drivers/soc/qcom/scm-boot.c | 22 ++++++++++++++++++++++
 include/soc/qcom/scm-boot.h |  1 +
 2 files changed, 23 insertions(+)

drivers/soc/qcom/scm-boot.c +0 -0

index 60ff7b4..5710967 100644

drivers/soc/qcom/scm-boot.c int scm_set_boot_addr(phys_addr_t addr, int flags) · line 41

Quoted 5 lines

drivers/soc/qcom/scm-boot.c +2 -0

@@ -37,3 +37,25 @@ int scm_set_boot_addr(phys_addr_t addr, int flags)
 			&cmd, sizeof(cmd), NULL, 0);
 }
 EXPORT_SYMBOL(scm_set_boot_addr);
+
+

extra line.

drivers/soc/qcom/scm-boot.c int scm_set_boot_addr(phys_addr_t addr, int flags) · line 49

Quoted 8 lines

drivers/soc/qcom/scm-boot.c +8 -0

+int scm_set_warm_boot_addr(void *entry, int cpu)
+{
+	static int flags[NR_CPUS] = {
+		SCM_FLAG_WARMBOOT_CPU0,
+		SCM_FLAG_WARMBOOT_CPU1,
+		SCM_FLAG_WARMBOOT_CPU2,
+		SCM_FLAG_WARMBOOT_CPU3,
+	};

Please do not do that, you don't know what NR_CPUS value could be in the future with the single kernel image and that could lead to a bug very hard to find. The kernel stack is 4096.

Move this out of the function:

static int scm_flags[] = {
SCM_FLAG_WARMBOOT_CPU0,
SCM_FLAG_WARMBOOT_CPU1,
SCM_FLAG_WARMBOOT_CPU2,
SCM_FLAG_WARMBOOT_CPU3,
};

drivers/soc/qcom/scm-boot.c int scm_set_boot_addr(phys_addr_t addr, int flags) · line 50

Quoted 2 lines

drivers/soc/qcom/scm-boot.c +1 -0

+	static DEFINE_PER_CPU(void *, last_known_entry);

It sounds odd to add those static declaration here even if I understand that is to encapsulate them.

drivers/soc/qcom/scm-boot.c int scm_set_boot_addr(phys_addr_t addr, int flags) · line 54

Quoted 4 lines

drivers/soc/qcom/scm-boot.c +4 -0

+	int ret;
+
+	if (entry == per_cpu(last_known_entry, cpu))
+		return 0;

My question is: why scm_set_warm_boot_addr could be called with different addresses ?

If this is really needed, please replace the per_cpu variable by:

struct scm_boot_addr {
	int flag;
	phys_addr_t entry;
};

static struct scm_boot_addr scm_flags[] = {
	{ SCM_FLAG_WARMBOOT_CPU0, },
	{ SCM_FLAG_WARMBOOT_CPU1, },
	{ SCM_FLAG_WARMBOOT_CPU2, },
	{ SCM_FLAG_WARMBOOT_CPU3, },
};

Quoted 7 lines

drivers/soc/qcom/scm-boot.c +6 -0

+	ret = scm_set_boot_addr(virt_to_phys(entry), flags[cpu]);
+	if (!ret)
+		per_cpu(last_known_entry, cpu) = entry;
+
+	return ret;
+}

include/soc/qcom/scm-boot.h +0 -0

include/soc/qcom/scm-boot.h line 22

Quoted 5 lines

index 02b445c..100938b 100644

include/soc/qcom/scm-boot.h +0 -0

@@ -22,5 +22,6 @@
 #define SCM_FLAG_WARMBOOT_CPU3		0x40

By the way, if you look for encapsulation, perhaps these macros could be moved into scm-boot.c, no ?

Quoted 5 lines

include/soc/qcom/scm-boot.h +1 -0

 int scm_set_boot_addr(phys_addr_t addr, int flags);
+int scm_set_warm_boot_addr(void *entry, int cpu);
 
 #endif
 

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

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

---
  .../devicetree/bindings/arm/msm/qcom,saw2.txt      |  31 +-
  drivers/soc/qcom/Kconfig                           |   8 +
  drivers/soc/qcom/Makefile                          |   1 +
  drivers/soc/qcom/spm.c                             | 334 +++++++++++++++++++++
  include/soc/qcom/pm.h                              |  31 ++
  5 files changed, 399 insertions(+), 6 deletions(-)

create mode 100644 drivers/soc/qcom/spm.c
create mode 100644 include/soc/qcom/pm.h

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

index 1505fb8..690c3c0 100644
@@ -2,11 +2,20 @@ SPM AVS Wrapper 2 (SAW2)
 
  The SAW2 is a wrapper around the Subsystem Power Manager (SPM) and the
  Adaptive Voltage Scaling (AVS) hardware. The SPM is a programmable
-micro-controller that transitions a piece of hardware (like a processor or
+power-controller that transitions a piece of hardware (like a processor or
  subsystem) into and out of low power modes via a direct connection to
  the PMIC. It can also be wired up to interact with other processors in the
  system, notifying them when a low power state is entered or exited.
 
+Multiple revisions of the SAW hardware are supported using these Device Nodes.
+SAW2 revisions differ in the register offset and configuration data. Also, the
+same revision of the SAW in different SoCs may have different configuration
+data due the the differences in hardware capabilities. Hence the SoC name, the
+version of the SAW hardware in that SoC and the distinction between cpu (big
+or Little) or cache, may be needed to uniquely identify the SAW register
+configuration and initialization data. The compatible string is used to
+indicate this parameter.
+
  PROPERTIES
 
  - compatible:

@@ -14,10 +23,13 @@ PROPERTIES
  	Value type: <string>
  	Definition: shall contain "qcom,saw2". A more specific value should be
  		    one of:
-			 "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"
+			"qcom,apq8064-saw2-v1.1-cpu"
+			"qcom,msm8974-saw2-v2.1-cpu"
+			"qcom,apq8084-saw2-v2.1-cpu"
 
  - reg:
  	Usage: required

@@ -26,10 +38,17 @@ PROPERTIES
  		    the register region. An optional second element specifies
  		    the base address and size of the alias register region.
 
+- regulator:
+	Usage: optional
+	Value type: boolean
+	Definition: Indicates that this SPM device acts as a regulator device
+			device for the core (CPU or Cache) the SPM is attached
+			to.
 
  Example:
 
-	regulator at 2099000 {
+	power-controller at 2099000 {
  		compatible = "qcom,saw2";
  		reg = <0x02099000 0x1000>, <0x02009000 0x1000>;
+		regulator;
  	};

drivers/soc/qcom/Kconfig +8 -0

index 7dcd554..012fb37 100644
@@ -11,3 +11,11 @@ config QCOM_GSBI
 
  config QCOM_SCM
  	bool
+
+config QCOM_PM
+	bool "Qualcomm Power Management"
+	depends on ARCH_QCOM
+	help
+	  QCOM Platform specific power driver to manage cores and L2 low power
+	  modes. It interface with various system drivers to put the cores in
+	  low power modes.

drivers/soc/qcom/Makefile +1 -0

index 70d52ed..20b329f 100644
@@ -1,3 +1,4 @@
  obj-$(CONFIG_QCOM_GSBI)	+=	qcom_gsbi.o
+obj-$(CONFIG_QCOM_PM)	+=	spm.o
  CFLAGS_scm.o :=$(call as-instr,.arch_extension sec,-DREQUIRES_SEC=1)
  obj-$(CONFIG_QCOM_SCM) += scm.o scm-boot.o

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

new file mode 100644
index 0000000..ee2e3ca
@@ -0,0 +1,334 @@
+/* Copyright (c) 2011-2014, The Linux Foundation. All rights reserved.
+ *
+ * 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/kernel.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
+
+#include <asm/proc-fns.h>
+#include <asm/suspend.h>
+
+#include <soc/qcom/pm.h>
+#include <soc/qcom/pm.h>
+#include <soc/qcom/scm.h>
+#include <soc/qcom/scm-boot.h>
+
+
+#define SCM_CMD_TERMINATE_PC	(0x2)
+#define SCM_FLUSH_FLAG_MASK	(0x3)
+#define SCM_L2_ON		(0x0)
+#define SCM_L2_OFF		(0x1)
+#define MAX_PMIC_DATA 2
+#define MAX_SEQ_DATA 64
+
+#define SPM_CTL_INDEX 0x7f
+#define SPM_CTL_INDEX_SHIFT 4
+#define SPM_CTL_EN BIT(0)
+
+enum spm_reg {
+	SPM_REG_CFG,
+	SPM_REG_SPM_CTL,
+	SPM_REG_DLY,
+	SPM_REG_PMIC_DLY,
+	SPM_REG_PMIC_DATA_0,
+	SPM_REG_PMIC_DATA_1,
+	SPM_REG_VCTL,
+	SPM_REG_SEQ_ENTRY,
+	SPM_REG_SPM_STS,
+	SPM_REG_PMIC_STS,
+	SPM_REG_NR,
+};
+
+struct spm_reg_data {
+	/* Register position */
+	const u8 *reg_offset;
+
+	/* Register initialization values */
+	u32 spm_cfg;
+	u32 spm_dly;
+	u32 pmic_dly;
+	u32 pmic_data[MAX_PMIC_DATA];
+
+	/* Sequences and start indices */
+	u8 seq[MAX_SEQ_DATA];
+	u8 start_index[PM_SLEEP_MODE_NR];
+
+};
+
+struct spm_driver_data {
+	void __iomem *reg_base;
+	const struct spm_reg_data *reg_data;
+	bool available;
+};
+
+static const u8 spm_reg_offset_v2_1[SPM_REG_NR] = {
+	[SPM_REG_CFG]		= 0x08,
+	[SPM_REG_SPM_CTL]	= 0x30,
+	[SPM_REG_DLY]		= 0x34,
+	[SPM_REG_SEQ_ENTRY]	= 0x80,
+};
+
+/* SPM register data for 8974, 8084 */
+static const struct spm_reg_data spm_reg_8974_8084_cpu  = {
+	.reg_offset = spm_reg_offset_v2_1,
+	.spm_cfg = 0x1,
+	.spm_dly = 0x3C102800,
+	.seq = { 0x03, 0x0B, 0x0F, 0x00, 0x20, 0x80, 0x10, 0xE8, 0x5B, 0x03,
+		0x3B, 0xE8, 0x5B, 0x82, 0x10, 0x0B, 0x30, 0x06, 0x26, 0x30,
+		0x0F },
+	.start_index[PM_SLEEP_MODE_STBY] = 0,
+	.start_index[PM_SLEEP_MODE_SPC] = 3,
+};
+
+static const u8 spm_reg_offset_v1_1[SPM_REG_NR] = {
+	[SPM_REG_CFG]		= 0x08,
+	[SPM_REG_SPM_CTL]	= 0x20,
+	[SPM_REG_PMIC_DLY]	= 0x24,
+	[SPM_REG_PMIC_DATA_0]	= 0x28,
+	[SPM_REG_PMIC_DATA_1]	= 0x2C,
+	[SPM_REG_SEQ_ENTRY]	= 0x80,
+};
+
+/* SPM register data for 8064 */
+static const struct spm_reg_data spm_reg_8064_cpu = {
+	.reg_offset = spm_reg_offset_v1_1,
+	.spm_cfg = 0x1F,
+	.pmic_dly = 0x02020004,
+	.pmic_data[0] = 0x0084009C,
+	.pmic_data[1] = 0x00A4001C,
+	.seq = { 0x03, 0x0F, 0x00, 0x24, 0x54, 0x10, 0x09, 0x03, 0x01,
+		0x10, 0x54, 0x30, 0x0C, 0x24, 0x30, 0x0F },
+	.start_index[PM_SLEEP_MODE_STBY] = 0,
+	.start_index[PM_SLEEP_MODE_SPC] = 2,
+};
+
+static DEFINE_PER_CPU_SHARED_ALIGNED(struct spm_driver_data, cpu_spm_drv);
+
+static inline void spm_register_write(struct spm_driver_data *drv,
+					enum spm_reg reg, u32 val)
+{
+	if (drv->reg_data->reg_offset[reg])
+		writel_relaxed(val, drv->reg_base +
+				drv->reg_data->reg_offset[reg]);
+}
+
+static inline u32 spm_register_read(struct spm_driver_data *drv,
+					enum spm_reg reg)
+{
+	return readl_relaxed(drv->reg_base + drv->reg_data->reg_offset[reg]);
+}
+
+/**
+ * spm_set_low_power_mode() - Configure SPM start address for low power mode

drivers/soc/qcom/spm.c new file +154 -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();
+
+	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) {
+		pr_err("Failed to set warm boot address for cpu %d\n", cpu);
+		return ret;
+	}
+
+	flag = SCM_L2_ON & SCM_FLUSH_FLAG_MASK;
+	scm_call_atomic1(SCM_SVC_BOOT, SCM_CMD_TERMINATE_PC, flag);
+
+	/**
+	 *  Returns here only if there was a pending interrupt and we did not
+	 *  power down as a result.
+	 */
+	return 0;
+}
+
+static int qcom_cpu_standby(void *unused)
+{
+	int ret;
+
+	ret = qcom_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;
+
+	ret = qcom_spm_set_low_power_mode(PM_SLEEP_MODE_STBY);
+	if (ret)
+		return ret;
+
+	cpu_pm_enter();
+	cpu_suspend(0, qcom_pm_collapse);
+	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;
+
+	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)
+				drv = &per_cpu(cpu_spm_drv, cpu);
+			of_node_put(saw_node);
+		}
+		of_node_put(cpu_node);
+		if (drv) {
+			*spm_cpu = cpu;
+			break;
+		}
+	}
+
+	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 struct qcom_cpu_pm_ops lpm_ops = {
+	.standby = qcom_cpu_standby,
+	.spc = qcom_cpu_spc,
+};
+
+struct platform_device qcom_cpuidle_device = {
+	.name              = "qcom_cpuidle",
+	.id                = -1,
+	.dev.platform_data = &lpm_ops,
+};
+
+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;
+	const u32 *seq_data;
+	int cpu = -EINVAL;
+	static bool cpuidle_drv_init;
+
+	drv = spm_get_drv(pdev, &cpu);
+	if (!drv)
+		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;
+	if (!drv->reg_data)
+		return -EINVAL;
+
+	/* Write the SPM sequences, first.. */
+	addr = drv->reg_base + drv->reg_data->reg_offset[SPM_REG_SEQ_ENTRY];
+	seq_data = (const u32 *)drv->reg_data->seq;
+	__iowrite32_copy(addr, seq_data, 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,

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

+	return 0;
+}
+
+static struct platform_driver spm_driver = {
+	.probe = spm_dev_probe,
+	.driver = {
+		.name = "qcom,spm",
+		.of_match_table = spm_match_table,
+	},
+};
+
+module_platform_driver(spm_driver);

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 */

drivers/soc/qcom/spm.c line 152

Quoted 12 lines · Lina Iyer

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

+/**
+ * spm_set_low_power_mode() - Configure SPM start address for low power mode
+ * @mode: SPM LPM mode to enter
+ */
+int qcom_spm_set_low_power_mode(enum pm_sleep_mode mode)
+{
+	struct spm_driver_data *drv = this_cpu_ptr(&cpu_spm_drv);
+	u32 start_index;
+	u32 ctl_val;
+
+	if (!drv->available)
+		return -ENXIO;

really weird how this was initialized.

Are you sure 'drv' is not NULL if it is not available ? (see below)

+static int spm_dev_probe(struct platform_device *pdev)

Quoted 41 lines

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

+{
+	struct spm_driver_data *drv;
+	struct resource *res;
+	const struct of_device_id *match_id;
+	void __iomem *addr;
+	const u32 *seq_data;
+	int cpu = -EINVAL;
+	static bool cpuidle_drv_init;
+
+	drv = spm_get_drv(pdev, &cpu);
+	if (!drv)
+		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;
+	if (!drv->reg_data)
+		return -EINVAL;
+
+	/* Write the SPM sequences, first.. */
+	addr = drv->reg_base + drv->reg_data->reg_offset[SPM_REG_SEQ_ENTRY];
+	seq_data = (const u32 *)drv->reg_data->seq;
+	__iowrite32_copy(addr, seq_data, 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,

drivers/soc/qcom/spm.c line 316

Quoted 10 lines

drivers/soc/qcom/spm.c new file +10 -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();
+	drv->available = true;

IMO if you have to do that, that means there is something wrong with how is initialized the driver.

It should be drv == NULL => not available

drivers/soc/qcom/spm.c line 321

Quoted 5 lines

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

+
+	if ((cpu > -1) && !cpuidle_drv_init) {
+		platform_device_register(&qcom_cpuidle_device);
+		cpuidle_drv_init = true;
+	}

'cpu' is always > -1.

If the 'spm_get_drv' succeed, cpu is no longer equal to -EINVAL. Otherwise we do not reach this point because we return right after spm_get_drv with an error.

Adding the platform_device_register depending in a static variable is not very nice. Why not add it explicitely in a separate init routine we know it will be called one time (eg. at the same place than cpufreq is) ?

Quoted 18 lines · Daniel Lezcano

drivers/soc/qcom/spm.c line 152

Quoted 13 lines · Lina Iyer

+/**
+ * spm_set_low_power_mode() - Configure SPM start address for low power mode
+ * @mode: SPM LPM mode to enter
+ */

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

+int qcom_spm_set_low_power_mode(enum pm_sleep_mode mode)
+{
+    struct spm_driver_data *drv = this_cpu_ptr(&cpu_spm_drv);
+    u32 start_index;
+    u32 ctl_val;
+
+    if (!drv->available)
+        return -ENXIO;

really weird how this was initialized.

Quoted 2 lines

Are you sure 'drv' is not NULL if it is not available ? (see below)

'drv' has some data that I need to decode the register address. Hence cant have that NULL. Therefore, the available flag.

...

Quoted 59 lines

+static int spm_dev_probe(struct platform_device *pdev)

Quoted 44 lines

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

+{
+    struct spm_driver_data *drv;
+    struct resource *res;
+    const struct of_device_id *match_id;
+    void __iomem *addr;
+    const u32 *seq_data;
+    int cpu = -EINVAL;
+    static bool cpuidle_drv_init;
+
+    drv = spm_get_drv(pdev, &cpu);
+    if (!drv)
+        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;
+    if (!drv->reg_data)
+        return -EINVAL;
+
+    /* 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 +5 -0

+    seq_data = (const u32 *)drv->reg_data->seq;
+    __iowrite32_copy(addr, seq_data, 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 +7 -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,

drivers/soc/qcom/spm.c line 316

Quoted 10 lines

drivers/soc/qcom/spm.c new file +10 -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();
+    drv->available = true;

IMO if you have to do that, that means there is something wrong with how is initialized the driver.

Quoted 2 lines

It should be drv == NULL => not available

drv has the register base that I dont want to pass seprately.

Quoted 8 lines

drivers/soc/qcom/spm.c line 321

Quoted 5 lines

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

+
+    if ((cpu > -1) && !cpuidle_drv_init) {
+        platform_device_register(&qcom_cpuidle_device);
+        cpuidle_drv_init = true;
+    }

'cpu' is always > -1.

OK. I was hoping to use -1 for not a cpu (i.e, L2) SPM. For now, I will change.

Quoted 9 lines

If the 'spm_get_drv' succeed, cpu is no longer equal to -EINVAL. Otherwise we do not reach this point because we return right after spm_get_drv with an error.

Adding the platform_device_register depending in a static variable is not very nice. Why not add it explicitely in a separate init routine we know it will be called one time (eg. at the same place than cpufreq is) ?

We want to register the cpuidle device only if any of the SPM devices have been probed.

Ideally, Stephen and I would like to register cpuidle device separately for each CPU SPM, when it is probed, so we would invoke cpuidle driver and thereby the low power modes only for those cpus. However, the complexity to do that, AFAICS, is very complex. I would need to change quite a bit of the framework and in the cpuidle driver, I may have to stray from the recommended format.

Here I set up cpuidle device, when I know atleast 1 cpu is ready to allow low power modes.

Quoted 35 lines · Lina Iyer

Quoted 11 lines · Daniel Lezcano

drivers/soc/qcom/spm.c line 321

Quoted 5 lines · Lina Iyer

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

+
+    if ((cpu > -1) && !cpuidle_drv_init) {
+        platform_device_register(&qcom_cpuidle_device);
+        cpuidle_drv_init = true;
+    }

'cpu' is always > -1.

OK. I was hoping to use -1 for not a cpu (i.e, L2) SPM. For now, I will change.

Quoted 9 lines

If the 'spm_get_drv' succeed, cpu is no longer equal to -EINVAL. Otherwise we do not reach this point because we return right after spm_get_drv with an error.

Adding the platform_device_register depending in a static variable is not very nice. Why not add it explicitely in a separate init routine we know it will be called one time (eg. at the same place than cpufreq is) ?

We want to register the cpuidle device only if any of the SPM devices have been probed.

Ideally, Stephen and I would like to register cpuidle device separately for each CPU SPM, when it is probed, so we would invoke cpuidle driver and thereby the low power modes only for those cpus. However, the complexity to do that, AFAICS, is very complex. I would need to change quite a bit of the framework and in the cpuidle driver, I may have to stray from the recommended format.

Quoted 2 lines

Here I set up cpuidle device, when I know atleast 1 cpu is ready to allow low power modes.

Yes, instead of using the generic cpuidle_register function, you can use the low level functions for that.

One call to cpuidle_register_driver in a single place and then cpuidle_register_device for each spm probe.

Wouldn't make sense ?

Quoted 39 lines · Daniel Lezcano

Quoted 35 lines · Lina Iyer

Quoted 11 lines · Daniel Lezcano

drivers/soc/qcom/spm.c line 321

Quoted 5 lines · Lina Iyer

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

+
+    if ((cpu > -1) && !cpuidle_drv_init) {
+        platform_device_register(&qcom_cpuidle_device);
+        cpuidle_drv_init = true;
+    }

'cpu' is always > -1.

OK. I was hoping to use -1 for not a cpu (i.e, L2) SPM. For now, I will change.

Quoted 9 lines

If the 'spm_get_drv' succeed, cpu is no longer equal to -EINVAL. Otherwise we do not reach this point because we return right after spm_get_drv with an error.

Adding the platform_device_register depending in a static variable is not very nice. Why not add it explicitely in a separate init routine we know it will be called one time (eg. at the same place than cpufreq is) ?

We want to register the cpuidle device only if any of the SPM devices have been probed.

Ideally, Stephen and I would like to register cpuidle device separately for each CPU SPM, when it is probed, so we would invoke cpuidle driver and thereby the low power modes only for those cpus. However, the complexity to do that, AFAICS, is very complex. I would need to change quite a bit of the framework and in the cpuidle driver, I may have to stray from the recommended format.

Quoted 2 lines

Here I set up cpuidle device, when I know atleast 1 cpu is ready to allow low power modes.

Quoted 7 lines

Yes, instead of using the generic cpuidle_register function, you can use the low level functions for that.

One call to cpuidle_register_driver in a single place and then cpuidle_register_device for each spm probe.

Wouldn't make sense ?

Yes, but there are some assumptions if we dont use MULTIPLE_CPUIDLE_DRIVERS like this -

static void __cpuidle_driver_init(struct cpuidle_driver *drv)

{
      int i;
	
	drv->refcnt = 0; // Overwrites any cpuidle_driver_get()

The clean way was to use MULTIPLE_CPUIDLE_DRIVERS, which seems like an incorrect use for this SoC.

Thanks,
Lina

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

+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
+
+#include <asm/proc-fns.h>
+#include <asm/suspend.h>
+
+#include <soc/qcom/pm.h>
+#include <soc/qcom/pm.h>
+#include <soc/qcom/scm.h>
+#include <soc/qcom/scm-boot.h>
+
+

drivers/soc/qcom/spm.c new file +14 -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();
+
+	return 0;
+}

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

+		return ret;
+	}
+
+	flag = SCM_L2_ON & SCM_FLUSH_FLAG_MASK;
+	scm_call_atomic1(SCM_SVC_BOOT, SCM_CMD_TERMINATE_PC, flag);
+
+	/**
+	 *  Returns here only if there was a pending interrupt and we did not
+	 *  power down as a result.
+	 */
+	return 0;
+}

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

+
+static int spm_dev_probe(struct platform_device *pdev)
+{

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

+
+	/* ..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,

drivers/soc/qcom/spm.c line 16

Quoted 8 lines · Lina Iyer

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

new file mode 100644
index 0000000..ee2e3ca
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/delay.h>

Is this used?

Quoted 24 lines

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

+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
+
+#include <asm/proc-fns.h>
+#include <asm/suspend.h>
+
+#include <soc/qcom/pm.h>
+#include <soc/qcom/pm.h>
+#include <soc/qcom/scm.h>
+#include <soc/qcom/scm-boot.h>
+
+
+#define SCM_CMD_TERMINATE_PC	(0x2)
+#define SCM_FLUSH_FLAG_MASK	(0x3)
+#define SCM_L2_ON		(0x0)
+#define SCM_L2_OFF		(0x1)

drivers/soc/qcom/spm.c line 46

Quoted 6 lines

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

+#define MAX_PMIC_DATA 2
+#define MAX_SEQ_DATA 64
+
+#define SPM_CTL_INDEX 0x7f
+#define SPM_CTL_INDEX_SHIFT 4
+#define SPM_CTL_EN BIT(0)

Nitpick, why aren't these also tabbed out?

drivers/soc/qcom/spm.c line 145

Quoted 6 lines

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

+
+/**
+ * spm_set_low_power_mode() - Configure SPM start address for low power mode
+ * @mode: SPM LPM mode to enter
+ */
+int qcom_spm_set_low_power_mode(enum pm_sleep_mode mode)

static?

drivers/soc/qcom/spm.c line 152

Quoted 7 lines

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

+{
+	struct spm_driver_data *drv = this_cpu_ptr(&cpu_spm_drv);
+	u32 start_index;
+	u32 ctl_val;
+
+	if (!drv->available)
+		return -ENXIO;

It would be nice if we didn't need this by only registering the cpuidle device for this CPU once we've initialized the SPM hardware.

Quoted 20 lines

drivers/soc/qcom/spm.c new file +20 -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();
+
+	return 0;
+}
+
+static int qcom_pm_collapse(unsigned long int unused)
+{
+	int ret;
+	u32 flag;
+	int cpu = smp_processor_id();

drivers/soc/qcom/spm.c line 176

Quoted 4 lines

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

+
+	ret = scm_set_warm_boot_addr(cpu_resume, cpu);
+	if (ret) {
+		pr_err("Failed to set warm boot address for cpu %d\n", cpu);

Do we want this print? Won't it start spamming the log if we go idle and we can't set the flag? Maybe we should just be silent and return an error.

Quoted 12 lines

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

+		return ret;
+	}
+
+	flag = SCM_L2_ON & SCM_FLUSH_FLAG_MASK;
+	scm_call_atomic1(SCM_SVC_BOOT, SCM_CMD_TERMINATE_PC, flag);
+
+	/**
+	 *  Returns here only if there was a pending interrupt and we did not
+	 *  power down as a result.
+	 */
+	return 0;
+}

[...]

drivers/soc/qcom/spm.c line 254

Quoted 2 lines

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

+
+static struct qcom_cpu_pm_ops lpm_ops = {

const?

drivers/soc/qcom/spm.c line 263

Quoted 9 lines

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

+	.standby = qcom_cpu_standby,
+	.spc = qcom_cpu_spc,
+};
+
+struct platform_device qcom_cpuidle_device = {
+	.name              = "qcom_cpuidle",
+	.id                = -1,
+	.dev.platform_data = &lpm_ops,
+};

This can be created dynamically instead of living statically.

Quoted 3 lines

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

+
+static int spm_dev_probe(struct platform_device *pdev)
+{

[...]

drivers/soc/qcom/spm.c line 290

Quoted 8 lines

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

+
+	match_id = of_match_node(spm_match_table, pdev->dev.of_node);
+	if (!match_id)
+		return -ENODEV;
+
+	drv->reg_data = match_id->data;
+	if (!drv->reg_data)
+		return -EINVAL;

This check seems useless. We control the .data field right above this function so there better be some reg_data.

drivers/soc/qcom/spm.c line 294

Quoted 4 lines

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

+
+	/* Write the SPM sequences, first.. */
+	addr = drv->reg_base + drv->reg_data->reg_offset[SPM_REG_SEQ_ENTRY];
+	seq_data = (const u32 *)drv->reg_data->seq;

Why do we need a cast?

drivers/soc/qcom/spm.c line 295

Quoted 1 line

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

+	__iowrite32_copy(addr, seq_data, ARRAY_SIZE(drv->reg_data->seq)/4);

nitpick: space around that / please.

Quoted 17 lines

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

+
+	/* ..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

drivers/soc/qcom/spm.c line 318

Quoted 6 lines

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

+	 * cpuidle driver is allowed to use the SPM.
+	 */
+	wmb();
+	drv->available = true;
+
+	if ((cpu > -1) && !cpuidle_drv_init) {

Nitpick: () are unnecessary.

Quoted 5 lines

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

+		platform_device_register(&qcom_cpuidle_device);
+		cpuidle_drv_init = true;
+	}
+
+	return 0;

drivers/soc/qcom/spm.c line 329

Quoted 6 lines

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

+}
+
+static struct platform_driver spm_driver = {
+	.probe = spm_dev_probe,
+	.driver = {
+		.name = "qcom,spm",

This is an odd driver name with the "qcom," part. Maybe call it "spm" or "saw"?

drivers/soc/qcom/spm.c line 334

Quoted 5 lines

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

+		.of_match_table = spm_match_table,
+	},
+};
+
+module_platform_driver(spm_driver);

MODULE_LICENSE()?
MODULE_ALIAS()?

Quoted 14 lines · Stephen Boyd

drivers/soc/qcom/spm.c line 152

Quoted 7 lines · Lina Iyer

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

+{
+    struct spm_driver_data *drv = this_cpu_ptr(&cpu_spm_drv);
+    u32 start_index;
+    u32 ctl_val;
+
+    if (!drv->available)
+        return -ENXIO;

It would be nice if we didn't need this by only registering the cpuidle device for this CPU once we've initialized the SPM hardware.

I did explore it. It strays our cpuidle code away from the standard code that we are trying to go towards with idle-states framework.

Quoted 8 lines

Quoted 5 lines

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

+
+    /* Write the SPM sequences, first.. */
+    addr = drv->reg_base +

drv->reg_data->reg_offset[SPM_REG_SEQ_ENTRY];
+ seq_data = (const u32 *)drv->reg_data->seq;

Why do we need a cast?

Compiler warns otherwise.

Quoted 10 lines

drivers/soc/qcom/spm.c line 334

Quoted 6 lines

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

+        .of_match_table = spm_match_table,
+    },
+};
+
+module_platform_driver(spm_driver);

MODULE_LICENSE()?
MODULE_ALIAS()?

MODULE_DESCRIPTION() would work?

(quoted diff) +2 -0

+}
+

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();

On 10/25/2014 01:40 AM, Lina Iyer wrote:

Hi Lina,

[ ... ]

drivers/soc/qcom/spm.c line 132

Quoted 6 lines

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

+static inline void spm_register_write(struct spm_driver_data *drv,
+					enum spm_reg reg, u32 val)
+{
+	if (drv->reg_data->reg_offset[reg])
+		writel_relaxed(val, drv->reg_base +
+				drv->reg_data->reg_offset[reg]);

Why not use writel and don't use 'wmb' below ?

Quoted 2 lines

(quoted diff) +2 -0

+}
+

[ ... ]

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();

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

On 10/25/2014 01:40 AM, Lina Iyer wrote:

Hi Lina,

[ ... ]

drivers/soc/qcom/spm.c line 132

Quoted 6 lines

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

+static inline void spm_register_write(struct spm_driver_data *drv,
+                                       enum spm_reg reg, u32 val)
+{
+       if (drv->reg_data->reg_offset[reg])
+               writel_relaxed(val, drv->reg_base +
+                               drv->reg_data->reg_offset[reg]);

Why not use writel and don't use 'wmb' below ?

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

---
 .../devicetree/bindings/arm/msm/qcom,saw2.txt      |  31 +-
 drivers/soc/qcom/Kconfig                           |   8 +
 drivers/soc/qcom/Makefile                          |   1 +
 drivers/soc/qcom/spm.c                             | 334 +++++++++++++++++++++
 include/soc/qcom/pm.h                              |  31 ++
 5 files changed, 399 insertions(+), 6 deletions(-)
 create mode 100644 drivers/soc/qcom/spm.c
 create mode 100644 include/soc/qcom/pm.h

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

index 1505fb8..690c3c0 100644
@@ -2,11 +2,20 @@ SPM AVS Wrapper 2 (SAW2)
 
 The SAW2 is a wrapper around the Subsystem Power Manager (SPM) and the
 Adaptive Voltage Scaling (AVS) hardware. The SPM is a programmable
-micro-controller that transitions a piece of hardware (like a processor or
+power-controller that transitions a piece of hardware (like a processor or
 subsystem) into and out of low power modes via a direct connection to
 the PMIC. It can also be wired up to interact with other processors in the
 system, notifying them when a low power state is entered or exited.
 
+Multiple revisions of the SAW hardware are supported using these Device Nodes.
+SAW2 revisions differ in the register offset and configuration data. Also, the
+same revision of the SAW in different SoCs may have different configuration
+data due the the differences in hardware capabilities. Hence the SoC name, the
+version of the SAW hardware in that SoC and the distinction between cpu (big
+or Little) or cache, may be needed to uniquely identify the SAW register
+configuration and initialization data. The compatible string is used to
+indicate this parameter.
+
 PROPERTIES
 
 - compatible:

@@ -14,10 +23,13 @@ PROPERTIES
 	Value type: <string>
 	Definition: shall contain "qcom,saw2". A more specific value should be
 		    one of:
-			 "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"
+			"qcom,apq8064-saw2-v1.1-cpu"
+			"qcom,msm8974-saw2-v2.1-cpu"
+			"qcom,apq8084-saw2-v2.1-cpu"
 
 - reg:
 	Usage: required

@@ -26,10 +38,17 @@ PROPERTIES
 		    the register region. An optional second element specifies
 		    the base address and size of the alias register region.
 
+- regulator:
+	Usage: optional
+	Value type: boolean
+	Definition: Indicates that this SPM device acts as a regulator device
+			device for the core (CPU or Cache) the SPM is attached
+			to.
 
 Example:
 
-	regulator at 2099000 {
+	power-controller at 2099000 {
 		compatible = "qcom,saw2";
 		reg = <0x02099000 0x1000>, <0x02009000 0x1000>;
+		regulator;
 	};

drivers/soc/qcom/Kconfig +8 -0

index 7dcd554..012fb37 100644
@@ -11,3 +11,11 @@ config QCOM_GSBI
 
 config QCOM_SCM
 	bool
+
+config QCOM_PM
+	bool "Qualcomm Power Management"
+	depends on ARCH_QCOM
+	help
+	  QCOM Platform specific power driver to manage cores and L2 low power
+	  modes. It interface with various system drivers to put the cores in
+	  low power modes.

drivers/soc/qcom/Makefile +1 -0

index 70d52ed..20b329f 100644
@@ -1,3 +1,4 @@
 obj-$(CONFIG_QCOM_GSBI)	+=	qcom_gsbi.o
+obj-$(CONFIG_QCOM_PM)	+=	spm.o
 CFLAGS_scm.o :=$(call as-instr,.arch_extension sec,-DREQUIRES_SEC=1)
 obj-$(CONFIG_QCOM_SCM) += scm.o scm-boot.o

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

new file mode 100644
index 0000000..ee2e3ca
@@ -0,0 +1,334 @@
+/* Copyright (c) 2011-2014, The Linux Foundation. All rights reserved.
+ *
+ * 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/kernel.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
+
+#include <asm/proc-fns.h>
+#include <asm/suspend.h>
+

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

+#define MAX_PMIC_DATA 2
+#define MAX_SEQ_DATA 64
+
+#define SPM_CTL_INDEX 0x7f
+#define SPM_CTL_INDEX_SHIFT 4
+#define SPM_CTL_EN BIT(0)
+
+enum spm_reg {
+	SPM_REG_CFG,
+	SPM_REG_SPM_CTL,
+	SPM_REG_DLY,
+	SPM_REG_PMIC_DLY,
+	SPM_REG_PMIC_DATA_0,

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

+	u8 seq[MAX_SEQ_DATA];
+	u8 start_index[PM_SLEEP_MODE_NR];
+
+};
+
+struct spm_driver_data {
+	void __iomem *reg_base;
+	const struct spm_reg_data *reg_data;
+	bool available;
+};
+
+static const u8 spm_reg_offset_v2_1[SPM_REG_NR] = {
+	[SPM_REG_CFG]		= 0x08,
+	[SPM_REG_SPM_CTL]	= 0x30,
+	[SPM_REG_DLY]		= 0x34,
+	[SPM_REG_SEQ_ENTRY]	= 0x80,
+};
+
+/* SPM register data for 8974, 8084 */
+static const struct spm_reg_data spm_reg_8974_8084_cpu  = {
+	.reg_offset = spm_reg_offset_v2_1,
+	.spm_cfg = 0x1,
+	.spm_dly = 0x3C102800,
+	.seq = { 0x03, 0x0B, 0x0F, 0x00, 0x20, 0x80, 0x10, 0xE8, 0x5B, 0x03,
+		0x3B, 0xE8, 0x5B, 0x82, 0x10, 0x0B, 0x30, 0x06, 0x26, 0x30,
+		0x0F },
+	.start_index[PM_SLEEP_MODE_STBY] = 0,
+	.start_index[PM_SLEEP_MODE_SPC] = 3,
+};
+
+static const u8 spm_reg_offset_v1_1[SPM_REG_NR] = {
+	[SPM_REG_CFG]		= 0x08,
+	[SPM_REG_SPM_CTL]	= 0x20,
+	[SPM_REG_PMIC_DLY]	= 0x24,
+	[SPM_REG_PMIC_DATA_0]	= 0x28,
+	[SPM_REG_PMIC_DATA_1]	= 0x2C,
+	[SPM_REG_SEQ_ENTRY]	= 0x80,
+};
+
+/* SPM register data for 8064 */
+static const struct spm_reg_data spm_reg_8064_cpu = {
+	.reg_offset = spm_reg_offset_v1_1,
+	.spm_cfg = 0x1F,
+	.pmic_dly = 0x02020004,
+	.pmic_data[0] = 0x0084009C,
+	.pmic_data[1] = 0x00A4001C,
+	.seq = { 0x03, 0x0F, 0x00, 0x24, 0x54, 0x10, 0x09, 0x03, 0x01,
+		0x10, 0x54, 0x30, 0x0C, 0x24, 0x30, 0x0F },
+	.start_index[PM_SLEEP_MODE_STBY] = 0,
+	.start_index[PM_SLEEP_MODE_SPC] = 2,
+};
+
+static DEFINE_PER_CPU_SHARED_ALIGNED(struct spm_driver_data, cpu_spm_drv);
+
+static inline void spm_register_write(struct spm_driver_data *drv,
+					enum spm_reg reg, u32 val)
+{
+	if (drv->reg_data->reg_offset[reg])
+		writel_relaxed(val, drv->reg_base +
+				drv->reg_data->reg_offset[reg]);
+}
+
+static inline u32 spm_register_read(struct spm_driver_data *drv,
+					enum spm_reg reg)
+{
+	return readl_relaxed(drv->reg_base + drv->reg_data->reg_offset[reg]);
+}
+
+/**
+ * spm_set_low_power_mode() - Configure SPM start address for low power mode
+ * @mode: SPM LPM mode to enter
+ */
+int qcom_spm_set_low_power_mode(enum pm_sleep_mode mode)
+{
+	struct spm_driver_data *drv = this_cpu_ptr(&cpu_spm_drv);
+	u32 start_index;
+	u32 ctl_val;
+
+	if (!drv->available)
+		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);

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

+static int qcom_pm_collapse(unsigned long int unused)
+{
+	int ret;
+	u32 flag;
+	int cpu = smp_processor_id();
+

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

+	 *  Returns here only if there was a pending interrupt and we did not
+	 *  power down as a result.
+	 */
+	return 0;
+}
+
+static int qcom_cpu_standby(void *unused)
+{
+	int ret;
+
+	ret = qcom_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;
+
+	ret = qcom_spm_set_low_power_mode(PM_SLEEP_MODE_STBY);
+	if (ret)
+		return ret;
+
+	cpu_pm_enter();
+	cpu_suspend(0, qcom_pm_collapse);
+	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;
+
+	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)
+				drv = &per_cpu(cpu_spm_drv, cpu);
+			of_node_put(saw_node);
+		}
+		of_node_put(cpu_node);
+		if (drv) {
+			*spm_cpu = cpu;
+			break;
+		}
+	}
+
+	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 struct qcom_cpu_pm_ops lpm_ops = {
+	.standby = qcom_cpu_standby,
+	.spc = qcom_cpu_spc,
+};
+
+struct platform_device qcom_cpuidle_device = {
+	.name              = "qcom_cpuidle",
+	.id                = -1,
+	.dev.platform_data = &lpm_ops,
+};
+
+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;
+	const u32 *seq_data;
+	int cpu = -EINVAL;
+	static bool cpuidle_drv_init;
+
+	drv = spm_get_drv(pdev, &cpu);
+	if (!drv)
+		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);

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

+	 * 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,

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

+	if ((cpu > -1) && !cpuidle_drv_init) {
+		platform_device_register(&qcom_cpuidle_device);
+		cpuidle_drv_init = true;
+	}
+
+	return 0;
+}
+
+static struct platform_driver spm_driver = {
+	.probe = spm_dev_probe,
+	.driver = {
+		.name = "qcom,spm",
+		.of_match_table = spm_match_table,
+	},
+};
+
+module_platform_driver(spm_driver);

include/soc/qcom/pm.h new file +30 -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 */

Oops, I thought I had sent this, but it was sitting in the drafts folder. Sending anyways because it looks like most of these issues still exist in v10.

Quoted 14 lines

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

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

drivers/soc/qcom/spm.c line 316

Quoted 5 lines

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

+	 * Ensure all observers see the above register writes before the
+	 * cpuidle driver is allowed to use the SPM.
+	 */
+	wmb();
+	drv->available = true;

Others have already commented on this, but I'll add my $0.02 that this suggest something is not right in the init sequence.

Quoted 8 lines

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

+	if ((cpu > -1) && !cpuidle_drv_init) {
+		platform_device_register(&qcom_cpuidle_device);
+		cpuidle_drv_init = true;
+	}
+
+	return 0;
+}
+

Add cpuidle driver interface to allow cpus to go into C-States. Use the cpuidle DT interface, common across ARM architectures, to provide the C-State information to the cpuidle framework.

Supported modes at this time are Standby and Standalone Power Collapse.

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

---

drivers/cpuidle/Kconfig.arm +3 -0

+	select DT_IDLE_STATES
+	help
+	  Select this to enable cpuidle for QCOM processors

drivers/cpuidle/Makefile +1 -0

index 4d177b9..6c222d5 100644
@@ -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

drivers/cpuidle/cpuidle-qcom.c new file +17 -0

new file mode 100644
index 0000000..1c1dcbc
@@ -0,0 +1,72 @@
+/*
+ * 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/cpuidle.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>

drivers/cpuidle/cpuidle-qcom.c new file +36 -0

+	return index;
+}
+
+static struct cpuidle_driver qcom_cpuidle_driver = {
+	.name	= "qcom_cpuidle",
+};
+
+static const struct of_device_id qcom_idle_state_match[] = {
+	{ .compatible = "qcom,idle-state-stby", .data = qcom_cpu_stby},
+	{ .compatible = "qcom,idle-state-spc", .data = qcom_cpu_spc },
+	{ },
+};
+
+static int qcom_cpuidle_probe(struct platform_device *pdev)
+{
+	struct cpuidle_driver *drv = &qcom_cpuidle_driver;
+	int ret;
+
+	lpm_ops = pdev->dev.platform_data;
+
+	/* Probe for other states, including standby */
+	ret = dt_init_idle_driver(drv, qcom_idle_state_match, 0);
+	if (ret < 0)
+		return ret;
+
+	return cpuidle_register(drv, NULL);
+}
+
+static struct platform_driver qcom_cpuidle_plat_driver = {
+	.probe	= qcom_cpuidle_probe,
+	.driver = {
+		.name = "qcom_cpuidle",
+	},
+};
+
+module_platform_driver(qcom_cpuidle_plat_driver);

Quoted 7 lines · Lina Iyer

Add cpuidle driver interface to allow cpus to go into C-States. Use the cpuidle DT interface, common across ARM architectures, to provide the C-State information to the cpuidle framework.

Supported modes at this time are Standby and Standalone Power Collapse.

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

A few comments below, otherwise it looks good to me.

Quoted 1 line

---

[ ... ]

drivers/cpuidle/Kconfig.arm config ARM_MVEBU_V7_CPUIDLE · line 69

Quoted 13 lines

+[1]. Documentation/devicetree/bindings/arm/idle-states.txt

drivers/cpuidle/Kconfig.arm +4 -0

index 8c16ab2..13c7c1f 100644
@@ -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

Could you stick to the ARCH dependency as the other drivers ?

drivers/cpuidle/Kconfig.arm config ARM_MVEBU_V7_CPUIDLE

Quoted 6 lines

drivers/cpuidle/Kconfig.arm +6 -0

+static struct qcom_cpu_pm_ops *lpm_ops;
+
+static int qcom_cpu_stby(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv, int index)
+{
+	lpm_ops->standby(NULL);

No check ?

drivers/cpuidle/Kconfig.arm

Quoted 7 lines

drivers/cpuidle/Kconfig.arm +7 -0

+	return index;
+}
+
+static int qcom_cpu_spc(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv, int index)
+{
+	lpm_ops->spc(NULL);

No check ?

Quoted 37 lines

drivers/cpuidle/Kconfig.arm +36 -0

+	return index;
+}
+
+static struct cpuidle_driver qcom_cpuidle_driver = {
+	.name	= "qcom_cpuidle",
+};
+
+static const struct of_device_id qcom_idle_state_match[] = {
+	{ .compatible = "qcom,idle-state-stby", .data = qcom_cpu_stby},
+	{ .compatible = "qcom,idle-state-spc", .data = qcom_cpu_spc },
+	{ },
+};
+
+static int qcom_cpuidle_probe(struct platform_device *pdev)
+{
+	struct cpuidle_driver *drv = &qcom_cpuidle_driver;
+	int ret;
+
+	lpm_ops = pdev->dev.platform_data;
+
+	/* Probe for other states, including standby */
+	ret = dt_init_idle_driver(drv, qcom_idle_state_match, 0);
+	if (ret < 0)
+		return ret;
+
+	return cpuidle_register(drv, NULL);
+}
+
+static struct platform_driver qcom_cpuidle_plat_driver = {
+	.probe	= qcom_cpuidle_probe,
+	.driver = {
+		.name = "qcom_cpuidle",
+	},
+};
+
+module_platform_driver(qcom_cpuidle_plat_driver);

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

Add cpuidle driver interface to allow cpus to go into C-States. Use the cpuidle DT interface, common across ARM architectures, to provide the C-State information to the cpuidle framework.

Supported modes at this time are Standby and Standalone Power Collapse.

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

---
 .../bindings/arm/msm/qcom,idle-state.txt           | 81 ++++++++++++++++++++++
 drivers/cpuidle/Kconfig.arm                        |  7 ++
 drivers/cpuidle/Makefile                           |  1 +
 drivers/cpuidle/cpuidle-qcom.c                     | 72 +++++++++++++++++++
 4 files changed, 161 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt
 create mode 100644 drivers/cpuidle/cpuidle-qcom.c

Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt new file +81 -0

new file mode 100644
index 0000000..ae1b07f
@@ -0,0 +1,81 @@
+QCOM Idle States for cpuidle driver
+
+ARM provides idle-state node to define the cpuidle states, as defined in [1].
+cpuidle-qcom is the cpuidle driver for Qualcomm SoCs and uses these idle
+states. Idle states have different enter/exit latency and residency values.
+The idle states supported by the QCOM SoC are defined as -
+
+    * Standby
+    * Retention
+    * Standalone Power Collapse (Standalone PC or SPC)
+    * Power Collapse (PC)
+
+Standby: Standby does a little more in addition to architectural clock gating.
+When the WFI instruction is executed the ARM core would gate its internal
+clocks. In addition to gating the clocks, QCOM cpus use this instruction as a
+trigger to execute the SPM state machine. The SPM state machine waits for the
+interrupt to trigger the core back in to active. This triggers the cache
+hierarchy to enter standby states, when all cpus are idle. An interrupt brings
+the SPM state machine out of its wait, the next step is to ensure that the
+cache hierarchy is also out of standby, and then the cpu is allowed to resume
+execution.
+
+Retention: Retention is a low power state where the core is clock gated and
+the memory and the registers associated with the core are retained. The
+voltage may be reduced to the minimum value needed to keep the processor
+registers active. The SPM should be configured to execute the retention
+sequence and would wait for interrupt, before restoring the cpu to execution
+state. Retention may have a slightly higher latency than Standby.
+
+Standalone PC: A cpu can power down and warmboot if there is a sufficient time
+between the time it enters idle and the next known wake up. SPC mode is used
+to indicate a core entering a power down state without consulting any other
+cpu or the system resources. This helps save power only on that core.  The SPM
+sequence for this idle state is programmed to power down the supply to the
+core, wait for the interrupt, restore power to the core, and ensure the
+system state including cache hierarchy is ready before allowing core to
+resume. Applying power and resetting the core causes the core to warmboot
+back into Elevation Level (EL) which trampolines the control back to the
+kernel. Entering a power down state for the cpu, needs to be done by trapping
+into a EL. Failing to do so, would result in a crash enforced by the warm boot
+code in the EL for the SoC. On SoCs with write-back L1 cache, the cache has to
+be flushed in s/w, before powering down the core.
+
+Power Collapse: This state is similar to the SPC mode, but distinguishes
+itself in that the cpu acknowledges and permits the SoC to enter deeper sleep
+modes. In a hierarchical power domain SoC, this means L2 and other caches can
+be flushed, system bus, clocks - lowered, and SoC main XO clock gated and
+voltages reduced, provided all cpus enter this state.  Since the span of low
+power modes possible at this state is vast, the exit latency and the residency
+of this low power mode would be considered high even though at a cpu level,
+this essentially is cpu power down. The SPM in this state also may handshake
+with the Resource power manager processor in the SoC to indicate a complete
+application processor subsystem shut down.
+
+The idle-state for QCOM SoCs are distinguished by the compatible property of
+the idle-states device node.
+The devicetree representation of the idle state should be -
+
+Required properties:
+
+- compatible: Must be one of -
+			"qcom,idle-state-stby",
+			"qcom,idle-state-ret",
+			"qcom,idle-state-spc",
+			"qcom,idle-state-pc",
+		and "arm,idle-state".
+
+Other required and optional properties are specified in [1].
+
+Example:
+
+	idle-states {
+		CPU_SPC: spc {
+			compatible = "qcom,idle-state-spc", "arm,idle-state";
+			entry-latency-us = <150>;
+			exit-latency-us = <200>;
+			min-residency-us = <2000>;
+		};
+	};
+
+[1]. Documentation/devicetree/bindings/arm/idle-states.txt

drivers/cpuidle/Kconfig.arm +7 -0

index 8c16ab2..13c7c1f 100644
@@ -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

drivers/cpuidle/Makefile +1 -0

index 4d177b9..6c222d5 100644
@@ -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

drivers/cpuidle/cpuidle-qcom.c new file +29 -0

new file mode 100644
index 0000000..1c1dcbc
@@ -0,0 +1,72 @@
+/*
+ * 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/cpuidle.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include <soc/qcom/pm.h>
+#include "dt_idle_states.h"
+
+static struct qcom_cpu_pm_ops *lpm_ops;
+
+static int qcom_cpu_stby(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv, int index)
+{
+	lpm_ops->standby(NULL);
+
+	return index;

drivers/cpuidle/cpuidle-qcom.c new file +9 -0

+static struct cpuidle_driver qcom_cpuidle_driver = {
+	.name	= "qcom_cpuidle",
+};
+
+static const struct of_device_id qcom_idle_state_match[] = {
+	{ .compatible = "qcom,idle-state-stby", .data = qcom_cpu_stby},
+	{ .compatible = "qcom,idle-state-spc", .data = qcom_cpu_spc },
+	{ },
+};

Quoted 3 lines · Lina Iyer

Add cpuidle driver interface to allow cpus to go into C-States. Use the cpuidle DT interface, common across ARM architectures, to provide the C-State information to the cpuidle framework.

"idle states", this is not ACPI.

Quoted 4 lines

Supported modes at this time are Standby and Standalone Power Collapse.

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

---

+static struct qcom_cpu_pm_ops *lpm_ops;

Quoted 7 lines

drivers/cpuidle/cpuidle-qcom.c new file +7 -0

+
+static int qcom_cpu_stby(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv, int index)
+{
+	lpm_ops->standby(NULL);
+
+	return index;

drivers/cpuidle/cpuidle-qcom.c line 39

Quoted 10 lines

drivers/cpuidle/cpuidle-qcom.c new file +10 -0

+}
+
+static int qcom_cpu_spc(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv, int index)
+{
+	lpm_ops->spc(NULL);
+
+	return index;
+}
+

I can't have a look at this and avoid thinking that this should look something like:

static qcom_cpu_idle(...., int index)

{
	lpm_ops[index].enter_idle(...);
	return index;
}

Before jumping to conclusions, see below.

Quoted 9 lines

drivers/cpuidle/cpuidle-qcom.c new file +9 -0

+static struct cpuidle_driver qcom_cpuidle_driver = {
+	.name	= "qcom_cpuidle",
+};
+
+static const struct of_device_id qcom_idle_state_match[] = {
+	{ .compatible = "qcom,idle-state-stby", .data = qcom_cpu_stby},
+	{ .compatible = "qcom,idle-state-spc", .data = qcom_cpu_spc },
+	{ },
+};

drivers/cpuidle/cpuidle-qcom.c line 58

Quoted 10 lines

drivers/cpuidle/cpuidle-qcom.c new file +10 -0

+
+static int qcom_cpuidle_probe(struct platform_device *pdev)
+{
+	struct cpuidle_driver *drv = &qcom_cpuidle_driver;
+	int ret;
+
+	lpm_ops = pdev->dev.platform_data;
+
+	/* Probe for other states, including standby */
+	ret = dt_init_idle_driver(drv, qcom_idle_state_match, 0);

This driver will be DT only. If an idle state is parsed correctly, it is initialized, otherwise it is skipped. Now, if we added glue code in arch arm (as arm64 does) that allows us to link an idle state index with the functions above (a DT idle state contains all information required to initialize its enter function, more so now that we are adding power domains to the picture), what would be the issue in defining a common API that just passes the index to the arch back-end ? No pointers to pass, no platform drivers required and still no arch/soc code in drivers/cpuidle.

I am obviously talking about DT CPUidle drivers only.

If the idle state is parsed correctly and the backend initializer (let's call it arm_cpu_init_idle(cpu)) is successful (which means that DT idle states contain valid information and the enter functions could be initialized from DT properly) I do not see what's the problem, give it some thought.

Anyway, I will put together an RFC to start the discussion when patch is merged and patch the relevant code as an example, you can go ahead with current code, I am reviewing it.

Lorenzo

Changes since v8:
[ https://www.mail-archive.com/[email protected]/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.

Hi Lina,

Quoted 11 lines · Lina Iyer

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.

And this is based on...? Please, could you be explicit what kind of patches are needed before applying this patch set. Is seems that they don't depend just on "Lorenzo's ARM idle-states patches", I see patches from Stephen Boyd, Olav Haugan and Vikram Mulukutla in your tree.

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 (9):
qcom: scm: scm_set_warm_boot_addr() to set the warmboot address
qcom: spm: Add Subsystem Power Manager driver
arm: dts: qcom: Add power-controller device node for 8974 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 8974
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                |  37 ++-
arch/arm/boot/dts/qcom-apq8084.dtsi                |  46 ++-
arch/arm/boot/dts/qcom-msm8974.dtsi                |  46 ++-
drivers/cpuidle/Kconfig.arm                        |   7 +
drivers/cpuidle/Makefile                           |   1 +
drivers/cpuidle/cpuidle-qcom.c                     |  72 +++++
drivers/soc/qcom/Kconfig                           |   8 +
drivers/soc/qcom/Makefile                          |   1 +
drivers/soc/qcom/scm-boot.c                        |  22 ++
drivers/soc/qcom/spm.c                             | 334 +++++++++++++++++++++
include/soc/qcom/pm.h                              |  31 ++
include/soc/qcom/scm-boot.h                        |   1 +
14 files changed, 702 insertions(+), 16 deletions(-)

create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt create mode 100644 drivers/cpuidle/cpuidle-qcom.c
create mode 100644 drivers/soc/qcom/spm.c
create mode 100644 include/soc/qcom/pm.h

Signature · 1 line

2.1.0