* [PATCH v14 00/10] cpuidle driver for QCOM SoCs: 8064, 8074, 8084
@ 2014-12-02 17:39 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: linux-arm-kernel
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 v13:
- Return values for idle states propagated back to the cpuidle driver.
- Remove static bool cpuidle_drv_init.
- Register cpuidle driver and cpuidle device separately.
- cpuidle device registered only when the SPM for the cpu is probed.
- Initialization changes to ensure dynamic cpuidle devices are registered only
after the cpuidle driver is registered.
- Removed wmb, replaced with a poll loop to ensure that the SPM registers are
written before executing wfi.
- Added spm_register_write_sync() for write guarantees.
- Removed irrelevant return value for spm_set_low_power_mode().
- Added comments, updated module description.
- Removed Reviewed-by and Acked-by on the spm and cpuidle-qcom patches.
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 | 38 ++-
arch/arm/boot/dts/qcom-apq8084.dtsi | 48 ++-
arch/arm/boot/dts/qcom-msm8974.dtsi | 48 ++-
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 | 99 ++++++
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 | 338 +++++++++++++++++++++
include/soc/qcom/pm.h | 31 ++
.../arm/mach-qcom => include/soc/qcom}/scm-boot.h | 3 +-
16 files changed, 754 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%)
--
2.1.0
^ permalink raw reply [flat|nested] 62+ messages in thread* [PATCH v14 01/10] qcom: scm: Move scm-boot files to drivers/soc/qcom/ and include/soc/qcom
2014-12-02 17:39 ` Lina Iyer
@ 2014-12-02 17:39 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: daniel.lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel
Cc: lorenzo.pieralisi, msivasub, devicetree, Lina Iyer
Follow the scm.c and move scm-boot files to drivers/soc/qcom. The
guidance is to clean files out from mach-qcom and move to drivers/soc
area.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/mach-qcom/Makefile | 1 -
arch/arm/mach-qcom/platsmp.c | 2 +-
drivers/soc/qcom/Makefile | 2 +-
{arch/arm/mach-qcom => drivers/soc/qcom}/scm-boot.c | 4 ++--
{arch/arm/mach-qcom => include/soc/qcom}/scm-boot.h | 0
5 files changed, 4 insertions(+), 5 deletions(-)
rename {arch/arm/mach-qcom => drivers/soc/qcom}/scm-boot.c (97%)
rename {arch/arm/mach-qcom => include/soc/qcom}/scm-boot.h (100%)
diff --git a/arch/arm/mach-qcom/Makefile b/arch/arm/mach-qcom/Makefile
index db41e8c..e324375 100644
--- a/arch/arm/mach-qcom/Makefile
+++ b/arch/arm/mach-qcom/Makefile
@@ -1,3 +1,2 @@
obj-y := board.o
obj-$(CONFIG_SMP) += platsmp.o
-obj-$(CONFIG_QCOM_SCM) += scm-boot.o
diff --git a/arch/arm/mach-qcom/platsmp.c b/arch/arm/mach-qcom/platsmp.c
index d690856..a692bcb 100644
--- a/arch/arm/mach-qcom/platsmp.c
+++ b/arch/arm/mach-qcom/platsmp.c
@@ -20,7 +20,7 @@
#include <asm/smp_plat.h>
-#include "scm-boot.h"
+#include <soc/qcom/scm-boot.h>
#define VDD_SC1_ARRAY_CLAMP_GFS_CTL 0x35a0
#define SCSS_CPU1CORE_RESET 0x2d80
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index a39446d..70d52ed 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -1,3 +1,3 @@
obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
CFLAGS_scm.o :=$(call as-instr,.arch_extension sec,-DREQUIRES_SEC=1)
-obj-$(CONFIG_QCOM_SCM) += scm.o
+obj-$(CONFIG_QCOM_SCM) += scm.o scm-boot.o
diff --git a/arch/arm/mach-qcom/scm-boot.c b/drivers/soc/qcom/scm-boot.c
similarity index 97%
rename from arch/arm/mach-qcom/scm-boot.c
rename to drivers/soc/qcom/scm-boot.c
index 5add20e..60ff7b4 100644
--- a/arch/arm/mach-qcom/scm-boot.c
+++ b/drivers/soc/qcom/scm-boot.c
@@ -17,9 +17,9 @@
#include <linux/module.h>
#include <linux/slab.h>
-#include <soc/qcom/scm.h>
-#include "scm-boot.h"
+#include <soc/qcom/scm.h>
+#include <soc/qcom/scm-boot.h>
/*
* Set the cold/warm boot address for one of the CPU cores.
diff --git a/arch/arm/mach-qcom/scm-boot.h b/include/soc/qcom/scm-boot.h
similarity index 100%
rename from arch/arm/mach-qcom/scm-boot.h
rename to include/soc/qcom/scm-boot.h
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread* [PATCH v14 01/10] qcom: scm: Move scm-boot files to drivers/soc/qcom/ and include/soc/qcom
@ 2014-12-02 17:39 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: linux-arm-kernel
Follow the scm.c and move scm-boot files to drivers/soc/qcom. The
guidance is to clean files out from mach-qcom and move to drivers/soc
area.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/mach-qcom/Makefile | 1 -
arch/arm/mach-qcom/platsmp.c | 2 +-
drivers/soc/qcom/Makefile | 2 +-
{arch/arm/mach-qcom => drivers/soc/qcom}/scm-boot.c | 4 ++--
{arch/arm/mach-qcom => include/soc/qcom}/scm-boot.h | 0
5 files changed, 4 insertions(+), 5 deletions(-)
rename {arch/arm/mach-qcom => drivers/soc/qcom}/scm-boot.c (97%)
rename {arch/arm/mach-qcom => include/soc/qcom}/scm-boot.h (100%)
diff --git a/arch/arm/mach-qcom/Makefile b/arch/arm/mach-qcom/Makefile
index db41e8c..e324375 100644
--- a/arch/arm/mach-qcom/Makefile
+++ b/arch/arm/mach-qcom/Makefile
@@ -1,3 +1,2 @@
obj-y := board.o
obj-$(CONFIG_SMP) += platsmp.o
-obj-$(CONFIG_QCOM_SCM) += scm-boot.o
diff --git a/arch/arm/mach-qcom/platsmp.c b/arch/arm/mach-qcom/platsmp.c
index d690856..a692bcb 100644
--- a/arch/arm/mach-qcom/platsmp.c
+++ b/arch/arm/mach-qcom/platsmp.c
@@ -20,7 +20,7 @@
#include <asm/smp_plat.h>
-#include "scm-boot.h"
+#include <soc/qcom/scm-boot.h>
#define VDD_SC1_ARRAY_CLAMP_GFS_CTL 0x35a0
#define SCSS_CPU1CORE_RESET 0x2d80
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index a39446d..70d52ed 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -1,3 +1,3 @@
obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
CFLAGS_scm.o :=$(call as-instr,.arch_extension sec,-DREQUIRES_SEC=1)
-obj-$(CONFIG_QCOM_SCM) += scm.o
+obj-$(CONFIG_QCOM_SCM) += scm.o scm-boot.o
diff --git a/arch/arm/mach-qcom/scm-boot.c b/drivers/soc/qcom/scm-boot.c
similarity index 97%
rename from arch/arm/mach-qcom/scm-boot.c
rename to drivers/soc/qcom/scm-boot.c
index 5add20e..60ff7b4 100644
--- a/arch/arm/mach-qcom/scm-boot.c
+++ b/drivers/soc/qcom/scm-boot.c
@@ -17,9 +17,9 @@
#include <linux/module.h>
#include <linux/slab.h>
-#include <soc/qcom/scm.h>
-#include "scm-boot.h"
+#include <soc/qcom/scm.h>
+#include <soc/qcom/scm-boot.h>
/*
* Set the cold/warm boot address for one of the CPU cores.
diff --git a/arch/arm/mach-qcom/scm-boot.h b/include/soc/qcom/scm-boot.h
similarity index 100%
rename from arch/arm/mach-qcom/scm-boot.h
rename to include/soc/qcom/scm-boot.h
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH v14 02/10] qcom: scm: Add SCM warmboot support for quad core SoCs
2014-12-02 17:39 ` Lina Iyer
@ 2014-12-02 17:39 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: daniel.lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel
Cc: lorenzo.pieralisi, msivasub, devicetree, Lina Iyer
Quad core SoCs like APQ8074, APQ8064, APQ8084 need SCM support set up
warm boot addresses in the Secure Monitor. Extend the SCM flags to
support warm boot addresses for secondary cores.
We do not need to export the warmboot flags. Move them into the
implementation file.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/soc/qcom/scm-boot.c | 35 +++++++++++++++++++++++++++++++++++
include/soc/qcom/scm-boot.h | 3 +--
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/qcom/scm-boot.c b/drivers/soc/qcom/scm-boot.c
index 60ff7b4..f653217a 100644
--- a/drivers/soc/qcom/scm-boot.c
+++ b/drivers/soc/qcom/scm-boot.c
@@ -21,6 +21,23 @@
#include <soc/qcom/scm.h>
#include <soc/qcom/scm-boot.h>
+#define SCM_FLAG_WARMBOOT_CPU0 0x04
+#define SCM_FLAG_WARMBOOT_CPU1 0x02
+#define SCM_FLAG_WARMBOOT_CPU2 0x10
+#define SCM_FLAG_WARMBOOT_CPU3 0x40
+
+struct scm_warmboot {
+ int flag;
+ void *entry;
+};
+
+static struct scm_warmboot scm_flags[] = {
+ { .flag = SCM_FLAG_WARMBOOT_CPU0 },
+ { .flag = SCM_FLAG_WARMBOOT_CPU1 },
+ { .flag = SCM_FLAG_WARMBOOT_CPU2 },
+ { .flag = SCM_FLAG_WARMBOOT_CPU3 },
+};
+
/*
* Set the cold/warm boot address for one of the CPU cores.
*/
@@ -37,3 +54,21 @@ int scm_set_boot_addr(phys_addr_t addr, int flags)
&cmd, sizeof(cmd), NULL, 0);
}
EXPORT_SYMBOL(scm_set_boot_addr);
+
+int scm_set_warm_boot_addr(void *entry, int cpu)
+{
+ int ret;
+
+ /*
+ * Reassign only if we are switching from hotplug entry point
+ * to cpuidle entry point or vice versa.
+ */
+ if (entry == scm_flags[cpu].entry)
+ return 0;
+
+ ret = scm_set_boot_addr(virt_to_phys(entry), scm_flags[cpu].flag);
+ if (!ret)
+ scm_flags[cpu].entry = entry;
+
+ return ret;
+}
diff --git a/include/soc/qcom/scm-boot.h b/include/soc/qcom/scm-boot.h
index 6aabb24..529f55a 100644
--- a/include/soc/qcom/scm-boot.h
+++ b/include/soc/qcom/scm-boot.h
@@ -16,9 +16,8 @@
#define SCM_FLAG_COLDBOOT_CPU1 0x01
#define SCM_FLAG_COLDBOOT_CPU2 0x08
#define SCM_FLAG_COLDBOOT_CPU3 0x20
-#define SCM_FLAG_WARMBOOT_CPU0 0x04
-#define SCM_FLAG_WARMBOOT_CPU1 0x02
int scm_set_boot_addr(phys_addr_t addr, int flags);
+int scm_set_warm_boot_addr(void *entry, int cpu);
#endif
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread* [PATCH v14 02/10] qcom: scm: Add SCM warmboot support for quad core SoCs
@ 2014-12-02 17:39 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: linux-arm-kernel
Quad core SoCs like APQ8074, APQ8064, APQ8084 need SCM support set up
warm boot addresses in the Secure Monitor. Extend the SCM flags to
support warm boot addresses for secondary cores.
We do not need to export the warmboot flags. Move them into the
implementation file.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/soc/qcom/scm-boot.c | 35 +++++++++++++++++++++++++++++++++++
include/soc/qcom/scm-boot.h | 3 +--
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/drivers/soc/qcom/scm-boot.c b/drivers/soc/qcom/scm-boot.c
index 60ff7b4..f653217a 100644
--- a/drivers/soc/qcom/scm-boot.c
+++ b/drivers/soc/qcom/scm-boot.c
@@ -21,6 +21,23 @@
#include <soc/qcom/scm.h>
#include <soc/qcom/scm-boot.h>
+#define SCM_FLAG_WARMBOOT_CPU0 0x04
+#define SCM_FLAG_WARMBOOT_CPU1 0x02
+#define SCM_FLAG_WARMBOOT_CPU2 0x10
+#define SCM_FLAG_WARMBOOT_CPU3 0x40
+
+struct scm_warmboot {
+ int flag;
+ void *entry;
+};
+
+static struct scm_warmboot scm_flags[] = {
+ { .flag = SCM_FLAG_WARMBOOT_CPU0 },
+ { .flag = SCM_FLAG_WARMBOOT_CPU1 },
+ { .flag = SCM_FLAG_WARMBOOT_CPU2 },
+ { .flag = SCM_FLAG_WARMBOOT_CPU3 },
+};
+
/*
* Set the cold/warm boot address for one of the CPU cores.
*/
@@ -37,3 +54,21 @@ int scm_set_boot_addr(phys_addr_t addr, int flags)
&cmd, sizeof(cmd), NULL, 0);
}
EXPORT_SYMBOL(scm_set_boot_addr);
+
+int scm_set_warm_boot_addr(void *entry, int cpu)
+{
+ int ret;
+
+ /*
+ * Reassign only if we are switching from hotplug entry point
+ * to cpuidle entry point or vice versa.
+ */
+ if (entry == scm_flags[cpu].entry)
+ return 0;
+
+ ret = scm_set_boot_addr(virt_to_phys(entry), scm_flags[cpu].flag);
+ if (!ret)
+ scm_flags[cpu].entry = entry;
+
+ return ret;
+}
diff --git a/include/soc/qcom/scm-boot.h b/include/soc/qcom/scm-boot.h
index 6aabb24..529f55a 100644
--- a/include/soc/qcom/scm-boot.h
+++ b/include/soc/qcom/scm-boot.h
@@ -16,9 +16,8 @@
#define SCM_FLAG_COLDBOOT_CPU1 0x01
#define SCM_FLAG_COLDBOOT_CPU2 0x08
#define SCM_FLAG_COLDBOOT_CPU3 0x20
-#define SCM_FLAG_WARMBOOT_CPU0 0x04
-#define SCM_FLAG_WARMBOOT_CPU1 0x02
int scm_set_boot_addr(phys_addr_t addr, int flags);
+int scm_set_warm_boot_addr(void *entry, int cpu);
#endif
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-02 17:39 ` Lina Iyer
@ 2014-12-02 17:39 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: daniel.lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel
Cc: lorenzo.pieralisi, msivasub, devicetree, Lina Iyer
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 <msivasub@codeaurora.org>,
Ai Li <ali@codeaurora.org>, Praveen Chidambaram <pchidamb@codeaurora.org>
Original tree available at -
git://codeaurora.org/quic/la/kernel/msm-3.10.git
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
.../devicetree/bindings/arm/msm/qcom,saw2.txt | 31 +-
drivers/soc/qcom/Kconfig | 8 +
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/spm.c | 338 +++++++++++++++++++++
include/soc/qcom/pm.h | 31 ++
5 files changed, 403 insertions(+), 6 deletions(-)
create mode 100644 drivers/soc/qcom/spm.c
create mode 100644 include/soc/qcom/pm.h
diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
index 1505fb8..690c3c0 100644
--- a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
+++ b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
@@ -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@2099000 {
+ power-controller@2099000 {
compatible = "qcom,saw2";
reg = <0x02099000 0x1000>, <0x02009000 0x1000>;
+ regulator;
};
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 7dcd554..012fb37 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -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.
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 70d52ed..20b329f 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -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
diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c
new file mode 100644
index 0000000..90812c5
--- /dev/null
+++ b/drivers/soc/qcom/spm.c
@@ -0,0 +1,338 @@
+/*
+ * 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/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 {
+ const u8 *reg_offset;
+ u32 spm_cfg;
+ u32 spm_dly;
+ u32 pmic_dly;
+ u32 pmic_data[MAX_PMIC_DATA];
+ 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;
+};
+
+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]);
+}
+
+/* Ensure a guaranteed write, before return */
+static inline void spm_register_write_sync(struct spm_driver_data *drv,
+ enum spm_reg reg, u32 val)
+{
+ u32 ret;
+
+ if (!drv->reg_data->reg_offset[reg])
+ return;
+
+ do {
+ writel_relaxed(val, drv->reg_base +
+ drv->reg_data->reg_offset[reg]);
+ ret = readl_relaxed(drv->reg_base +
+ drv->reg_data->reg_offset[reg]);
+ if (ret == val)
+ break;
+ cpu_relax();
+ } while (1);
+}
+
+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]);
+}
+
+static void 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;
+
+ 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_sync(drv, SPM_REG_SPM_CTL, ctl_val);
+}
+
+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;
+ ret = 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 ret;
+}
+
+static int qcom_cpu_standby(void *unused)
+{
+ spm_set_low_power_mode(PM_SLEEP_MODE_STBY);
+ cpu_do_idle();
+
+ return 0;
+}
+
+static int qcom_cpu_spc(void *unused)
+{
+ int ret;
+
+ spm_set_low_power_mode(PM_SLEEP_MODE_STBY);
+ cpu_pm_enter();
+ ret = cpu_suspend(0, qcom_pm_collapse);
+ cpu_pm_exit();
+
+ return ret;
+}
+
+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;
+
+ 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);
+ found = (saw_node == pdev->dev.of_node);
+ 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 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;
+ struct cpuidle_device *dev;
+
+ 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;
+
+ /* 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 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]);
+
+ per_cpu(cpu_spm_drv, cpu) = drv;
+
+ /* Register the cpuidle device for the cpu, we are ready for cpuidle */
+ dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ return -ENOMEM;
+
+ dev->cpu = cpu;
+ return cpuidle_register_device(dev);
+}
+
+static struct qcom_cpu_pm_ops lpm_ops = {
+ .standby = qcom_cpu_standby,
+ .spc = qcom_cpu_spc,
+};
+
+static struct platform_device qcom_cpuidle_drv = {
+ .name = "qcom_cpuidle",
+ .id = -1,
+ .dev.platform_data = &lpm_ops,
+};
+
+static struct platform_driver spm_driver = {
+ .probe = spm_dev_probe,
+ .driver = {
+ .name = "saw",
+ .of_match_table = spm_match_table,
+ },
+};
+
+static int __init qcom_spm_init(void)
+{
+ int ret;
+
+ /*
+ * cpuidle driver need to registered before the cpuidle device
+ * for any cpu. Register the device for the the cpuidle driver.
+ */
+ ret = platform_device_register(&qcom_cpuidle_drv);
+ if (ret)
+ return ret;
+
+ return platform_driver_register(&spm_driver);
+}
+module_init(qcom_spm_init);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("SAW power controller driver");
+MODULE_ALIAS("platform:saw");
diff --git a/include/soc/qcom/pm.h b/include/soc/qcom/pm.h
new file mode 100644
index 0000000..d9a56d7
--- /dev/null
+++ b/include/soc/qcom/pm.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2009-2014, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef __QCOM_PM_H
+#define __QCOM_PM_H
+
+enum pm_sleep_mode {
+ PM_SLEEP_MODE_STBY,
+ PM_SLEEP_MODE_RET,
+ PM_SLEEP_MODE_SPC,
+ PM_SLEEP_MODE_PC,
+ PM_SLEEP_MODE_NR,
+};
+
+struct qcom_cpu_pm_ops {
+ int (*standby)(void *data);
+ int (*spc)(void *data);
+};
+
+#endif /* __QCOM_PM_H */
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-02 17:39 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: linux-arm-kernel
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 <msivasub@codeaurora.org>,
Ai Li <ali@codeaurora.org>, Praveen Chidambaram <pchidamb@codeaurora.org>
Original tree available at -
git://codeaurora.org/quic/la/kernel/msm-3.10.git
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
.../devicetree/bindings/arm/msm/qcom,saw2.txt | 31 +-
drivers/soc/qcom/Kconfig | 8 +
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/spm.c | 338 +++++++++++++++++++++
include/soc/qcom/pm.h | 31 ++
5 files changed, 403 insertions(+), 6 deletions(-)
create mode 100644 drivers/soc/qcom/spm.c
create mode 100644 include/soc/qcom/pm.h
diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
index 1505fb8..690c3c0 100644
--- a/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
+++ b/Documentation/devicetree/bindings/arm/msm/qcom,saw2.txt
@@ -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;
};
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 7dcd554..012fb37 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -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.
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 70d52ed..20b329f 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -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
diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c
new file mode 100644
index 0000000..90812c5
--- /dev/null
+++ b/drivers/soc/qcom/spm.c
@@ -0,0 +1,338 @@
+/*
+ * 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/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 {
+ const u8 *reg_offset;
+ u32 spm_cfg;
+ u32 spm_dly;
+ u32 pmic_dly;
+ u32 pmic_data[MAX_PMIC_DATA];
+ 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;
+};
+
+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]);
+}
+
+/* Ensure a guaranteed write, before return */
+static inline void spm_register_write_sync(struct spm_driver_data *drv,
+ enum spm_reg reg, u32 val)
+{
+ u32 ret;
+
+ if (!drv->reg_data->reg_offset[reg])
+ return;
+
+ do {
+ writel_relaxed(val, drv->reg_base +
+ drv->reg_data->reg_offset[reg]);
+ ret = readl_relaxed(drv->reg_base +
+ drv->reg_data->reg_offset[reg]);
+ if (ret == val)
+ break;
+ cpu_relax();
+ } while (1);
+}
+
+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]);
+}
+
+static void 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;
+
+ 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_sync(drv, SPM_REG_SPM_CTL, ctl_val);
+}
+
+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;
+ ret = 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 ret;
+}
+
+static int qcom_cpu_standby(void *unused)
+{
+ spm_set_low_power_mode(PM_SLEEP_MODE_STBY);
+ cpu_do_idle();
+
+ return 0;
+}
+
+static int qcom_cpu_spc(void *unused)
+{
+ int ret;
+
+ spm_set_low_power_mode(PM_SLEEP_MODE_STBY);
+ cpu_pm_enter();
+ ret = cpu_suspend(0, qcom_pm_collapse);
+ cpu_pm_exit();
+
+ return ret;
+}
+
+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;
+
+ 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);
+ found = (saw_node == pdev->dev.of_node);
+ 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 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;
+ struct cpuidle_device *dev;
+
+ 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;
+
+ /* 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 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]);
+
+ per_cpu(cpu_spm_drv, cpu) = drv;
+
+ /* Register the cpuidle device for the cpu, we are ready for cpuidle */
+ dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ return -ENOMEM;
+
+ dev->cpu = cpu;
+ return cpuidle_register_device(dev);
+}
+
+static struct qcom_cpu_pm_ops lpm_ops = {
+ .standby = qcom_cpu_standby,
+ .spc = qcom_cpu_spc,
+};
+
+static struct platform_device qcom_cpuidle_drv = {
+ .name = "qcom_cpuidle",
+ .id = -1,
+ .dev.platform_data = &lpm_ops,
+};
+
+static struct platform_driver spm_driver = {
+ .probe = spm_dev_probe,
+ .driver = {
+ .name = "saw",
+ .of_match_table = spm_match_table,
+ },
+};
+
+static int __init qcom_spm_init(void)
+{
+ int ret;
+
+ /*
+ * cpuidle driver need to registered before the cpuidle device
+ * for any cpu. Register the device for the the cpuidle driver.
+ */
+ ret = platform_device_register(&qcom_cpuidle_drv);
+ if (ret)
+ return ret;
+
+ return platform_driver_register(&spm_driver);
+}
+module_init(qcom_spm_init);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("SAW power controller driver");
+MODULE_ALIAS("platform:saw");
diff --git a/include/soc/qcom/pm.h b/include/soc/qcom/pm.h
new file mode 100644
index 0000000..d9a56d7
--- /dev/null
+++ b/include/soc/qcom/pm.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2009-2014, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef __QCOM_PM_H
+#define __QCOM_PM_H
+
+enum pm_sleep_mode {
+ PM_SLEEP_MODE_STBY,
+ PM_SLEEP_MODE_RET,
+ PM_SLEEP_MODE_SPC,
+ PM_SLEEP_MODE_PC,
+ PM_SLEEP_MODE_NR,
+};
+
+struct qcom_cpu_pm_ops {
+ int (*standby)(void *data);
+ int (*spc)(void *data);
+};
+
+#endif /* __QCOM_PM_H */
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-02 17:39 ` Lina Iyer
@ 2014-12-02 23:05 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 23:05 UTC (permalink / raw)
To: daniel.lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel
Cc: lorenzo.pieralisi, msivasub, devicetree
On Tue, Dec 02 2014 at 10:40 -0700, Lina Iyer wrote:
+
[...]
>+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;
>+ struct cpuidle_device *dev;
>+
>+ 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;
>+
>+ /* 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 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]);
>+
>+ per_cpu(cpu_spm_drv, cpu) = drv;
>+
>+ /* Register the cpuidle device for the cpu, we are ready for cpuidle */
>+ dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
>+ if (!dev)
>+ return -ENOMEM;
>+
>+ dev->cpu = cpu;
>+ return cpuidle_register_device(dev);
>+}
>+
>+static struct qcom_cpu_pm_ops lpm_ops = {
>+ .standby = qcom_cpu_standby,
>+ .spc = qcom_cpu_spc,
>+};
>+
>+static struct platform_device qcom_cpuidle_drv = {
>+ .name = "qcom_cpuidle",
>+ .id = -1,
>+ .dev.platform_data = &lpm_ops,
>+};
>+
>+static struct platform_driver spm_driver = {
>+ .probe = spm_dev_probe,
>+ .driver = {
>+ .name = "saw",
>+ .of_match_table = spm_match_table,
>+ },
>+};
>+
>+static int __init qcom_spm_init(void)
>+{
>+ int ret;
>+
>+ /*
>+ * cpuidle driver need to registered before the cpuidle device
>+ * for any cpu. Register the device for the the cpuidle driver.
>+ */
>+ ret = platform_device_register(&qcom_cpuidle_drv);
>+ if (ret)
>+ return ret;
Stephen pointed out that we would have the platform device lying around
on a non-QCOM device when using multi_v7_defconfig.
So instead of doing this here, we could do this in the probe..
if (!cpuidle_get_driver()) {
int ret = platform_device_register(&qcom_cpuidle_drv);
if (ret)
return ret;
}
Would that be okay?
The successful probe indicates that we are on a QCOM SoC, and we have not
registered a cpuidle_driver before this.
Thanks,
Lina
>+
>+ return platform_driver_register(&spm_driver);
>+}
>+module_init(qcom_spm_init);
>+
>+MODULE_LICENSE("GPL v2");
>+MODULE_DESCRIPTION("SAW power controller driver");
>+MODULE_ALIAS("platform:saw");
>diff --git a/include/soc/qcom/pm.h b/include/soc/qcom/pm.h
>new file mode 100644
>index 0000000..d9a56d7
>--- /dev/null
>+++ b/include/soc/qcom/pm.h
>@@ -0,0 +1,31 @@
>+/*
>+ * Copyright (c) 2009-2014, The Linux Foundation. All rights reserved.
>+ *
>+ * This software is licensed under the terms of the GNU General Public
>+ * License version 2, as published by the Free Software Foundation, and
>+ * may be copied, distributed, and modified under those terms.
>+ *
>+ * This program is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+ * GNU General Public License for more details.
>+ *
>+ */
>+
>+#ifndef __QCOM_PM_H
>+#define __QCOM_PM_H
>+
>+enum pm_sleep_mode {
>+ PM_SLEEP_MODE_STBY,
>+ PM_SLEEP_MODE_RET,
>+ PM_SLEEP_MODE_SPC,
>+ PM_SLEEP_MODE_PC,
>+ PM_SLEEP_MODE_NR,
>+};
>+
>+struct qcom_cpu_pm_ops {
>+ int (*standby)(void *data);
>+ int (*spc)(void *data);
>+};
>+
>+#endif /* __QCOM_PM_H */
>--
>2.1.0
>
^ permalink raw reply [flat|nested] 62+ messages in thread* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-02 23:05 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 23:05 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Dec 02 2014 at 10:40 -0700, Lina Iyer wrote:
+
[...]
>+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;
>+ struct cpuidle_device *dev;
>+
>+ 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;
>+
>+ /* 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 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]);
>+
>+ per_cpu(cpu_spm_drv, cpu) = drv;
>+
>+ /* Register the cpuidle device for the cpu, we are ready for cpuidle */
>+ dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
>+ if (!dev)
>+ return -ENOMEM;
>+
>+ dev->cpu = cpu;
>+ return cpuidle_register_device(dev);
>+}
>+
>+static struct qcom_cpu_pm_ops lpm_ops = {
>+ .standby = qcom_cpu_standby,
>+ .spc = qcom_cpu_spc,
>+};
>+
>+static struct platform_device qcom_cpuidle_drv = {
>+ .name = "qcom_cpuidle",
>+ .id = -1,
>+ .dev.platform_data = &lpm_ops,
>+};
>+
>+static struct platform_driver spm_driver = {
>+ .probe = spm_dev_probe,
>+ .driver = {
>+ .name = "saw",
>+ .of_match_table = spm_match_table,
>+ },
>+};
>+
>+static int __init qcom_spm_init(void)
>+{
>+ int ret;
>+
>+ /*
>+ * cpuidle driver need to registered before the cpuidle device
>+ * for any cpu. Register the device for the the cpuidle driver.
>+ */
>+ ret = platform_device_register(&qcom_cpuidle_drv);
>+ if (ret)
>+ return ret;
Stephen pointed out that we would have the platform device lying around
on a non-QCOM device when using multi_v7_defconfig.
So instead of doing this here, we could do this in the probe..
if (!cpuidle_get_driver()) {
int ret = platform_device_register(&qcom_cpuidle_drv);
if (ret)
return ret;
}
Would that be okay?
The successful probe indicates that we are on a QCOM SoC, and we have not
registered a cpuidle_driver before this.
Thanks,
Lina
>+
>+ return platform_driver_register(&spm_driver);
>+}
>+module_init(qcom_spm_init);
>+
>+MODULE_LICENSE("GPL v2");
>+MODULE_DESCRIPTION("SAW power controller driver");
>+MODULE_ALIAS("platform:saw");
>diff --git a/include/soc/qcom/pm.h b/include/soc/qcom/pm.h
>new file mode 100644
>index 0000000..d9a56d7
>--- /dev/null
>+++ b/include/soc/qcom/pm.h
>@@ -0,0 +1,31 @@
>+/*
>+ * Copyright (c) 2009-2014, The Linux Foundation. All rights reserved.
>+ *
>+ * This software is licensed under the terms of the GNU General Public
>+ * License version 2, as published by the Free Software Foundation, and
>+ * may be copied, distributed, and modified under those terms.
>+ *
>+ * This program is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+ * GNU General Public License for more details.
>+ *
>+ */
>+
>+#ifndef __QCOM_PM_H
>+#define __QCOM_PM_H
>+
>+enum pm_sleep_mode {
>+ PM_SLEEP_MODE_STBY,
>+ PM_SLEEP_MODE_RET,
>+ PM_SLEEP_MODE_SPC,
>+ PM_SLEEP_MODE_PC,
>+ PM_SLEEP_MODE_NR,
>+};
>+
>+struct qcom_cpu_pm_ops {
>+ int (*standby)(void *data);
>+ int (*spc)(void *data);
>+};
>+
>+#endif /* __QCOM_PM_H */
>--
>2.1.0
>
^ permalink raw reply [flat|nested] 62+ messages in thread* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-02 23:05 ` Lina Iyer
@ 2014-12-03 9:11 ` Daniel Lezcano
-1 siblings, 0 replies; 62+ messages in thread
From: Daniel Lezcano @ 2014-12-03 9:11 UTC (permalink / raw)
To: Lina Iyer, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel
Cc: lorenzo.pieralisi, msivasub, devicetree
On 12/03/2014 12:05 AM, Lina Iyer wrote:
> On Tue, Dec 02 2014 at 10:40 -0700, Lina Iyer wrote:
> +
> [...]
[ ... ]
>> +static int __init qcom_spm_init(void)
>> +{
>> + int ret;
>> +
>> + /*
>> + * cpuidle driver need to registered before the cpuidle device
>> + * for any cpu. Register the device for the the cpuidle driver.
>> + */
>> + ret = platform_device_register(&qcom_cpuidle_drv);
>> + if (ret)
>> + return ret;
> Stephen pointed out that we would have the platform device lying around
> on a non-QCOM device when using multi_v7_defconfig.
Perhaps I am missing the point, but this is not supposed to happen, no ?
> So instead of doing this here, we could do this in the probe..
>
> if (!cpuidle_get_driver()) {
> int ret = platform_device_register(&qcom_cpuidle_drv);
> if (ret)
> return ret;
> }
>
> Would that be okay?
>
> The successful probe indicates that we are on a QCOM SoC, and we have not
> registered a cpuidle_driver before this.
>
> Thanks,
> Lina
>
>> +
>> + return platform_driver_register(&spm_driver);
>> +}
>> +module_init(qcom_spm_init);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("SAW power controller driver");
>> +MODULE_ALIAS("platform:saw");
>> diff --git a/include/soc/qcom/pm.h b/include/soc/qcom/pm.h
>> new file mode 100644
>> index 0000000..d9a56d7
>> --- /dev/null
>> +++ b/include/soc/qcom/pm.h
>> @@ -0,0 +1,31 @@
>> +/*
>> + * Copyright (c) 2009-2014, The Linux Foundation. All rights reserved.
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#ifndef __QCOM_PM_H
>> +#define __QCOM_PM_H
>> +
>> +enum pm_sleep_mode {
>> + PM_SLEEP_MODE_STBY,
>> + PM_SLEEP_MODE_RET,
>> + PM_SLEEP_MODE_SPC,
>> + PM_SLEEP_MODE_PC,
>> + PM_SLEEP_MODE_NR,
>> +};
>> +
>> +struct qcom_cpu_pm_ops {
>> + int (*standby)(void *data);
>> + int (*spc)(void *data);
>> +};
>> +
>> +#endif /* __QCOM_PM_H */
>> --
>> 2.1.0
>>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 62+ messages in thread* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-03 9:11 ` Daniel Lezcano
0 siblings, 0 replies; 62+ messages in thread
From: Daniel Lezcano @ 2014-12-03 9:11 UTC (permalink / raw)
To: linux-arm-kernel
On 12/03/2014 12:05 AM, Lina Iyer wrote:
> On Tue, Dec 02 2014 at 10:40 -0700, Lina Iyer wrote:
> +
> [...]
[ ... ]
>> +static int __init qcom_spm_init(void)
>> +{
>> + int ret;
>> +
>> + /*
>> + * cpuidle driver need to registered before the cpuidle device
>> + * for any cpu. Register the device for the the cpuidle driver.
>> + */
>> + ret = platform_device_register(&qcom_cpuidle_drv);
>> + if (ret)
>> + return ret;
> Stephen pointed out that we would have the platform device lying around
> on a non-QCOM device when using multi_v7_defconfig.
Perhaps I am missing the point, but this is not supposed to happen, no ?
> So instead of doing this here, we could do this in the probe..
>
> if (!cpuidle_get_driver()) {
> int ret = platform_device_register(&qcom_cpuidle_drv);
> if (ret)
> return ret;
> }
>
> Would that be okay?
>
> The successful probe indicates that we are on a QCOM SoC, and we have not
> registered a cpuidle_driver before this.
>
> Thanks,
> Lina
>
>> +
>> + return platform_driver_register(&spm_driver);
>> +}
>> +module_init(qcom_spm_init);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("SAW power controller driver");
>> +MODULE_ALIAS("platform:saw");
>> diff --git a/include/soc/qcom/pm.h b/include/soc/qcom/pm.h
>> new file mode 100644
>> index 0000000..d9a56d7
>> --- /dev/null
>> +++ b/include/soc/qcom/pm.h
>> @@ -0,0 +1,31 @@
>> +/*
>> + * Copyright (c) 2009-2014, The Linux Foundation. All rights reserved.
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#ifndef __QCOM_PM_H
>> +#define __QCOM_PM_H
>> +
>> +enum pm_sleep_mode {
>> + PM_SLEEP_MODE_STBY,
>> + PM_SLEEP_MODE_RET,
>> + PM_SLEEP_MODE_SPC,
>> + PM_SLEEP_MODE_PC,
>> + PM_SLEEP_MODE_NR,
>> +};
>> +
>> +struct qcom_cpu_pm_ops {
>> + int (*standby)(void *data);
>> + int (*spc)(void *data);
>> +};
>> +
>> +#endif /* __QCOM_PM_H */
>> --
>> 2.1.0
>>
--
<http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 62+ messages in thread* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-03 9:11 ` Daniel Lezcano
@ 2014-12-03 14:31 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-03 14:31 UTC (permalink / raw)
To: Daniel Lezcano
Cc: khilman, sboyd, galak, linux-arm-msm, linux-pm, linux-arm-kernel,
lorenzo.pieralisi, msivasub, devicetree
On Wed, Dec 03 2014 at 02:11 -0700, Daniel Lezcano wrote:
>On 12/03/2014 12:05 AM, Lina Iyer wrote:
>>On Tue, Dec 02 2014 at 10:40 -0700, Lina Iyer wrote:
>>+
>>[...]
>
>[ ... ]
>
>>>+static int __init qcom_spm_init(void)
>>>+{
>>>+ int ret;
>>>+
>>>+ /*
>>>+ * cpuidle driver need to registered before the cpuidle device
>>>+ * for any cpu. Register the device for the the cpuidle driver.
>>>+ */
>>>+ ret = platform_device_register(&qcom_cpuidle_drv);
>>>+ if (ret)
>>>+ return ret;
>>Stephen pointed out that we would have the platform device lying around
>>on a non-QCOM device when using multi_v7_defconfig.
>
>Perhaps I am missing the point, but this is not supposed to happen, no ?
>
This would happen, since the file would compile on multi_v7 and we would
initialize and register this device regardless. The cpuidle-qcom.c
driver probe would bail out looking for a matching compatible property.
So we would not register a cpuidle driver but the device would lay
around.
>>So instead of doing this here, we could do this in the probe..
>>
>>if (!cpuidle_get_driver()) {
>> int ret = platform_device_register(&qcom_cpuidle_drv);
>> if (ret)
>> return ret;
>>}
>>
>>Would that be okay?
>>
>>The successful probe indicates that we are on a QCOM SoC, and we have not
>>registered a cpuidle_driver before this.
>>
>>Thanks,
>>Lina
>>
>>>+
>>>+ return platform_driver_register(&spm_driver);
>>>+}
>>>+module_init(qcom_spm_init);
>>>+
>>>+MODULE_LICENSE("GPL v2");
>>>+MODULE_DESCRIPTION("SAW power controller driver");
>>>+MODULE_ALIAS("platform:saw");
>>>diff --git a/include/soc/qcom/pm.h b/include/soc/qcom/pm.h
>>>new file mode 100644
>>>index 0000000..d9a56d7
>>>--- /dev/null
>>>+++ b/include/soc/qcom/pm.h
>>>@@ -0,0 +1,31 @@
>>>+/*
>>>+ * Copyright (c) 2009-2014, The Linux Foundation. All rights reserved.
>>>+ *
>>>+ * This software is licensed under the terms of the GNU General Public
>>>+ * License version 2, as published by the Free Software Foundation, and
>>>+ * may be copied, distributed, and modified under those terms.
>>>+ *
>>>+ * This program is distributed in the hope that it will be useful,
>>>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>+ * GNU General Public License for more details.
>>>+ *
>>>+ */
>>>+
>>>+#ifndef __QCOM_PM_H
>>>+#define __QCOM_PM_H
>>>+
>>>+enum pm_sleep_mode {
>>>+ PM_SLEEP_MODE_STBY,
>>>+ PM_SLEEP_MODE_RET,
>>>+ PM_SLEEP_MODE_SPC,
>>>+ PM_SLEEP_MODE_PC,
>>>+ PM_SLEEP_MODE_NR,
>>>+};
>>>+
>>>+struct qcom_cpu_pm_ops {
>>>+ int (*standby)(void *data);
>>>+ int (*spc)(void *data);
>>>+};
>>>+
>>>+#endif /* __QCOM_PM_H */
>>>--
>>>2.1.0
>>>
>
>
>--
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
>Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
><http://twitter.com/#!/linaroorg> Twitter |
><http://www.linaro.org/linaro-blog/> Blog
>
^ permalink raw reply [flat|nested] 62+ messages in thread* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-03 14:31 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-03 14:31 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Dec 03 2014 at 02:11 -0700, Daniel Lezcano wrote:
>On 12/03/2014 12:05 AM, Lina Iyer wrote:
>>On Tue, Dec 02 2014 at 10:40 -0700, Lina Iyer wrote:
>>+
>>[...]
>
>[ ... ]
>
>>>+static int __init qcom_spm_init(void)
>>>+{
>>>+ int ret;
>>>+
>>>+ /*
>>>+ * cpuidle driver need to registered before the cpuidle device
>>>+ * for any cpu. Register the device for the the cpuidle driver.
>>>+ */
>>>+ ret = platform_device_register(&qcom_cpuidle_drv);
>>>+ if (ret)
>>>+ return ret;
>>Stephen pointed out that we would have the platform device lying around
>>on a non-QCOM device when using multi_v7_defconfig.
>
>Perhaps I am missing the point, but this is not supposed to happen, no ?
>
This would happen, since the file would compile on multi_v7 and we would
initialize and register this device regardless. The cpuidle-qcom.c
driver probe would bail out looking for a matching compatible property.
So we would not register a cpuidle driver but the device would lay
around.
>>So instead of doing this here, we could do this in the probe..
>>
>>if (!cpuidle_get_driver()) {
>> int ret = platform_device_register(&qcom_cpuidle_drv);
>> if (ret)
>> return ret;
>>}
>>
>>Would that be okay?
>>
>>The successful probe indicates that we are on a QCOM SoC, and we have not
>>registered a cpuidle_driver before this.
>>
>>Thanks,
>>Lina
>>
>>>+
>>>+ return platform_driver_register(&spm_driver);
>>>+}
>>>+module_init(qcom_spm_init);
>>>+
>>>+MODULE_LICENSE("GPL v2");
>>>+MODULE_DESCRIPTION("SAW power controller driver");
>>>+MODULE_ALIAS("platform:saw");
>>>diff --git a/include/soc/qcom/pm.h b/include/soc/qcom/pm.h
>>>new file mode 100644
>>>index 0000000..d9a56d7
>>>--- /dev/null
>>>+++ b/include/soc/qcom/pm.h
>>>@@ -0,0 +1,31 @@
>>>+/*
>>>+ * Copyright (c) 2009-2014, The Linux Foundation. All rights reserved.
>>>+ *
>>>+ * This software is licensed under the terms of the GNU General Public
>>>+ * License version 2, as published by the Free Software Foundation, and
>>>+ * may be copied, distributed, and modified under those terms.
>>>+ *
>>>+ * This program is distributed in the hope that it will be useful,
>>>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>+ * GNU General Public License for more details.
>>>+ *
>>>+ */
>>>+
>>>+#ifndef __QCOM_PM_H
>>>+#define __QCOM_PM_H
>>>+
>>>+enum pm_sleep_mode {
>>>+ PM_SLEEP_MODE_STBY,
>>>+ PM_SLEEP_MODE_RET,
>>>+ PM_SLEEP_MODE_SPC,
>>>+ PM_SLEEP_MODE_PC,
>>>+ PM_SLEEP_MODE_NR,
>>>+};
>>>+
>>>+struct qcom_cpu_pm_ops {
>>>+ int (*standby)(void *data);
>>>+ int (*spc)(void *data);
>>>+};
>>>+
>>>+#endif /* __QCOM_PM_H */
>>>--
>>>2.1.0
>>>
>
>
>--
> <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
>
>Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
><http://twitter.com/#!/linaroorg> Twitter |
><http://www.linaro.org/linaro-blog/> Blog
>
^ permalink raw reply [flat|nested] 62+ messages in thread* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-03 14:31 ` Lina Iyer
@ 2014-12-03 14:55 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-03 14:55 UTC (permalink / raw)
To: Daniel Lezcano
Cc: khilman, sboyd, galak, linux-arm-msm, linux-pm, linux-arm-kernel,
lorenzo.pieralisi, msivasub, devicetree
On Wed, Dec 03 2014 at 07:31 -0700, Lina Iyer wrote:
>On Wed, Dec 03 2014 at 02:11 -0700, Daniel Lezcano wrote:
>>On 12/03/2014 12:05 AM, Lina Iyer wrote:
>>>On Tue, Dec 02 2014 at 10:40 -0700, Lina Iyer wrote:
>>>+
>>>[...]
>>
>>[ ... ]
>>
>>>>+static int __init qcom_spm_init(void)
>>>>+{
>>>>+ int ret;
>>>>+
>>>>+ /*
>>>>+ * cpuidle driver need to registered before the cpuidle device
>>>>+ * for any cpu. Register the device for the the cpuidle driver.
>>>>+ */
>>>>+ ret = platform_device_register(&qcom_cpuidle_drv);
>>>>+ if (ret)
>>>>+ return ret;
>>>Stephen pointed out that we would have the platform device lying around
>>>on a non-QCOM device when using multi_v7_defconfig.
>>
>>Perhaps I am missing the point, but this is not supposed to happen, no ?
>>
>This would happen, since the file would compile on multi_v7 and we would
>initialize and register this device regardless. The cpuidle-qcom.c
>driver probe would bail out looking for a matching compatible property.
>So we would not register a cpuidle driver but the device would lay
>around.
>
Not the best thing to do, but I noticed that ACPI driver does reference
count to register cpuidle driver for the first device and remove them as
the reference count decreases. I am not sure, if we need to reference
count here, since the SPM devices themselves are not removable.
>>>So instead of doing this here, we could do this in the probe..
>>>
>>>if (!cpuidle_get_driver()) {
>>> int ret = platform_device_register(&qcom_cpuidle_drv);
>>> if (ret)
>>> return ret;
>>>}
>>>
>>>Would that be okay?
>>>
>>>The successful probe indicates that we are on a QCOM SoC, and we have not
>>>registered a cpuidle_driver before this.
>>>
>>>Thanks,
>>>Lina
>>>
>>>>+
>>>>+ return platform_driver_register(&spm_driver);
>>>>+}
>>>>+module_init(qcom_spm_init);
>>>>+
>>>>+MODULE_LICENSE("GPL v2");
>>>>+MODULE_DESCRIPTION("SAW power controller driver");
>>>>+MODULE_ALIAS("platform:saw");
>>>>diff --git a/include/soc/qcom/pm.h b/include/soc/qcom/pm.h
>>>>new file mode 100644
>>>>index 0000000..d9a56d7
>>>>--- /dev/null
>>>>+++ b/include/soc/qcom/pm.h
>>>>@@ -0,0 +1,31 @@
>>>>+/*
>>>>+ * Copyright (c) 2009-2014, The Linux Foundation. All rights reserved.
>>>>+ *
>>>>+ * This software is licensed under the terms of the GNU General Public
>>>>+ * License version 2, as published by the Free Software Foundation, and
>>>>+ * may be copied, distributed, and modified under those terms.
>>>>+ *
>>>>+ * This program is distributed in the hope that it will be useful,
>>>>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>>+ * GNU General Public License for more details.
>>>>+ *
>>>>+ */
>>>>+
>>>>+#ifndef __QCOM_PM_H
>>>>+#define __QCOM_PM_H
>>>>+
>>>>+enum pm_sleep_mode {
>>>>+ PM_SLEEP_MODE_STBY,
>>>>+ PM_SLEEP_MODE_RET,
>>>>+ PM_SLEEP_MODE_SPC,
>>>>+ PM_SLEEP_MODE_PC,
>>>>+ PM_SLEEP_MODE_NR,
>>>>+};
>>>>+
>>>>+struct qcom_cpu_pm_ops {
>>>>+ int (*standby)(void *data);
>>>>+ int (*spc)(void *data);
>>>>+};
>>>>+
>>>>+#endif /* __QCOM_PM_H */
>>>>--
>>>>2.1.0
>>>>
>>
>>
>>--
>><http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>>
>>Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
>><http://twitter.com/#!/linaroorg> Twitter |
>><http://www.linaro.org/linaro-blog/> Blog
>>
^ permalink raw reply [flat|nested] 62+ messages in thread* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-03 14:55 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-03 14:55 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Dec 03 2014 at 07:31 -0700, Lina Iyer wrote:
>On Wed, Dec 03 2014 at 02:11 -0700, Daniel Lezcano wrote:
>>On 12/03/2014 12:05 AM, Lina Iyer wrote:
>>>On Tue, Dec 02 2014 at 10:40 -0700, Lina Iyer wrote:
>>>+
>>>[...]
>>
>>[ ... ]
>>
>>>>+static int __init qcom_spm_init(void)
>>>>+{
>>>>+ int ret;
>>>>+
>>>>+ /*
>>>>+ * cpuidle driver need to registered before the cpuidle device
>>>>+ * for any cpu. Register the device for the the cpuidle driver.
>>>>+ */
>>>>+ ret = platform_device_register(&qcom_cpuidle_drv);
>>>>+ if (ret)
>>>>+ return ret;
>>>Stephen pointed out that we would have the platform device lying around
>>>on a non-QCOM device when using multi_v7_defconfig.
>>
>>Perhaps I am missing the point, but this is not supposed to happen, no ?
>>
>This would happen, since the file would compile on multi_v7 and we would
>initialize and register this device regardless. The cpuidle-qcom.c
>driver probe would bail out looking for a matching compatible property.
>So we would not register a cpuidle driver but the device would lay
>around.
>
Not the best thing to do, but I noticed that ACPI driver does reference
count to register cpuidle driver for the first device and remove them as
the reference count decreases. I am not sure, if we need to reference
count here, since the SPM devices themselves are not removable.
>>>So instead of doing this here, we could do this in the probe..
>>>
>>>if (!cpuidle_get_driver()) {
>>> int ret = platform_device_register(&qcom_cpuidle_drv);
>>> if (ret)
>>> return ret;
>>>}
>>>
>>>Would that be okay?
>>>
>>>The successful probe indicates that we are on a QCOM SoC, and we have not
>>>registered a cpuidle_driver before this.
>>>
>>>Thanks,
>>>Lina
>>>
>>>>+
>>>>+ return platform_driver_register(&spm_driver);
>>>>+}
>>>>+module_init(qcom_spm_init);
>>>>+
>>>>+MODULE_LICENSE("GPL v2");
>>>>+MODULE_DESCRIPTION("SAW power controller driver");
>>>>+MODULE_ALIAS("platform:saw");
>>>>diff --git a/include/soc/qcom/pm.h b/include/soc/qcom/pm.h
>>>>new file mode 100644
>>>>index 0000000..d9a56d7
>>>>--- /dev/null
>>>>+++ b/include/soc/qcom/pm.h
>>>>@@ -0,0 +1,31 @@
>>>>+/*
>>>>+ * Copyright (c) 2009-2014, The Linux Foundation. All rights reserved.
>>>>+ *
>>>>+ * This software is licensed under the terms of the GNU General Public
>>>>+ * License version 2, as published by the Free Software Foundation, and
>>>>+ * may be copied, distributed, and modified under those terms.
>>>>+ *
>>>>+ * This program is distributed in the hope that it will be useful,
>>>>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>>+ * GNU General Public License for more details.
>>>>+ *
>>>>+ */
>>>>+
>>>>+#ifndef __QCOM_PM_H
>>>>+#define __QCOM_PM_H
>>>>+
>>>>+enum pm_sleep_mode {
>>>>+ PM_SLEEP_MODE_STBY,
>>>>+ PM_SLEEP_MODE_RET,
>>>>+ PM_SLEEP_MODE_SPC,
>>>>+ PM_SLEEP_MODE_PC,
>>>>+ PM_SLEEP_MODE_NR,
>>>>+};
>>>>+
>>>>+struct qcom_cpu_pm_ops {
>>>>+ int (*standby)(void *data);
>>>>+ int (*spc)(void *data);
>>>>+};
>>>>+
>>>>+#endif /* __QCOM_PM_H */
>>>>--
>>>>2.1.0
>>>>
>>
>>
>>--
>><http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs
>>
>>Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
>><http://twitter.com/#!/linaroorg> Twitter |
>><http://www.linaro.org/linaro-blog/> Blog
>>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-03 14:31 ` Lina Iyer
@ 2014-12-03 20:35 ` Arnd Bergmann
-1 siblings, 0 replies; 62+ messages in thread
From: Arnd Bergmann @ 2014-12-03 20:35 UTC (permalink / raw)
To: Lina Iyer
Cc: Daniel Lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel, lorenzo.pieralisi, msivasub, devicetree
On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote:
> >>>+static int __init qcom_spm_init(void)
> >>>+{
> >>>+ int ret;
> >>>+
> >>>+ /*
> >>>+ * cpuidle driver need to registered before the cpuidle device
> >>>+ * for any cpu. Register the device for the the cpuidle driver.
> >>>+ */
> >>>+ ret = platform_device_register(&qcom_cpuidle_drv);
> >>>+ if (ret)
> >>>+ return ret;
> >>Stephen pointed out that we would have the platform device lying around
> >>on a non-QCOM device when using multi_v7_defconfig.
> >
> >Perhaps I am missing the point, but this is not supposed to happen, no ?
> >
> This would happen, since the file would compile on multi_v7 and we would
> initialize and register this device regardless. The cpuidle-qcom.c
> driver probe would bail out looking for a matching compatible property.
> So we would not register a cpuidle driver but the device would lay
> around.
I think the problem is registering a platform_device. I've complained
about this before, but it still seems to get copied all over the
place. Please don't do this but have a driver that looks at DT to
figure out whether to access hardware or not.
Arnd
^ permalink raw reply [flat|nested] 62+ messages in thread* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-03 20:35 ` Arnd Bergmann
0 siblings, 0 replies; 62+ messages in thread
From: Arnd Bergmann @ 2014-12-03 20:35 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote:
> >>>+static int __init qcom_spm_init(void)
> >>>+{
> >>>+ int ret;
> >>>+
> >>>+ /*
> >>>+ * cpuidle driver need to registered before the cpuidle device
> >>>+ * for any cpu. Register the device for the the cpuidle driver.
> >>>+ */
> >>>+ ret = platform_device_register(&qcom_cpuidle_drv);
> >>>+ if (ret)
> >>>+ return ret;
> >>Stephen pointed out that we would have the platform device lying around
> >>on a non-QCOM device when using multi_v7_defconfig.
> >
> >Perhaps I am missing the point, but this is not supposed to happen, no ?
> >
> This would happen, since the file would compile on multi_v7 and we would
> initialize and register this device regardless. The cpuidle-qcom.c
> driver probe would bail out looking for a matching compatible property.
> So we would not register a cpuidle driver but the device would lay
> around.
I think the problem is registering a platform_device. I've complained
about this before, but it still seems to get copied all over the
place. Please don't do this but have a driver that looks at DT to
figure out whether to access hardware or not.
Arnd
^ permalink raw reply [flat|nested] 62+ messages in thread* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-03 20:35 ` Arnd Bergmann
@ 2014-12-04 8:52 ` Daniel Lezcano
-1 siblings, 0 replies; 62+ messages in thread
From: Daniel Lezcano @ 2014-12-04 8:52 UTC (permalink / raw)
To: Arnd Bergmann, Lina Iyer
Cc: khilman, sboyd, galak, linux-arm-msm, linux-pm, linux-arm-kernel,
lorenzo.pieralisi, msivasub, devicetree
On 12/03/2014 09:35 PM, Arnd Bergmann wrote:
> On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote:
>>>>> +static int __init qcom_spm_init(void)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + /*
>>>>> + * cpuidle driver need to registered before the cpuidle device
>>>>> + * for any cpu. Register the device for the the cpuidle driver.
>>>>> + */
>>>>> + ret = platform_device_register(&qcom_cpuidle_drv);
>>>>> + if (ret)
>>>>> + return ret;
>>>> Stephen pointed out that we would have the platform device lying around
>>>> on a non-QCOM device when using multi_v7_defconfig.
>>>
>>> Perhaps I am missing the point, but this is not supposed to happen, no ?
>>>
>> This would happen, since the file would compile on multi_v7 and we would
>> initialize and register this device regardless. The cpuidle-qcom.c
>> driver probe would bail out looking for a matching compatible property.
>> So we would not register a cpuidle driver but the device would lay
>> around.
>
> I think the problem is registering a platform_device. I've complained
> about this before, but it still seems to get copied all over the
> place. Please don't do this but have a driver that looks at DT to
> figure out whether to access hardware or not.
We did this approach but, I can remember why, someone was complaining
about it also :)
The platform device/driver paradigm allowed us to split the arch
specific parts by passing the pm ops through the platform data.
Would make sense to have a single common place for the ARM arch where we
initialize the platform device for cpuidle ?
--
<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
^ permalink raw reply [flat|nested] 62+ messages in thread* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-04 8:52 ` Daniel Lezcano
0 siblings, 0 replies; 62+ messages in thread
From: Daniel Lezcano @ 2014-12-04 8:52 UTC (permalink / raw)
To: linux-arm-kernel
On 12/03/2014 09:35 PM, Arnd Bergmann wrote:
> On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote:
>>>>> +static int __init qcom_spm_init(void)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + /*
>>>>> + * cpuidle driver need to registered before the cpuidle device
>>>>> + * for any cpu. Register the device for the the cpuidle driver.
>>>>> + */
>>>>> + ret = platform_device_register(&qcom_cpuidle_drv);
>>>>> + if (ret)
>>>>> + return ret;
>>>> Stephen pointed out that we would have the platform device lying around
>>>> on a non-QCOM device when using multi_v7_defconfig.
>>>
>>> Perhaps I am missing the point, but this is not supposed to happen, no ?
>>>
>> This would happen, since the file would compile on multi_v7 and we would
>> initialize and register this device regardless. The cpuidle-qcom.c
>> driver probe would bail out looking for a matching compatible property.
>> So we would not register a cpuidle driver but the device would lay
>> around.
>
> I think the problem is registering a platform_device. I've complained
> about this before, but it still seems to get copied all over the
> place. Please don't do this but have a driver that looks at DT to
> figure out whether to access hardware or not.
We did this approach but, I can remember why, someone was complaining
about it also :)
The platform device/driver paradigm allowed us to split the arch
specific parts by passing the pm ops through the platform data.
Would make sense to have a single common place for the ARM arch where we
initialize the platform device for cpuidle ?
--
<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
^ permalink raw reply [flat|nested] 62+ messages in thread* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-04 8:52 ` Daniel Lezcano
@ 2014-12-04 9:01 ` Arnd Bergmann
-1 siblings, 0 replies; 62+ messages in thread
From: Arnd Bergmann @ 2014-12-04 9:01 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Lina Iyer, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel, lorenzo.pieralisi, msivasub, devicetree
On Thursday 04 December 2014 09:52:39 Daniel Lezcano wrote:
> On 12/03/2014 09:35 PM, Arnd Bergmann wrote:
> > On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote:
> >>>>> +static int __init qcom_spm_init(void)
> >>>>> +{
> >>>>> + int ret;
> >>>>> +
> >>>>> + /*
> >>>>> + * cpuidle driver need to registered before the cpuidle device
> >>>>> + * for any cpu. Register the device for the the cpuidle driver.
> >>>>> + */
> >>>>> + ret = platform_device_register(&qcom_cpuidle_drv);
> >>>>> + if (ret)
> >>>>> + return ret;
> >>>> Stephen pointed out that we would have the platform device lying around
> >>>> on a non-QCOM device when using multi_v7_defconfig.
> >>>
> >>> Perhaps I am missing the point, but this is not supposed to happen, no ?
> >>>
> >> This would happen, since the file would compile on multi_v7 and we would
> >> initialize and register this device regardless. The cpuidle-qcom.c
> >> driver probe would bail out looking for a matching compatible property.
> >> So we would not register a cpuidle driver but the device would lay
> >> around.
> >
> > I think the problem is registering a platform_device. I've complained
> > about this before, but it still seems to get copied all over the
> > place. Please don't do this but have a driver that looks at DT to
> > figure out whether to access hardware or not.
>
> We did this approach but, I can remember why, someone was complaining
> about it also
>
> The platform device/driver paradigm allowed us to split the arch
> specific parts by passing the pm ops through the platform data.
>
> Would make sense to have a single common place for the ARM arch where we
> initialize the platform device for cpuidle ?
No. It's really not a device, and if you pretend that it is, you get
into problems like this.
Arnd
^ permalink raw reply [flat|nested] 62+ messages in thread* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-04 9:01 ` Arnd Bergmann
0 siblings, 0 replies; 62+ messages in thread
From: Arnd Bergmann @ 2014-12-04 9:01 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 04 December 2014 09:52:39 Daniel Lezcano wrote:
> On 12/03/2014 09:35 PM, Arnd Bergmann wrote:
> > On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote:
> >>>>> +static int __init qcom_spm_init(void)
> >>>>> +{
> >>>>> + int ret;
> >>>>> +
> >>>>> + /*
> >>>>> + * cpuidle driver need to registered before the cpuidle device
> >>>>> + * for any cpu. Register the device for the the cpuidle driver.
> >>>>> + */
> >>>>> + ret = platform_device_register(&qcom_cpuidle_drv);
> >>>>> + if (ret)
> >>>>> + return ret;
> >>>> Stephen pointed out that we would have the platform device lying around
> >>>> on a non-QCOM device when using multi_v7_defconfig.
> >>>
> >>> Perhaps I am missing the point, but this is not supposed to happen, no ?
> >>>
> >> This would happen, since the file would compile on multi_v7 and we would
> >> initialize and register this device regardless. The cpuidle-qcom.c
> >> driver probe would bail out looking for a matching compatible property.
> >> So we would not register a cpuidle driver but the device would lay
> >> around.
> >
> > I think the problem is registering a platform_device. I've complained
> > about this before, but it still seems to get copied all over the
> > place. Please don't do this but have a driver that looks at DT to
> > figure out whether to access hardware or not.
>
> We did this approach but, I can remember why, someone was complaining
> about it also
>
> The platform device/driver paradigm allowed us to split the arch
> specific parts by passing the pm ops through the platform data.
>
> Would make sense to have a single common place for the ARM arch where we
> initialize the platform device for cpuidle ?
No. It's really not a device, and if you pretend that it is, you get
into problems like this.
Arnd
^ permalink raw reply [flat|nested] 62+ messages in thread* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-04 9:01 ` Arnd Bergmann
@ 2014-12-04 16:28 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-04 16:28 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Daniel Lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel, lorenzo.pieralisi, msivasub, devicetree
On Thu, Dec 04 2014 at 02:02 -0700, Arnd Bergmann wrote:
>On Thursday 04 December 2014 09:52:39 Daniel Lezcano wrote:
>> On 12/03/2014 09:35 PM, Arnd Bergmann wrote:
>> > On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote:
>> >>>>> +static int __init qcom_spm_init(void)
>> >>>>> +{
>> >>>>> + int ret;
>> >>>>> +
>> >>>>> + /*
>> >>>>> + * cpuidle driver need to registered before the cpuidle device
>> >>>>> + * for any cpu. Register the device for the the cpuidle driver.
>> >>>>> + */
>> >>>>> + ret = platform_device_register(&qcom_cpuidle_drv);
>> >>>>> + if (ret)
>> >>>>> + return ret;
>> >>>> Stephen pointed out that we would have the platform device lying around
>> >>>> on a non-QCOM device when using multi_v7_defconfig.
>> >>>
>> >>> Perhaps I am missing the point, but this is not supposed to happen, no ?
>> >>>
>> >> This would happen, since the file would compile on multi_v7 and we would
>> >> initialize and register this device regardless. The cpuidle-qcom.c
>> >> driver probe would bail out looking for a matching compatible property.
>> >> So we would not register a cpuidle driver but the device would lay
>> >> around.
>> >
>> > I think the problem is registering a platform_device. I've complained
>> > about this before, but it still seems to get copied all over the
>> > place. Please don't do this but have a driver that looks at DT to
>> > figure out whether to access hardware or not.
>>
>> We did this approach but, I can remember why, someone was complaining
>> about it also
>>
>> The platform device/driver paradigm allowed us to split the arch
>> specific parts by passing the pm ops through the platform data.
>>
>> Would make sense to have a single common place for the ARM arch where we
>> initialize the platform device for cpuidle ?
>
>No. It's really not a device, and if you pretend that it is, you get
>into problems like this.
Arnd, the problem is that the provides function pointers to the SoC code
that the cpuilde driver uses to call based on the idle state.
After much discussions, we came down to using function pointers from
translating from DT strings, other than using that again, I dont see a
good way of achieving the ability of cpuidle driver staying a separate
driver from the SPM driver.
Daniel, thoughts?
^ permalink raw reply [flat|nested] 62+ messages in thread* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-04 16:28 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-04 16:28 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 04 2014 at 02:02 -0700, Arnd Bergmann wrote:
>On Thursday 04 December 2014 09:52:39 Daniel Lezcano wrote:
>> On 12/03/2014 09:35 PM, Arnd Bergmann wrote:
>> > On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote:
>> >>>>> +static int __init qcom_spm_init(void)
>> >>>>> +{
>> >>>>> + int ret;
>> >>>>> +
>> >>>>> + /*
>> >>>>> + * cpuidle driver need to registered before the cpuidle device
>> >>>>> + * for any cpu. Register the device for the the cpuidle driver.
>> >>>>> + */
>> >>>>> + ret = platform_device_register(&qcom_cpuidle_drv);
>> >>>>> + if (ret)
>> >>>>> + return ret;
>> >>>> Stephen pointed out that we would have the platform device lying around
>> >>>> on a non-QCOM device when using multi_v7_defconfig.
>> >>>
>> >>> Perhaps I am missing the point, but this is not supposed to happen, no ?
>> >>>
>> >> This would happen, since the file would compile on multi_v7 and we would
>> >> initialize and register this device regardless. The cpuidle-qcom.c
>> >> driver probe would bail out looking for a matching compatible property.
>> >> So we would not register a cpuidle driver but the device would lay
>> >> around.
>> >
>> > I think the problem is registering a platform_device. I've complained
>> > about this before, but it still seems to get copied all over the
>> > place. Please don't do this but have a driver that looks at DT to
>> > figure out whether to access hardware or not.
>>
>> We did this approach but, I can remember why, someone was complaining
>> about it also
>>
>> The platform device/driver paradigm allowed us to split the arch
>> specific parts by passing the pm ops through the platform data.
>>
>> Would make sense to have a single common place for the ARM arch where we
>> initialize the platform device for cpuidle ?
>
>No. It's really not a device, and if you pretend that it is, you get
>into problems like this.
Arnd, the problem is that the provides function pointers to the SoC code
that the cpuilde driver uses to call based on the idle state.
After much discussions, we came down to using function pointers from
translating from DT strings, other than using that again, I dont see a
good way of achieving the ability of cpuidle driver staying a separate
driver from the SPM driver.
Daniel, thoughts?
^ permalink raw reply [flat|nested] 62+ messages in thread* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-04 16:28 ` Lina Iyer
@ 2014-12-04 18:20 ` Arnd Bergmann
-1 siblings, 0 replies; 62+ messages in thread
From: Arnd Bergmann @ 2014-12-04 18:20 UTC (permalink / raw)
To: Lina Iyer
Cc: Daniel Lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel, lorenzo.pieralisi, msivasub, devicetree
On Thursday 04 December 2014 09:28:34 Lina Iyer wrote:
> On Thu, Dec 04 2014 at 02:02 -0700, Arnd Bergmann wrote:
> >On Thursday 04 December 2014 09:52:39 Daniel Lezcano wrote:
> >> On 12/03/2014 09:35 PM, Arnd Bergmann wrote:
> >> > On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote:
> >> >>>>> +static int __init qcom_spm_init(void)
> >> >>>>> +{
> >> >>>>> + int ret;
> >> >>>>> +
> >> >>>>> + /*
> >> >>>>> + * cpuidle driver need to registered before the cpuidle device
> >> >>>>> + * for any cpu. Register the device for the the cpuidle driver.
> >> >>>>> + */
> >> >>>>> + ret = platform_device_register(&qcom_cpuidle_drv);
> >> >>>>> + if (ret)
> >> >>>>> + return ret;
> >> >>>> Stephen pointed out that we would have the platform device lying around
> >> >>>> on a non-QCOM device when using multi_v7_defconfig.
> >> >>>
> >> >>> Perhaps I am missing the point, but this is not supposed to happen, no ?
> >> >>>
> >> >> This would happen, since the file would compile on multi_v7 and we would
> >> >> initialize and register this device regardless. The cpuidle-qcom.c
> >> >> driver probe would bail out looking for a matching compatible property.
> >> >> So we would not register a cpuidle driver but the device would lay
> >> >> around.
> >> >
> >> > I think the problem is registering a platform_device. I've complained
> >> > about this before, but it still seems to get copied all over the
> >> > place. Please don't do this but have a driver that looks at DT to
> >> > figure out whether to access hardware or not.
> >>
> >> We did this approach but, I can remember why, someone was complaining
> >> about it also
> >>
> >> The platform device/driver paradigm allowed us to split the arch
> >> specific parts by passing the pm ops through the platform data.
> >>
> >> Would make sense to have a single common place for the ARM arch where we
> >> initialize the platform device for cpuidle ?
> >
> >No. It's really not a device, and if you pretend that it is, you get
> >into problems like this.
>
> Arnd, the problem is that the provides function pointers to the SoC code
> that the cpuilde driver uses to call based on the idle state.
>
> After much discussions, we came down to using function pointers from
> translating from DT strings, other than using that again, I dont see a
> good way of achieving the ability of cpuidle driver staying a separate
> driver from the SPM driver.
>
> Daniel, thoughts?
Maybe the problem is trying too hard to separate two things that really
belong together then. In general, the strategy of coming up with subsystems
for a class of devices and them turning platform code into drivers for
that subsystem has worked out really well, but I think for cpufreq, cpuidle
and smp, it really hasn't, and in the third case we haven't even tried
coming up with a subsystem for that reason.
Having all cpuidle code generally live in drivers/cpuidle is still a good
idea IMO, but using a platform device just for the purpose of passing
data between some platform specific code and another platform specific
driver hasn't worked out that well here.
Arnd
^ permalink raw reply [flat|nested] 62+ messages in thread* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-04 18:20 ` Arnd Bergmann
0 siblings, 0 replies; 62+ messages in thread
From: Arnd Bergmann @ 2014-12-04 18:20 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 04 December 2014 09:28:34 Lina Iyer wrote:
> On Thu, Dec 04 2014 at 02:02 -0700, Arnd Bergmann wrote:
> >On Thursday 04 December 2014 09:52:39 Daniel Lezcano wrote:
> >> On 12/03/2014 09:35 PM, Arnd Bergmann wrote:
> >> > On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote:
> >> >>>>> +static int __init qcom_spm_init(void)
> >> >>>>> +{
> >> >>>>> + int ret;
> >> >>>>> +
> >> >>>>> + /*
> >> >>>>> + * cpuidle driver need to registered before the cpuidle device
> >> >>>>> + * for any cpu. Register the device for the the cpuidle driver.
> >> >>>>> + */
> >> >>>>> + ret = platform_device_register(&qcom_cpuidle_drv);
> >> >>>>> + if (ret)
> >> >>>>> + return ret;
> >> >>>> Stephen pointed out that we would have the platform device lying around
> >> >>>> on a non-QCOM device when using multi_v7_defconfig.
> >> >>>
> >> >>> Perhaps I am missing the point, but this is not supposed to happen, no ?
> >> >>>
> >> >> This would happen, since the file would compile on multi_v7 and we would
> >> >> initialize and register this device regardless. The cpuidle-qcom.c
> >> >> driver probe would bail out looking for a matching compatible property.
> >> >> So we would not register a cpuidle driver but the device would lay
> >> >> around.
> >> >
> >> > I think the problem is registering a platform_device. I've complained
> >> > about this before, but it still seems to get copied all over the
> >> > place. Please don't do this but have a driver that looks at DT to
> >> > figure out whether to access hardware or not.
> >>
> >> We did this approach but, I can remember why, someone was complaining
> >> about it also
> >>
> >> The platform device/driver paradigm allowed us to split the arch
> >> specific parts by passing the pm ops through the platform data.
> >>
> >> Would make sense to have a single common place for the ARM arch where we
> >> initialize the platform device for cpuidle ?
> >
> >No. It's really not a device, and if you pretend that it is, you get
> >into problems like this.
>
> Arnd, the problem is that the provides function pointers to the SoC code
> that the cpuilde driver uses to call based on the idle state.
>
> After much discussions, we came down to using function pointers from
> translating from DT strings, other than using that again, I dont see a
> good way of achieving the ability of cpuidle driver staying a separate
> driver from the SPM driver.
>
> Daniel, thoughts?
Maybe the problem is trying too hard to separate two things that really
belong together then. In general, the strategy of coming up with subsystems
for a class of devices and them turning platform code into drivers for
that subsystem has worked out really well, but I think for cpufreq, cpuidle
and smp, it really hasn't, and in the third case we haven't even tried
coming up with a subsystem for that reason.
Having all cpuidle code generally live in drivers/cpuidle is still a good
idea IMO, but using a platform device just for the purpose of passing
data between some platform specific code and another platform specific
driver hasn't worked out that well here.
Arnd
^ permalink raw reply [flat|nested] 62+ messages in thread* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-04 18:20 ` Arnd Bergmann
@ 2014-12-05 15:45 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-05 15:45 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Daniel Lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel, lorenzo.pieralisi, msivasub, devicetree
On Thu, Dec 04 2014 at 11:20 -0700, Arnd Bergmann wrote:
>On Thursday 04 December 2014 09:28:34 Lina Iyer wrote:
>> On Thu, Dec 04 2014 at 02:02 -0700, Arnd Bergmann wrote:
>> >On Thursday 04 December 2014 09:52:39 Daniel Lezcano wrote:
>> >> On 12/03/2014 09:35 PM, Arnd Bergmann wrote:
>> >> > On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote:
>> >> >>>>> +static int __init qcom_spm_init(void)
>> >> >>>>> +{
>> >> >>>>> + int ret;
>> >> >>>>> +
>> >> >>>>> + /*
>> >> >>>>> + * cpuidle driver need to registered before the cpuidle device
>> >> >>>>> + * for any cpu. Register the device for the the cpuidle driver.
>> >> >>>>> + */
>> >> >>>>> + ret = platform_device_register(&qcom_cpuidle_drv);
>> >> >>>>> + if (ret)
>> >> >>>>> + return ret;
>> >> >>>> Stephen pointed out that we would have the platform device lying around
>> >> >>>> on a non-QCOM device when using multi_v7_defconfig.
>> >> >>>
>> >> >>> Perhaps I am missing the point, but this is not supposed to happen, no ?
>> >> >>>
>> >> >> This would happen, since the file would compile on multi_v7 and we would
>> >> >> initialize and register this device regardless. The cpuidle-qcom.c
>> >> >> driver probe would bail out looking for a matching compatible property.
>> >> >> So we would not register a cpuidle driver but the device would lay
>> >> >> around.
>> >> >
>> >> > I think the problem is registering a platform_device. I've complained
>> >> > about this before, but it still seems to get copied all over the
>> >> > place. Please don't do this but have a driver that looks at DT to
>> >> > figure out whether to access hardware or not.
>> >>
>> >> We did this approach but, I can remember why, someone was complaining
>> >> about it also
>> >>
>> >> The platform device/driver paradigm allowed us to split the arch
>> >> specific parts by passing the pm ops through the platform data.
>> >>
>> >> Would make sense to have a single common place for the ARM arch where we
>> >> initialize the platform device for cpuidle ?
>> >
>> >No. It's really not a device, and if you pretend that it is, you get
>> >into problems like this.
>>
>> Arnd, the problem is that the provides function pointers to the SoC code
>> that the cpuilde driver uses to call based on the idle state.
>>
>> After much discussions, we came down to using function pointers from
>> translating from DT strings, other than using that again, I dont see a
>> good way of achieving the ability of cpuidle driver staying a separate
>> driver from the SPM driver.
>>
>> Daniel, thoughts?
>
>Maybe the problem is trying too hard to separate two things that really
>belong together then. In general, the strategy of coming up with subsystems
>for a class of devices and them turning platform code into drivers for
>that subsystem has worked out really well, but I think for cpufreq, cpuidle
>and smp, it really hasn't, and in the third case we haven't even tried
>coming up with a subsystem for that reason.
>
>Having all cpuidle code generally live in drivers/cpuidle is still a good
>idea IMO, but using a platform device just for the purpose of passing
>data between some platform specific code and another platform specific
>driver hasn't worked out that well here.
>
To achieve both, I cant think of a better way to initialize the cpuidle
driver without the use of reference count (0 ==>
platform_driver_register).
I tried creating dummy platform device in the DT but something or
another gives in to an ugly implementation.
Any other ideas?
Lina.
^ permalink raw reply [flat|nested] 62+ messages in thread* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-05 15:45 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-05 15:45 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 04 2014 at 11:20 -0700, Arnd Bergmann wrote:
>On Thursday 04 December 2014 09:28:34 Lina Iyer wrote:
>> On Thu, Dec 04 2014 at 02:02 -0700, Arnd Bergmann wrote:
>> >On Thursday 04 December 2014 09:52:39 Daniel Lezcano wrote:
>> >> On 12/03/2014 09:35 PM, Arnd Bergmann wrote:
>> >> > On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote:
>> >> >>>>> +static int __init qcom_spm_init(void)
>> >> >>>>> +{
>> >> >>>>> + int ret;
>> >> >>>>> +
>> >> >>>>> + /*
>> >> >>>>> + * cpuidle driver need to registered before the cpuidle device
>> >> >>>>> + * for any cpu. Register the device for the the cpuidle driver.
>> >> >>>>> + */
>> >> >>>>> + ret = platform_device_register(&qcom_cpuidle_drv);
>> >> >>>>> + if (ret)
>> >> >>>>> + return ret;
>> >> >>>> Stephen pointed out that we would have the platform device lying around
>> >> >>>> on a non-QCOM device when using multi_v7_defconfig.
>> >> >>>
>> >> >>> Perhaps I am missing the point, but this is not supposed to happen, no ?
>> >> >>>
>> >> >> This would happen, since the file would compile on multi_v7 and we would
>> >> >> initialize and register this device regardless. The cpuidle-qcom.c
>> >> >> driver probe would bail out looking for a matching compatible property.
>> >> >> So we would not register a cpuidle driver but the device would lay
>> >> >> around.
>> >> >
>> >> > I think the problem is registering a platform_device. I've complained
>> >> > about this before, but it still seems to get copied all over the
>> >> > place. Please don't do this but have a driver that looks at DT to
>> >> > figure out whether to access hardware or not.
>> >>
>> >> We did this approach but, I can remember why, someone was complaining
>> >> about it also
>> >>
>> >> The platform device/driver paradigm allowed us to split the arch
>> >> specific parts by passing the pm ops through the platform data.
>> >>
>> >> Would make sense to have a single common place for the ARM arch where we
>> >> initialize the platform device for cpuidle ?
>> >
>> >No. It's really not a device, and if you pretend that it is, you get
>> >into problems like this.
>>
>> Arnd, the problem is that the provides function pointers to the SoC code
>> that the cpuilde driver uses to call based on the idle state.
>>
>> After much discussions, we came down to using function pointers from
>> translating from DT strings, other than using that again, I dont see a
>> good way of achieving the ability of cpuidle driver staying a separate
>> driver from the SPM driver.
>>
>> Daniel, thoughts?
>
>Maybe the problem is trying too hard to separate two things that really
>belong together then. In general, the strategy of coming up with subsystems
>for a class of devices and them turning platform code into drivers for
>that subsystem has worked out really well, but I think for cpufreq, cpuidle
>and smp, it really hasn't, and in the third case we haven't even tried
>coming up with a subsystem for that reason.
>
>Having all cpuidle code generally live in drivers/cpuidle is still a good
>idea IMO, but using a platform device just for the purpose of passing
>data between some platform specific code and another platform specific
>driver hasn't worked out that well here.
>
To achieve both, I cant think of a better way to initialize the cpuidle
driver without the use of reference count (0 ==>
platform_driver_register).
I tried creating dummy platform device in the DT but something or
another gives in to an ugly implementation.
Any other ideas?
Lina.
^ permalink raw reply [flat|nested] 62+ messages in thread* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-05 15:45 ` Lina Iyer
@ 2014-12-16 14:39 ` Arnd Bergmann
-1 siblings, 0 replies; 62+ messages in thread
From: Arnd Bergmann @ 2014-12-16 14:39 UTC (permalink / raw)
To: Lina Iyer
Cc: Daniel Lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel, lorenzo.pieralisi, msivasub, devicetree
On Friday 05 December 2014 08:45:26 Lina Iyer wrote:
> On Thu, Dec 04 2014 at 11:20 -0700, Arnd Bergmann wrote:
> >On Thursday 04 December 2014 09:28:34 Lina Iyer wrote:
> >Having all cpuidle code generally live in drivers/cpuidle is still a good
> >idea IMO, but using a platform device just for the purpose of passing
> >data between some platform specific code and another platform specific
> >driver hasn't worked out that well here.
> >
> To achieve both, I cant think of a better way to initialize the cpuidle
> driver without the use of reference count (0 ==>
> platform_driver_register).
>
> I tried creating dummy platform device in the DT but something or
> another gives in to an ugly implementation.
>
> Any other ideas?
Hi Lina,
sorry for missing your mail earlier, I just replied to Daniel on this
topic, when the thread popped up again.
Arnd
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-16 14:39 ` Arnd Bergmann
0 siblings, 0 replies; 62+ messages in thread
From: Arnd Bergmann @ 2014-12-16 14:39 UTC (permalink / raw)
To: linux-arm-kernel
On Friday 05 December 2014 08:45:26 Lina Iyer wrote:
> On Thu, Dec 04 2014 at 11:20 -0700, Arnd Bergmann wrote:
> >On Thursday 04 December 2014 09:28:34 Lina Iyer wrote:
> >Having all cpuidle code generally live in drivers/cpuidle is still a good
> >idea IMO, but using a platform device just for the purpose of passing
> >data between some platform specific code and another platform specific
> >driver hasn't worked out that well here.
> >
> To achieve both, I cant think of a better way to initialize the cpuidle
> driver without the use of reference count (0 ==>
> platform_driver_register).
>
> I tried creating dummy platform device in the DT but something or
> another gives in to an ugly implementation.
>
> Any other ideas?
Hi Lina,
sorry for missing your mail earlier, I just replied to Daniel on this
topic, when the thread popped up again.
Arnd
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-04 18:20 ` Arnd Bergmann
@ 2014-12-16 14:12 ` Daniel Lezcano
-1 siblings, 0 replies; 62+ messages in thread
From: Daniel Lezcano @ 2014-12-16 14:12 UTC (permalink / raw)
To: Arnd Bergmann, Lina Iyer
Cc: khilman, sboyd, galak, linux-arm-msm, linux-pm, linux-arm-kernel,
lorenzo.pieralisi, msivasub, devicetree
On 12/04/2014 07:20 PM, Arnd Bergmann wrote:
> On Thursday 04 December 2014 09:28:34 Lina Iyer wrote:
>> On Thu, Dec 04 2014 at 02:02 -0700, Arnd Bergmann wrote:
>>> On Thursday 04 December 2014 09:52:39 Daniel Lezcano wrote:
>>>> On 12/03/2014 09:35 PM, Arnd Bergmann wrote:
>>>>> On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote:
>>>>>>>>> +static int __init qcom_spm_init(void)
>>>>>>>>> +{
>>>>>>>>> + int ret;
>>>>>>>>> +
>>>>>>>>> + /*
>>>>>>>>> + * cpuidle driver need to registered before the cpuidle device
>>>>>>>>> + * for any cpu. Register the device for the the cpuidle driver.
>>>>>>>>> + */
>>>>>>>>> + ret = platform_device_register(&qcom_cpuidle_drv);
>>>>>>>>> + if (ret)
>>>>>>>>> + return ret;
>>>>>>>> Stephen pointed out that we would have the platform device lying around
>>>>>>>> on a non-QCOM device when using multi_v7_defconfig.
>>>>>>>
>>>>>>> Perhaps I am missing the point, but this is not supposed to happen, no ?
>>>>>>>
>>>>>> This would happen, since the file would compile on multi_v7 and we would
>>>>>> initialize and register this device regardless. The cpuidle-qcom.c
>>>>>> driver probe would bail out looking for a matching compatible property.
>>>>>> So we would not register a cpuidle driver but the device would lay
>>>>>> around.
>>>>>
>>>>> I think the problem is registering a platform_device. I've complained
>>>>> about this before, but it still seems to get copied all over the
>>>>> place. Please don't do this but have a driver that looks at DT to
>>>>> figure out whether to access hardware or not.
>>>>
>>>> We did this approach but, I can remember why, someone was complaining
>>>> about it also
>>>>
>>>> The platform device/driver paradigm allowed us to split the arch
>>>> specific parts by passing the pm ops through the platform data.
>>>>
>>>> Would make sense to have a single common place for the ARM arch where we
>>>> initialize the platform device for cpuidle ?
>>>
>>> No. It's really not a device, and if you pretend that it is, you get
>>> into problems like this.
>>
>> Arnd, the problem is that the provides function pointers to the SoC code
>> that the cpuilde driver uses to call based on the idle state.
>>
>> After much discussions, we came down to using function pointers from
>> translating from DT strings, other than using that again, I dont see a
>> good way of achieving the ability of cpuidle driver staying a separate
>> driver from the SPM driver.
>>
>> Daniel, thoughts?
>
> Maybe the problem is trying too hard to separate two things that really
> belong together then. In general, the strategy of coming up with subsystems
> for a class of devices and them turning platform code into drivers for
> that subsystem has worked out really well, but I think for cpufreq, cpuidle
> and smp, it really hasn't, and in the third case we haven't even tried
> coming up with a subsystem for that reason.
>
> Having all cpuidle code generally live in drivers/cpuidle is still a good
> idea IMO, but using a platform device just for the purpose of passing
> data between some platform specific code and another platform specific
> driver hasn't worked out that well here.
At the beginning, all that become from not including mach files from the
drivers directory which make sense.
Perhaps it is time to write a similar mechanism for the cpuidle drivers
where we can still separate the low level PM code from the generic
cpuidle code.
Something like (very roughly):
Index: cpuidle-next/drivers/cpuidle/driver.c
===================================================================
--- cpuidle-next.orig/drivers/cpuidle/driver.c 2014-12-16
14:04:51.750861310 +0100
+++ cpuidle-next/drivers/cpuidle/driver.c 2014-12-16 15:09:52.202706756
+0100
@@ -178,6 +178,45 @@ static void __cpuidle_driver_init(struct
}
}
+struct cpuidle_ops_reg {
+ const char *name;
+ struct cpuidle_ops *ops;
+ struct list_head *list;
+};
+
+static LIST_HEAD(ops_list);
+
+static struct cpuidle_ops *cpuidle_find_ops(const char *name)
+{
+ struct cpuidle_ops_reg *ops_reg;
+
+ list_for_each_entry(ops_reg, &ops_list, list) {
+ if (!strcmp(ops_reg->name, name))
+ return ops_reg->ops;
+ }
+
+ return NULL;
+}
+
+int cpuidle_register_ops(const char *name, struct cpuidle_ops *ops)
+{
+ struct cpuidle_ops_reg *reg;
+
+ reg = kmalloc(sizeof(*reg), GFP_KERNEL);
+ if (!reg)
+ return -ENOMEM;
+
+ reg->name = name;
+ reg->ops = ops;
+ INIT_LIST_HEAD(®->list);
+
+ spin_lock(&cpuidle_driver_lock);
+ list_add(&ops_list, ®->list);
+ spin_unlock(&cpuidle_driver_lock);
+
+ return 0;
+}
+
/**
* __cpuidle_register_driver: register the driver
* @drv: a valid pointer to a struct cpuidle_driver
@@ -194,6 +233,7 @@ static void __cpuidle_driver_init(struct
static int __cpuidle_register_driver(struct cpuidle_driver *drv)
{
int ret;
+ struct cpuidle_ops *ops;
if (!drv || !drv->state_count)
return -EINVAL;
@@ -201,6 +241,10 @@ static int __cpuidle_register_driver(str
if (cpuidle_disabled())
return -ENODEV;
+ ops = cpuidle_find_ops(drv->name);
+ if (ops)
+ drv->ops = ops;
+
__cpuidle_driver_init(drv);
ret = __cpuidle_set_driver(drv);
Index: cpuidle-next/include/linux/cpuidle.h
===================================================================
--- cpuidle-next.orig/include/linux/cpuidle.h 2014-12-16
14:06:09.442858231 +0100
+++ cpuidle-next/include/linux/cpuidle.h 2014-12-16 14:59:46.594730753 +0100
@@ -109,11 +109,21 @@ static inline int cpuidle_get_last_resid
* CPUIDLE DRIVER INTERFACE *
****************************/
+struct cpuidle_ops {
+ int (*standby)(struct cpuidle_driver *drv,
+ struct cpuidle_device *dev);
+ int (*retention)(struct cpuidle_driver *drv,
+ struct cpuidle_device *dev);
+ int (*poweroff)(struct cpuidle_driver *drv,
+ struct cpuidle_device *dev);
+};
+
struct cpuidle_driver {
const char *name;
struct module *owner;
int refcnt;
+ struct cpuidle_ops *ops;
/* used by the cpuidle framework to setup the broadcast timer */
unsigned int bctimer:1;
/* states array must be ordered in decreasing power consumption */
--
<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
^ permalink raw reply [flat|nested] 62+ messages in thread* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-16 14:12 ` Daniel Lezcano
0 siblings, 0 replies; 62+ messages in thread
From: Daniel Lezcano @ 2014-12-16 14:12 UTC (permalink / raw)
To: linux-arm-kernel
On 12/04/2014 07:20 PM, Arnd Bergmann wrote:
> On Thursday 04 December 2014 09:28:34 Lina Iyer wrote:
>> On Thu, Dec 04 2014 at 02:02 -0700, Arnd Bergmann wrote:
>>> On Thursday 04 December 2014 09:52:39 Daniel Lezcano wrote:
>>>> On 12/03/2014 09:35 PM, Arnd Bergmann wrote:
>>>>> On Wednesday 03 December 2014 07:31:22 Lina Iyer wrote:
>>>>>>>>> +static int __init qcom_spm_init(void)
>>>>>>>>> +{
>>>>>>>>> + int ret;
>>>>>>>>> +
>>>>>>>>> + /*
>>>>>>>>> + * cpuidle driver need to registered before the cpuidle device
>>>>>>>>> + * for any cpu. Register the device for the the cpuidle driver.
>>>>>>>>> + */
>>>>>>>>> + ret = platform_device_register(&qcom_cpuidle_drv);
>>>>>>>>> + if (ret)
>>>>>>>>> + return ret;
>>>>>>>> Stephen pointed out that we would have the platform device lying around
>>>>>>>> on a non-QCOM device when using multi_v7_defconfig.
>>>>>>>
>>>>>>> Perhaps I am missing the point, but this is not supposed to happen, no ?
>>>>>>>
>>>>>> This would happen, since the file would compile on multi_v7 and we would
>>>>>> initialize and register this device regardless. The cpuidle-qcom.c
>>>>>> driver probe would bail out looking for a matching compatible property.
>>>>>> So we would not register a cpuidle driver but the device would lay
>>>>>> around.
>>>>>
>>>>> I think the problem is registering a platform_device. I've complained
>>>>> about this before, but it still seems to get copied all over the
>>>>> place. Please don't do this but have a driver that looks at DT to
>>>>> figure out whether to access hardware or not.
>>>>
>>>> We did this approach but, I can remember why, someone was complaining
>>>> about it also
>>>>
>>>> The platform device/driver paradigm allowed us to split the arch
>>>> specific parts by passing the pm ops through the platform data.
>>>>
>>>> Would make sense to have a single common place for the ARM arch where we
>>>> initialize the platform device for cpuidle ?
>>>
>>> No. It's really not a device, and if you pretend that it is, you get
>>> into problems like this.
>>
>> Arnd, the problem is that the provides function pointers to the SoC code
>> that the cpuilde driver uses to call based on the idle state.
>>
>> After much discussions, we came down to using function pointers from
>> translating from DT strings, other than using that again, I dont see a
>> good way of achieving the ability of cpuidle driver staying a separate
>> driver from the SPM driver.
>>
>> Daniel, thoughts?
>
> Maybe the problem is trying too hard to separate two things that really
> belong together then. In general, the strategy of coming up with subsystems
> for a class of devices and them turning platform code into drivers for
> that subsystem has worked out really well, but I think for cpufreq, cpuidle
> and smp, it really hasn't, and in the third case we haven't even tried
> coming up with a subsystem for that reason.
>
> Having all cpuidle code generally live in drivers/cpuidle is still a good
> idea IMO, but using a platform device just for the purpose of passing
> data between some platform specific code and another platform specific
> driver hasn't worked out that well here.
At the beginning, all that become from not including mach files from the
drivers directory which make sense.
Perhaps it is time to write a similar mechanism for the cpuidle drivers
where we can still separate the low level PM code from the generic
cpuidle code.
Something like (very roughly):
Index: cpuidle-next/drivers/cpuidle/driver.c
===================================================================
--- cpuidle-next.orig/drivers/cpuidle/driver.c 2014-12-16
14:04:51.750861310 +0100
+++ cpuidle-next/drivers/cpuidle/driver.c 2014-12-16 15:09:52.202706756
+0100
@@ -178,6 +178,45 @@ static void __cpuidle_driver_init(struct
}
}
+struct cpuidle_ops_reg {
+ const char *name;
+ struct cpuidle_ops *ops;
+ struct list_head *list;
+};
+
+static LIST_HEAD(ops_list);
+
+static struct cpuidle_ops *cpuidle_find_ops(const char *name)
+{
+ struct cpuidle_ops_reg *ops_reg;
+
+ list_for_each_entry(ops_reg, &ops_list, list) {
+ if (!strcmp(ops_reg->name, name))
+ return ops_reg->ops;
+ }
+
+ return NULL;
+}
+
+int cpuidle_register_ops(const char *name, struct cpuidle_ops *ops)
+{
+ struct cpuidle_ops_reg *reg;
+
+ reg = kmalloc(sizeof(*reg), GFP_KERNEL);
+ if (!reg)
+ return -ENOMEM;
+
+ reg->name = name;
+ reg->ops = ops;
+ INIT_LIST_HEAD(®->list);
+
+ spin_lock(&cpuidle_driver_lock);
+ list_add(&ops_list, ®->list);
+ spin_unlock(&cpuidle_driver_lock);
+
+ return 0;
+}
+
/**
* __cpuidle_register_driver: register the driver
* @drv: a valid pointer to a struct cpuidle_driver
@@ -194,6 +233,7 @@ static void __cpuidle_driver_init(struct
static int __cpuidle_register_driver(struct cpuidle_driver *drv)
{
int ret;
+ struct cpuidle_ops *ops;
if (!drv || !drv->state_count)
return -EINVAL;
@@ -201,6 +241,10 @@ static int __cpuidle_register_driver(str
if (cpuidle_disabled())
return -ENODEV;
+ ops = cpuidle_find_ops(drv->name);
+ if (ops)
+ drv->ops = ops;
+
__cpuidle_driver_init(drv);
ret = __cpuidle_set_driver(drv);
Index: cpuidle-next/include/linux/cpuidle.h
===================================================================
--- cpuidle-next.orig/include/linux/cpuidle.h 2014-12-16
14:06:09.442858231 +0100
+++ cpuidle-next/include/linux/cpuidle.h 2014-12-16 14:59:46.594730753 +0100
@@ -109,11 +109,21 @@ static inline int cpuidle_get_last_resid
* CPUIDLE DRIVER INTERFACE *
****************************/
+struct cpuidle_ops {
+ int (*standby)(struct cpuidle_driver *drv,
+ struct cpuidle_device *dev);
+ int (*retention)(struct cpuidle_driver *drv,
+ struct cpuidle_device *dev);
+ int (*poweroff)(struct cpuidle_driver *drv,
+ struct cpuidle_device *dev);
+};
+
struct cpuidle_driver {
const char *name;
struct module *owner;
int refcnt;
+ struct cpuidle_ops *ops;
/* used by the cpuidle framework to setup the broadcast timer */
unsigned int bctimer:1;
/* states array must be ordered in decreasing power consumption */
--
<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
^ permalink raw reply [flat|nested] 62+ messages in thread* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-16 14:12 ` Daniel Lezcano
@ 2014-12-16 14:38 ` Arnd Bergmann
-1 siblings, 0 replies; 62+ messages in thread
From: Arnd Bergmann @ 2014-12-16 14:38 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Lina Iyer, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel, lorenzo.pieralisi, msivasub, devicetree
On Tuesday 16 December 2014 15:12:22 Daniel Lezcano wrote:
> At the beginning, all that become from not including mach files from the
> drivers directory which make sense.
>
> Perhaps it is time to write a similar mechanism for the cpuidle drivers
> where we can still separate the low level PM code from the generic
> cpuidle code.
That way you basically duplicate the same thing we already have, which
isn't much better.
In the example of drivers/soc/qcom/spm.c, just call cpuidle_register
from the spm_dev_probe() function and be done with it. You already
have a device that is responsible for handling this, don't try to
construct more than you already need.
I would assume that the same can be done for most other platforms.
There are probably cases where the same piece of hardware is responsible
for both cpuidle and cpufreq, but what that means is really that you
should have a single driver for it that does both things. Same for
SMP support: if you have one register block that does both the SMP
bringup and the cpuidle stuff, then have *one* driver for this block
that does it all. There are currently a few dependencies that require
doing SMP bringup early during boot, but we decided years ago that those
are all artificial dependencies and we should be able to boot secondary
CPUs much later than we currently do.
Arnd
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-16 14:38 ` Arnd Bergmann
0 siblings, 0 replies; 62+ messages in thread
From: Arnd Bergmann @ 2014-12-16 14:38 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday 16 December 2014 15:12:22 Daniel Lezcano wrote:
> At the beginning, all that become from not including mach files from the
> drivers directory which make sense.
>
> Perhaps it is time to write a similar mechanism for the cpuidle drivers
> where we can still separate the low level PM code from the generic
> cpuidle code.
That way you basically duplicate the same thing we already have, which
isn't much better.
In the example of drivers/soc/qcom/spm.c, just call cpuidle_register
from the spm_dev_probe() function and be done with it. You already
have a device that is responsible for handling this, don't try to
construct more than you already need.
I would assume that the same can be done for most other platforms.
There are probably cases where the same piece of hardware is responsible
for both cpuidle and cpufreq, but what that means is really that you
should have a single driver for it that does both things. Same for
SMP support: if you have one register block that does both the SMP
bringup and the cpuidle stuff, then have *one* driver for this block
that does it all. There are currently a few dependencies that require
doing SMP bringup early during boot, but we decided years ago that those
are all artificial dependencies and we should be able to boot secondary
CPUs much later than we currently do.
Arnd
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-16 14:38 ` Arnd Bergmann
@ 2014-12-16 19:18 ` Stephen Boyd
-1 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2014-12-16 19:18 UTC (permalink / raw)
To: Arnd Bergmann, Daniel Lezcano
Cc: Lina Iyer, khilman, galak, linux-arm-msm, linux-pm,
linux-arm-kernel, lorenzo.pieralisi, msivasub, devicetree
On 12/16/2014 06:38 AM, Arnd Bergmann wrote:
> On Tuesday 16 December 2014 15:12:22 Daniel Lezcano wrote:
>> At the beginning, all that become from not including mach files from the
>> drivers directory which make sense.
>>
>> Perhaps it is time to write a similar mechanism for the cpuidle drivers
>> where we can still separate the low level PM code from the generic
>> cpuidle code.
> That way you basically duplicate the same thing we already have, which
> isn't much better.
>
> In the example of drivers/soc/qcom/spm.c, just call cpuidle_register
> from the spm_dev_probe() function and be done with it. You already
> have a device that is responsible for handling this, don't try to
> construct more than you already need.
>
> I would assume that the same can be done for most other platforms.
>
> There are probably cases where the same piece of hardware is responsible
> for both cpuidle and cpufreq, but what that means is really that you
> should have a single driver for it that does both things. Same for
> SMP support: if you have one register block that does both the SMP
> bringup and the cpuidle stuff, then have *one* driver for this block
> that does it all. There are currently a few dependencies that require
> doing SMP bringup early during boot, but we decided years ago that those
> are all artificial dependencies and we should be able to boot secondary
> CPUs much later than we currently do.
>
+1. The SPM harware is used for hotplug, suspend, cpuidle, as well as
provides a regulator for a CPU, so all these things should be done in a
single driver. Booting secondary CPUs early is not hard here either if
we move the smp ops into the same driver. The only downside then is that
it can't be a module, but I would guess that we can work on making that
possible by allowing smp ops to be inserted and removed at any time.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-16 19:18 ` Stephen Boyd
0 siblings, 0 replies; 62+ messages in thread
From: Stephen Boyd @ 2014-12-16 19:18 UTC (permalink / raw)
To: linux-arm-kernel
On 12/16/2014 06:38 AM, Arnd Bergmann wrote:
> On Tuesday 16 December 2014 15:12:22 Daniel Lezcano wrote:
>> At the beginning, all that become from not including mach files from the
>> drivers directory which make sense.
>>
>> Perhaps it is time to write a similar mechanism for the cpuidle drivers
>> where we can still separate the low level PM code from the generic
>> cpuidle code.
> That way you basically duplicate the same thing we already have, which
> isn't much better.
>
> In the example of drivers/soc/qcom/spm.c, just call cpuidle_register
> from the spm_dev_probe() function and be done with it. You already
> have a device that is responsible for handling this, don't try to
> construct more than you already need.
>
> I would assume that the same can be done for most other platforms.
>
> There are probably cases where the same piece of hardware is responsible
> for both cpuidle and cpufreq, but what that means is really that you
> should have a single driver for it that does both things. Same for
> SMP support: if you have one register block that does both the SMP
> bringup and the cpuidle stuff, then have *one* driver for this block
> that does it all. There are currently a few dependencies that require
> doing SMP bringup early during boot, but we decided years ago that those
> are all artificial dependencies and we should be able to boot secondary
> CPUs much later than we currently do.
>
+1. The SPM harware is used for hotplug, suspend, cpuidle, as well as
provides a regulator for a CPU, so all these things should be done in a
single driver. Booting secondary CPUs early is not hard here either if
we move the smp ops into the same driver. The only downside then is that
it can't be a module, but I would guess that we can work on making that
possible by allowing smp ops to be inserted and removed at any time.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-16 19:18 ` Stephen Boyd
@ 2014-12-16 19:44 ` Arnd Bergmann
-1 siblings, 0 replies; 62+ messages in thread
From: Arnd Bergmann @ 2014-12-16 19:44 UTC (permalink / raw)
To: Stephen Boyd
Cc: Daniel Lezcano, Lina Iyer, khilman, galak, linux-arm-msm,
linux-pm, linux-arm-kernel, lorenzo.pieralisi, msivasub,
devicetree
On Tuesday 16 December 2014 11:18:18 Stephen Boyd wrote:
> On 12/16/2014 06:38 AM, Arnd Bergmann wrote:
> > On Tuesday 16 December 2014 15:12:22 Daniel Lezcano wrote:
> >> At the beginning, all that become from not including mach files from the
> >> drivers directory which make sense.
> >>
> >> Perhaps it is time to write a similar mechanism for the cpuidle drivers
> >> where we can still separate the low level PM code from the generic
> >> cpuidle code.
> > That way you basically duplicate the same thing we already have, which
> > isn't much better.
> >
> > In the example of drivers/soc/qcom/spm.c, just call cpuidle_register
> > from the spm_dev_probe() function and be done with it. You already
> > have a device that is responsible for handling this, don't try to
> > construct more than you already need.
> >
> > I would assume that the same can be done for most other platforms.
> >
> > There are probably cases where the same piece of hardware is responsible
> > for both cpuidle and cpufreq, but what that means is really that you
> > should have a single driver for it that does both things. Same for
> > SMP support: if you have one register block that does both the SMP
> > bringup and the cpuidle stuff, then have *one* driver for this block
> > that does it all. There are currently a few dependencies that require
> > doing SMP bringup early during boot, but we decided years ago that those
> > are all artificial dependencies and we should be able to boot secondary
> > CPUs much later than we currently do.
> >
>
> +1. The SPM harware is used for hotplug, suspend, cpuidle, as well as
> provides a regulator for a CPU, so all these things should be done in a
> single driver. Booting secondary CPUs early is not hard here either if
> we move the smp ops into the same driver. The only downside then is that
> it can't be a module, but I would guess that we can work on making that
> possible by allowing smp ops to be inserted and removed at any time.
Right, I don't see modular SMP operations happening any time soon,
but it also doesn't seem like a fundamental restriction.
Arnd
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-16 19:44 ` Arnd Bergmann
0 siblings, 0 replies; 62+ messages in thread
From: Arnd Bergmann @ 2014-12-16 19:44 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday 16 December 2014 11:18:18 Stephen Boyd wrote:
> On 12/16/2014 06:38 AM, Arnd Bergmann wrote:
> > On Tuesday 16 December 2014 15:12:22 Daniel Lezcano wrote:
> >> At the beginning, all that become from not including mach files from the
> >> drivers directory which make sense.
> >>
> >> Perhaps it is time to write a similar mechanism for the cpuidle drivers
> >> where we can still separate the low level PM code from the generic
> >> cpuidle code.
> > That way you basically duplicate the same thing we already have, which
> > isn't much better.
> >
> > In the example of drivers/soc/qcom/spm.c, just call cpuidle_register
> > from the spm_dev_probe() function and be done with it. You already
> > have a device that is responsible for handling this, don't try to
> > construct more than you already need.
> >
> > I would assume that the same can be done for most other platforms.
> >
> > There are probably cases where the same piece of hardware is responsible
> > for both cpuidle and cpufreq, but what that means is really that you
> > should have a single driver for it that does both things. Same for
> > SMP support: if you have one register block that does both the SMP
> > bringup and the cpuidle stuff, then have *one* driver for this block
> > that does it all. There are currently a few dependencies that require
> > doing SMP bringup early during boot, but we decided years ago that those
> > are all artificial dependencies and we should be able to boot secondary
> > CPUs much later than we currently do.
> >
>
> +1. The SPM harware is used for hotplug, suspend, cpuidle, as well as
> provides a regulator for a CPU, so all these things should be done in a
> single driver. Booting secondary CPUs early is not hard here either if
> we move the smp ops into the same driver. The only downside then is that
> it can't be a module, but I would guess that we can work on making that
> possible by allowing smp ops to be inserted and removed at any time.
Right, I don't see modular SMP operations happening any time soon,
but it also doesn't seem like a fundamental restriction.
Arnd
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-16 19:18 ` Stephen Boyd
@ 2014-12-17 15:22 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-17 15:22 UTC (permalink / raw)
To: Stephen Boyd
Cc: Arnd Bergmann, Daniel Lezcano, khilman, galak, linux-arm-msm,
linux-pm, linux-arm-kernel, lorenzo.pieralisi, msivasub,
devicetree
On Tue, Dec 16 2014 at 12:18 -0700, Stephen Boyd wrote:
>On 12/16/2014 06:38 AM, Arnd Bergmann wrote:
>> On Tuesday 16 December 2014 15:12:22 Daniel Lezcano wrote:
>>> At the beginning, all that become from not including mach files from the
>>> drivers directory which make sense.
>>>
>>> Perhaps it is time to write a similar mechanism for the cpuidle drivers
>>> where we can still separate the low level PM code from the generic
>>> cpuidle code.
>> That way you basically duplicate the same thing we already have, which
>> isn't much better.
>>
>> In the example of drivers/soc/qcom/spm.c, just call cpuidle_register
>> from the spm_dev_probe() function and be done with it. You already
>> have a device that is responsible for handling this, don't try to
>> construct more than you already need.
>>
>> I would assume that the same can be done for most other platforms.
>>
>> There are probably cases where the same piece of hardware is responsible
>> for both cpuidle and cpufreq, but what that means is really that you
>> should have a single driver for it that does both things. Same for
>> SMP support: if you have one register block that does both the SMP
>> bringup and the cpuidle stuff, then have *one* driver for this block
>> that does it all. There are currently a few dependencies that require
>> doing SMP bringup early during boot, but we decided years ago that those
>> are all artificial dependencies and we should be able to boot secondary
>> CPUs much later than we currently do.
>>
>
>+1. The SPM harware is used for hotplug, suspend, cpuidle, as well as
>provides a regulator for a CPU, so all these things should be done in a
>single driver. Booting secondary CPUs early is not hard here either if
>we move the smp ops into the same driver. The only downside then is that
>it can't be a module, but I would guess that we can work on making that
>possible by allowing smp ops to be inserted and removed at any time.
>
I am not sure, just because the hardware supports multiple
functionality, everything should go into the same file.
If you think of a SPM driver as something that provides a service, you
would see that all the functionality of cpuidle, hotplug, suspend, use
the service and can have their own place to share their commonality. SPM
h/w is not just for cpus. It could be a generic power controller (to an
extent) for the cpu, cache, coherenency interface and pretty much any
bus/clock exports the services of that device. There needs to be a
distinction between an entity that provides the functionality and a one
that uses it. You cannot build on additional functionality neatly if you
have them all together.
As such, I am quite appalled by the addition of the idle functions in
the SPM driver, but seems like its the direction many want, hence went
with it. But, please dont suggest using SPM driver as a dumping ground
for all functionality that SPM may indirectly influence.
Thanks,
Lina
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-17 15:22 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-17 15:22 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Dec 16 2014 at 12:18 -0700, Stephen Boyd wrote:
>On 12/16/2014 06:38 AM, Arnd Bergmann wrote:
>> On Tuesday 16 December 2014 15:12:22 Daniel Lezcano wrote:
>>> At the beginning, all that become from not including mach files from the
>>> drivers directory which make sense.
>>>
>>> Perhaps it is time to write a similar mechanism for the cpuidle drivers
>>> where we can still separate the low level PM code from the generic
>>> cpuidle code.
>> That way you basically duplicate the same thing we already have, which
>> isn't much better.
>>
>> In the example of drivers/soc/qcom/spm.c, just call cpuidle_register
>> from the spm_dev_probe() function and be done with it. You already
>> have a device that is responsible for handling this, don't try to
>> construct more than you already need.
>>
>> I would assume that the same can be done for most other platforms.
>>
>> There are probably cases where the same piece of hardware is responsible
>> for both cpuidle and cpufreq, but what that means is really that you
>> should have a single driver for it that does both things. Same for
>> SMP support: if you have one register block that does both the SMP
>> bringup and the cpuidle stuff, then have *one* driver for this block
>> that does it all. There are currently a few dependencies that require
>> doing SMP bringup early during boot, but we decided years ago that those
>> are all artificial dependencies and we should be able to boot secondary
>> CPUs much later than we currently do.
>>
>
>+1. The SPM harware is used for hotplug, suspend, cpuidle, as well as
>provides a regulator for a CPU, so all these things should be done in a
>single driver. Booting secondary CPUs early is not hard here either if
>we move the smp ops into the same driver. The only downside then is that
>it can't be a module, but I would guess that we can work on making that
>possible by allowing smp ops to be inserted and removed at any time.
>
I am not sure, just because the hardware supports multiple
functionality, everything should go into the same file.
If you think of a SPM driver as something that provides a service, you
would see that all the functionality of cpuidle, hotplug, suspend, use
the service and can have their own place to share their commonality. SPM
h/w is not just for cpus. It could be a generic power controller (to an
extent) for the cpu, cache, coherenency interface and pretty much any
bus/clock exports the services of that device. There needs to be a
distinction between an entity that provides the functionality and a one
that uses it. You cannot build on additional functionality neatly if you
have them all together.
As such, I am quite appalled by the addition of the idle functions in
the SPM driver, but seems like its the direction many want, hence went
with it. But, please dont suggest using SPM driver as a dumping ground
for all functionality that SPM may indirectly influence.
Thanks,
Lina
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-16 14:38 ` Arnd Bergmann
@ 2014-12-17 13:15 ` Daniel Lezcano
-1 siblings, 0 replies; 62+ messages in thread
From: Daniel Lezcano @ 2014-12-17 13:15 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Lina Iyer, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel, lorenzo.pieralisi, msivasub, devicetree
On 12/16/2014 03:38 PM, Arnd Bergmann wrote:
> On Tuesday 16 December 2014 15:12:22 Daniel Lezcano wrote:
>> At the beginning, all that become from not including mach files from the
>> drivers directory which make sense.
>>
>> Perhaps it is time to write a similar mechanism for the cpuidle drivers
>> where we can still separate the low level PM code from the generic
>> cpuidle code.
>
> That way you basically duplicate the same thing we already have, which
> isn't much better.
>
> In the example of drivers/soc/qcom/spm.c, just call cpuidle_register
> from the spm_dev_probe() function and be done with it. You already
> have a device that is responsible for handling this, don't try to
> construct more than you already need.
If you have to call cpuidle_register, then the cpuidle_driver should be
provided with all the idle states definition and so on.
That is exactly the opposite of what we have been doing since a couple
of years where each SoC had their own driver, in their own directory and
duplicating the code again and again from one platform to another platform.
The changes we have been through cleaned up most of the situation but
still we have more to do and I would like to prevent stepping back or
give the opportunity to step back.
I don't think we separated code which is not supposed to be. That has a
positive side effect of cleaning up the drivers.
Also, I understand your point and I am willing to solve this issue but
still by keeping the pm low level code separated from the cpuidle driver.
> I would assume that the same can be done for most other platforms.
>
> There are probably cases where the same piece of hardware is responsible
> for both cpuidle and cpufreq, but what that means is really that you
> should have a single driver for it that does both things. Same for
> SMP support: if you have one register block that does both the SMP
> bringup and the cpuidle stuff, then have *one* driver for this block
> that does it all. There are currently a few dependencies that require
> doing SMP bringup early during boot, but we decided years ago that those
> are all artificial dependencies and we should be able to boot secondary
> CPUs much later than we currently do.
I agree with this point and given the number of drivers, the easiest way
to do that is to create cpu pm ops as I gave in the previous email and
reuse them for cpu hotplug/bringup. And I believe that is possible if we
continue our approach by splitting the low level pm code from the
cpuidle driver.
What about doing something simple ? Like creating a struct
arm_cpu_pm_ops variable visible from everywhere and filled by the
different platform ?
--
<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
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-17 13:15 ` Daniel Lezcano
0 siblings, 0 replies; 62+ messages in thread
From: Daniel Lezcano @ 2014-12-17 13:15 UTC (permalink / raw)
To: linux-arm-kernel
On 12/16/2014 03:38 PM, Arnd Bergmann wrote:
> On Tuesday 16 December 2014 15:12:22 Daniel Lezcano wrote:
>> At the beginning, all that become from not including mach files from the
>> drivers directory which make sense.
>>
>> Perhaps it is time to write a similar mechanism for the cpuidle drivers
>> where we can still separate the low level PM code from the generic
>> cpuidle code.
>
> That way you basically duplicate the same thing we already have, which
> isn't much better.
>
> In the example of drivers/soc/qcom/spm.c, just call cpuidle_register
> from the spm_dev_probe() function and be done with it. You already
> have a device that is responsible for handling this, don't try to
> construct more than you already need.
If you have to call cpuidle_register, then the cpuidle_driver should be
provided with all the idle states definition and so on.
That is exactly the opposite of what we have been doing since a couple
of years where each SoC had their own driver, in their own directory and
duplicating the code again and again from one platform to another platform.
The changes we have been through cleaned up most of the situation but
still we have more to do and I would like to prevent stepping back or
give the opportunity to step back.
I don't think we separated code which is not supposed to be. That has a
positive side effect of cleaning up the drivers.
Also, I understand your point and I am willing to solve this issue but
still by keeping the pm low level code separated from the cpuidle driver.
> I would assume that the same can be done for most other platforms.
>
> There are probably cases where the same piece of hardware is responsible
> for both cpuidle and cpufreq, but what that means is really that you
> should have a single driver for it that does both things. Same for
> SMP support: if you have one register block that does both the SMP
> bringup and the cpuidle stuff, then have *one* driver for this block
> that does it all. There are currently a few dependencies that require
> doing SMP bringup early during boot, but we decided years ago that those
> are all artificial dependencies and we should be able to boot secondary
> CPUs much later than we currently do.
I agree with this point and given the number of drivers, the easiest way
to do that is to create cpu pm ops as I gave in the previous email and
reuse them for cpu hotplug/bringup. And I believe that is possible if we
continue our approach by splitting the low level pm code from the
cpuidle driver.
What about doing something simple ? Like creating a struct
arm_cpu_pm_ops variable visible from everywhere and filled by the
different platform ?
--
<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
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
2014-12-17 13:15 ` Daniel Lezcano
@ 2014-12-17 14:04 ` Lorenzo Pieralisi
-1 siblings, 0 replies; 62+ messages in thread
From: Lorenzo Pieralisi @ 2014-12-17 14:04 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Arnd Bergmann, Lina Iyer, khilman, sboyd, galak, linux-arm-msm,
linux-pm, linux-arm-kernel, msivasub, devicetree
On Wed, Dec 17, 2014 at 01:15:28PM +0000, Daniel Lezcano wrote:
[...]
> > I would assume that the same can be done for most other platforms.
> >
> > There are probably cases where the same piece of hardware is responsible
> > for both cpuidle and cpufreq, but what that means is really that you
> > should have a single driver for it that does both things. Same for
> > SMP support: if you have one register block that does both the SMP
> > bringup and the cpuidle stuff, then have *one* driver for this block
> > that does it all. There are currently a few dependencies that require
> > doing SMP bringup early during boot, but we decided years ago that those
> > are all artificial dependencies and we should be able to boot secondary
> > CPUs much later than we currently do.
>
> I agree with this point and given the number of drivers, the easiest way
> to do that is to create cpu pm ops as I gave in the previous email and
> reuse them for cpu hotplug/bringup. And I believe that is possible if we
> continue our approach by splitting the low level pm code from the
> cpuidle driver.
>
> What about doing something simple ? Like creating a struct
> arm_cpu_pm_ops variable visible from everywhere and filled by the
> different platform ?
I agree with this approach, which by the way consists in defining
cpu operations as ARM64 does and use those from cpuidle, suspend and
hotplug code. The MCPM interface does that already, probably we should
enhance/rename it since people think it must be used for multicluster
power management whereas it is a pretty generic [drivers -> mach] code
interface (I am talking about the API, not the synchronization scheme),
and can certainly be extended/refactored according to new platforms need.
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v14 03/10] qcom: spm: Add Subsystem Power Manager driver
@ 2014-12-17 14:04 ` Lorenzo Pieralisi
0 siblings, 0 replies; 62+ messages in thread
From: Lorenzo Pieralisi @ 2014-12-17 14:04 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Dec 17, 2014 at 01:15:28PM +0000, Daniel Lezcano wrote:
[...]
> > I would assume that the same can be done for most other platforms.
> >
> > There are probably cases where the same piece of hardware is responsible
> > for both cpuidle and cpufreq, but what that means is really that you
> > should have a single driver for it that does both things. Same for
> > SMP support: if you have one register block that does both the SMP
> > bringup and the cpuidle stuff, then have *one* driver for this block
> > that does it all. There are currently a few dependencies that require
> > doing SMP bringup early during boot, but we decided years ago that those
> > are all artificial dependencies and we should be able to boot secondary
> > CPUs much later than we currently do.
>
> I agree with this point and given the number of drivers, the easiest way
> to do that is to create cpu pm ops as I gave in the previous email and
> reuse them for cpu hotplug/bringup. And I believe that is possible if we
> continue our approach by splitting the low level pm code from the
> cpuidle driver.
>
> What about doing something simple ? Like creating a struct
> arm_cpu_pm_ops variable visible from everywhere and filled by the
> different platform ?
I agree with this approach, which by the way consists in defining
cpu operations as ARM64 does and use those from cpuidle, suspend and
hotplug code. The MCPM interface does that already, probably we should
enhance/rename it since people think it must be used for multicluster
power management whereas it is a pretty generic [drivers -> mach] code
interface (I am talking about the API, not the synchronization scheme),
and can certainly be extended/refactored according to new platforms need.
Thanks,
Lorenzo
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v14 04/10] arm: dts: qcom: Add power-controller device node for 8074 Krait CPUs
2014-12-02 17:39 ` Lina Iyer
@ 2014-12-02 17:39 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: daniel.lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel
Cc: lorenzo.pieralisi, msivasub, devicetree, Lina Iyer
Each Krait CPU in the QCOM 8074/8974 SoC has an SAW power controller to
regulate the power to the cpu and aide the core in entering idle states.
Reference the SAW instance and associate the instance with the CPU core.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/boot/dts/qcom-msm8974.dtsi | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index e265ec1..5a41f44 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -21,6 +21,7 @@
reg = <0>;
next-level-cache = <&L2>;
qcom,acc = <&acc0>;
+ qcom,saw = <&saw0>;
};
cpu@1 {
@@ -30,6 +31,7 @@
reg = <1>;
next-level-cache = <&L2>;
qcom,acc = <&acc1>;
+ qcom,saw = <&saw1>;
};
cpu@2 {
@@ -39,6 +41,7 @@
reg = <2>;
next-level-cache = <&L2>;
qcom,acc = <&acc2>;
+ qcom,saw = <&saw2>;
};
cpu@3 {
@@ -48,6 +51,7 @@
reg = <3>;
next-level-cache = <&L2>;
qcom,acc = <&acc3>;
+ qcom,saw = <&saw3>;
};
L2: l2-cache {
@@ -144,7 +148,27 @@
};
};
- saw_l2: regulator@f9012000 {
+ saw0: power-controller@f9089000 {
+ compatible = "qcom,msm8974-saw2-v2.1-cpu";
+ reg = <0xf9089000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw1: power-controller@f9099000 {
+ compatible = "qcom,msm8974-saw2-v2.1-cpu";
+ reg = <0xf9099000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw2: power-controller@f90a9000 {
+ compatible = "qcom,msm8974-saw2-v2.1-cpu";
+ reg = <0xf90a9000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw3: power-controller@f90b9000 {
+ compatible = "qcom,msm8974-saw2-v2.1-cpu";
+ reg = <0xf90b9000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw_l2: power-controller@f9012000 {
compatible = "qcom,saw2";
reg = <0xf9012000 0x1000>;
regulator;
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread* [PATCH v14 04/10] arm: dts: qcom: Add power-controller device node for 8074 Krait CPUs
@ 2014-12-02 17:39 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: linux-arm-kernel
Each Krait CPU in the QCOM 8074/8974 SoC has an SAW power controller to
regulate the power to the cpu and aide the core in entering idle states.
Reference the SAW instance and associate the instance with the CPU core.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/boot/dts/qcom-msm8974.dtsi | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index e265ec1..5a41f44 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -21,6 +21,7 @@
reg = <0>;
next-level-cache = <&L2>;
qcom,acc = <&acc0>;
+ qcom,saw = <&saw0>;
};
cpu at 1 {
@@ -30,6 +31,7 @@
reg = <1>;
next-level-cache = <&L2>;
qcom,acc = <&acc1>;
+ qcom,saw = <&saw1>;
};
cpu at 2 {
@@ -39,6 +41,7 @@
reg = <2>;
next-level-cache = <&L2>;
qcom,acc = <&acc2>;
+ qcom,saw = <&saw2>;
};
cpu at 3 {
@@ -48,6 +51,7 @@
reg = <3>;
next-level-cache = <&L2>;
qcom,acc = <&acc3>;
+ qcom,saw = <&saw3>;
};
L2: l2-cache {
@@ -144,7 +148,27 @@
};
};
- saw_l2: regulator at f9012000 {
+ saw0: power-controller at f9089000 {
+ compatible = "qcom,msm8974-saw2-v2.1-cpu";
+ reg = <0xf9089000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw1: power-controller at f9099000 {
+ compatible = "qcom,msm8974-saw2-v2.1-cpu";
+ reg = <0xf9099000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw2: power-controller at f90a9000 {
+ compatible = "qcom,msm8974-saw2-v2.1-cpu";
+ reg = <0xf90a9000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw3: power-controller at f90b9000 {
+ compatible = "qcom,msm8974-saw2-v2.1-cpu";
+ reg = <0xf90b9000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw_l2: power-controller at f9012000 {
compatible = "qcom,saw2";
reg = <0xf9012000 0x1000>;
regulator;
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH v14 05/10] arm: dts: qcom: Add power-controller device node for 8084 Krait CPUs
2014-12-02 17:39 ` Lina Iyer
@ 2014-12-02 17:39 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: daniel.lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel
Cc: lorenzo.pieralisi, msivasub, devicetree, Lina Iyer
Each Krait CPU in the QCOM 8084 SoC has an SAW power controller to
regulate the power to the cpu and aide the core in entering idle states.
Reference the SAW instance and associate the instance with the CPU core.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/boot/dts/qcom-apq8084.dtsi | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
index 1f130bc..71182bf 100644
--- a/arch/arm/boot/dts/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
@@ -21,6 +21,7 @@
enable-method = "qcom,kpss-acc-v2";
next-level-cache = <&L2>;
qcom,acc = <&acc0>;
+ qcom,saw = <&saw0>;
};
cpu@1 {
@@ -30,6 +31,7 @@
enable-method = "qcom,kpss-acc-v2";
next-level-cache = <&L2>;
qcom,acc = <&acc1>;
+ qcom,saw = <&saw1>;
};
cpu@2 {
@@ -39,6 +41,7 @@
enable-method = "qcom,kpss-acc-v2";
next-level-cache = <&L2>;
qcom,acc = <&acc2>;
+ qcom,saw = <&saw2>;
};
cpu@3 {
@@ -48,6 +51,7 @@
enable-method = "qcom,kpss-acc-v2";
next-level-cache = <&L2>;
qcom,acc = <&acc3>;
+ qcom,saw = <&saw3>;
};
L2: l2-cache {
@@ -144,7 +148,27 @@
};
};
- saw_l2: regulator@f9012000 {
+ saw0: power-controller@f9089000 {
+ compatible = "qcom,apq8084-saw2-v2.1-cpu";
+ reg = <0xf9089000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw1: power-controller@f9099000 {
+ compatible = "qcom,apq8084-saw2-v2.1-cpu";
+ reg = <0xf9099000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw2: power-controller@f90a9000 {
+ compatible = "qcom,apq8084-saw2-v2.1-cpu";
+ reg = <0xf90a9000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw3: power-controller@f90b9000 {
+ compatible = "qcom,apq8084-saw2-v2.1-cpu";
+ reg = <0xf90b9000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw_l2: power-controller@f9012000 {
compatible = "qcom,saw2";
reg = <0xf9012000 0x1000>;
regulator;
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread* [PATCH v14 05/10] arm: dts: qcom: Add power-controller device node for 8084 Krait CPUs
@ 2014-12-02 17:39 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: linux-arm-kernel
Each Krait CPU in the QCOM 8084 SoC has an SAW power controller to
regulate the power to the cpu and aide the core in entering idle states.
Reference the SAW instance and associate the instance with the CPU core.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/boot/dts/qcom-apq8084.dtsi | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
index 1f130bc..71182bf 100644
--- a/arch/arm/boot/dts/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
@@ -21,6 +21,7 @@
enable-method = "qcom,kpss-acc-v2";
next-level-cache = <&L2>;
qcom,acc = <&acc0>;
+ qcom,saw = <&saw0>;
};
cpu at 1 {
@@ -30,6 +31,7 @@
enable-method = "qcom,kpss-acc-v2";
next-level-cache = <&L2>;
qcom,acc = <&acc1>;
+ qcom,saw = <&saw1>;
};
cpu at 2 {
@@ -39,6 +41,7 @@
enable-method = "qcom,kpss-acc-v2";
next-level-cache = <&L2>;
qcom,acc = <&acc2>;
+ qcom,saw = <&saw2>;
};
cpu at 3 {
@@ -48,6 +51,7 @@
enable-method = "qcom,kpss-acc-v2";
next-level-cache = <&L2>;
qcom,acc = <&acc3>;
+ qcom,saw = <&saw3>;
};
L2: l2-cache {
@@ -144,7 +148,27 @@
};
};
- saw_l2: regulator at f9012000 {
+ saw0: power-controller at f9089000 {
+ compatible = "qcom,apq8084-saw2-v2.1-cpu";
+ reg = <0xf9089000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw1: power-controller at f9099000 {
+ compatible = "qcom,apq8084-saw2-v2.1-cpu";
+ reg = <0xf9099000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw2: power-controller at f90a9000 {
+ compatible = "qcom,apq8084-saw2-v2.1-cpu";
+ reg = <0xf90a9000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw3: power-controller at f90b9000 {
+ compatible = "qcom,apq8084-saw2-v2.1-cpu";
+ reg = <0xf90b9000 0x1000>, <0xf9009000 0x1000>;
+ };
+
+ saw_l2: power-controller at f9012000 {
compatible = "qcom,saw2";
reg = <0xf9012000 0x1000>;
regulator;
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH v14 06/10] arm: dts: qcom: Update power-controller device node for 8064 Krait CPUs
2014-12-02 17:39 ` Lina Iyer
@ 2014-12-02 17:39 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: daniel.lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel
Cc: lorenzo.pieralisi, msivasub, devicetree, Lina Iyer
Update the SAW2 DT bindings to add qcom,apq8064-saw2-v1.1-cpu compatible
binding string to configure SPM registers and allow the SPM to put the
core in deeper idle states when the core is idle.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/boot/dts/qcom-apq8064.dtsi | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index b3154c0..9fd24bc 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -139,26 +139,26 @@
reg = <0x020b8000 0x1000>, <0x02008000 0x1000>;
};
- saw0: regulator@2089000 {
- compatible = "qcom,saw2";
+ saw0: power-controller@2089000 {
+ compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x02089000 0x1000>, <0x02009000 0x1000>;
regulator;
};
- saw1: regulator@2099000 {
- compatible = "qcom,saw2";
+ saw1: power-controller@2099000 {
+ compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x02099000 0x1000>, <0x02009000 0x1000>;
regulator;
};
- saw2: regulator@20a9000 {
- compatible = "qcom,saw2";
+ saw2: power-controller@20a9000 {
+ compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x020a9000 0x1000>, <0x02009000 0x1000>;
regulator;
};
- saw3: regulator@20b9000 {
- compatible = "qcom,saw2";
+ saw3: power-controller@20b9000 {
+ compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x020b9000 0x1000>, <0x02009000 0x1000>;
regulator;
};
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread* [PATCH v14 06/10] arm: dts: qcom: Update power-controller device node for 8064 Krait CPUs
@ 2014-12-02 17:39 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: linux-arm-kernel
Update the SAW2 DT bindings to add qcom,apq8064-saw2-v1.1-cpu compatible
binding string to configure SPM registers and allow the SPM to put the
core in deeper idle states when the core is idle.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/boot/dts/qcom-apq8064.dtsi | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index b3154c0..9fd24bc 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -139,26 +139,26 @@
reg = <0x020b8000 0x1000>, <0x02008000 0x1000>;
};
- saw0: regulator at 2089000 {
- compatible = "qcom,saw2";
+ saw0: power-controller at 2089000 {
+ compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x02089000 0x1000>, <0x02009000 0x1000>;
regulator;
};
- saw1: regulator at 2099000 {
- compatible = "qcom,saw2";
+ saw1: power-controller at 2099000 {
+ compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x02099000 0x1000>, <0x02009000 0x1000>;
regulator;
};
- saw2: regulator at 20a9000 {
- compatible = "qcom,saw2";
+ saw2: power-controller at 20a9000 {
+ compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x020a9000 0x1000>, <0x02009000 0x1000>;
regulator;
};
- saw3: regulator at 20b9000 {
- compatible = "qcom,saw2";
+ saw3: power-controller at 20b9000 {
+ compatible = "qcom,apq8064-saw2-v1.1-cpu", "qcom,saw2";
reg = <0x020b9000 0x1000>, <0x02009000 0x1000>;
regulator;
};
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH v14 07/10] qcom: cpuidle: Add cpuidle driver for QCOM cpus
2014-12-02 17:39 ` Lina Iyer
@ 2014-12-02 17:39 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: daniel.lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel
Cc: lorenzo.pieralisi, msivasub, devicetree, Lina Iyer
Add cpuidle driver interface to allow cpus to go into idle states. Use
the cpuidle DT interface, common across ARM architectures, to provide
the idle state information to the cpuidle framework.
Supported modes at this time are Standby and Standalone Power Collapse.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
.../bindings/arm/msm/qcom,idle-state.txt | 81 ++++++++++++++++++
drivers/cpuidle/Kconfig.arm | 7 ++
drivers/cpuidle/Makefile | 1 +
drivers/cpuidle/cpuidle-qcom.c | 99 ++++++++++++++++++++++
4 files changed, 188 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt
create mode 100644 drivers/cpuidle/cpuidle-qcom.c
diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt b/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt
new file mode 100644
index 0000000..ae1b07f
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt
@@ -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
diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index 8c16ab2..e98993c 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -63,3 +63,10 @@ config ARM_MVEBU_V7_CPUIDLE
depends on ARCH_MVEBU
help
Select this to enable cpuidle on Armada 370, 38x and XP processors.
+
+config ARM_QCOM_CPUIDLE
+ bool "CPU Idle drivers for Qualcomm processors"
+ depends on ARCH_QCOM
+ select DT_IDLE_STATES
+ help
+ Select this to enable cpuidle for QCOM processors
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index 4d177b9..6c222d5 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_ARM_ZYNQ_CPUIDLE) += cpuidle-zynq.o
obj-$(CONFIG_ARM_U8500_CPUIDLE) += cpuidle-ux500.o
obj-$(CONFIG_ARM_AT91_CPUIDLE) += cpuidle-at91.o
obj-$(CONFIG_ARM_EXYNOS_CPUIDLE) += cpuidle-exynos.o
+obj-$(CONFIG_ARM_QCOM_CPUIDLE) += cpuidle-qcom.o
###############################################################################
# MIPS drivers
diff --git a/drivers/cpuidle/cpuidle-qcom.c b/drivers/cpuidle/cpuidle-qcom.c
new file mode 100644
index 0000000..aa6a6c9
--- /dev/null
+++ b/drivers/cpuidle/cpuidle-qcom.c
@@ -0,0 +1,99 @@
+/*
+ * 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)
+{
+ int ret;
+
+ ret = lpm_ops->standby(NULL);
+ if (ret)
+ return ret;
+
+ return index;
+}
+
+static int qcom_cpu_spc(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int index)
+{
+ int ret;
+
+ ret = lpm_ops->spc(NULL);
+ if (ret)
+ return ret;
+
+ 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;
+
+ /*
+ * We will not register for cpu's cpuidle device here,
+ * they will be registered as and when their power controllers
+ * are ready.
+ */
+ return cpuidle_register_driver(drv);
+}
+
+static struct platform_driver qcom_cpuidle = {
+ .probe = qcom_cpuidle_probe,
+ .driver = {
+ .name = "qcom_cpuidle",
+ },
+};
+
+/*
+ * Register the driver early so the we have a successul registration
+ * when the device shows up.
+ * This way the cpuidle driver could be registered before the cpuidle
+ * devices are registered.
+ */
+static int __init qcom_cpuidle_driver_init(void)
+{
+ return platform_driver_register(&qcom_cpuidle);
+}
+core_initcall(qcom_cpuidle_driver_init);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("CPUIDLE driver for QCOM SoC");
+MODULE_ALIAS("platform:qcom-cpuidle");
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread* [PATCH v14 07/10] qcom: cpuidle: Add cpuidle driver for QCOM cpus
@ 2014-12-02 17:39 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: linux-arm-kernel
Add cpuidle driver interface to allow cpus to go into idle states. Use
the cpuidle DT interface, common across ARM architectures, to provide
the idle state information to the cpuidle framework.
Supported modes at this time are Standby and Standalone Power Collapse.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
.../bindings/arm/msm/qcom,idle-state.txt | 81 ++++++++++++++++++
drivers/cpuidle/Kconfig.arm | 7 ++
drivers/cpuidle/Makefile | 1 +
drivers/cpuidle/cpuidle-qcom.c | 99 ++++++++++++++++++++++
4 files changed, 188 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt
create mode 100644 drivers/cpuidle/cpuidle-qcom.c
diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt b/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt
new file mode 100644
index 0000000..ae1b07f
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt
@@ -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
diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index 8c16ab2..e98993c 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -63,3 +63,10 @@ config ARM_MVEBU_V7_CPUIDLE
depends on ARCH_MVEBU
help
Select this to enable cpuidle on Armada 370, 38x and XP processors.
+
+config ARM_QCOM_CPUIDLE
+ bool "CPU Idle drivers for Qualcomm processors"
+ depends on ARCH_QCOM
+ select DT_IDLE_STATES
+ help
+ Select this to enable cpuidle for QCOM processors
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index 4d177b9..6c222d5 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_ARM_ZYNQ_CPUIDLE) += cpuidle-zynq.o
obj-$(CONFIG_ARM_U8500_CPUIDLE) += cpuidle-ux500.o
obj-$(CONFIG_ARM_AT91_CPUIDLE) += cpuidle-at91.o
obj-$(CONFIG_ARM_EXYNOS_CPUIDLE) += cpuidle-exynos.o
+obj-$(CONFIG_ARM_QCOM_CPUIDLE) += cpuidle-qcom.o
###############################################################################
# MIPS drivers
diff --git a/drivers/cpuidle/cpuidle-qcom.c b/drivers/cpuidle/cpuidle-qcom.c
new file mode 100644
index 0000000..aa6a6c9
--- /dev/null
+++ b/drivers/cpuidle/cpuidle-qcom.c
@@ -0,0 +1,99 @@
+/*
+ * 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)
+{
+ int ret;
+
+ ret = lpm_ops->standby(NULL);
+ if (ret)
+ return ret;
+
+ return index;
+}
+
+static int qcom_cpu_spc(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int index)
+{
+ int ret;
+
+ ret = lpm_ops->spc(NULL);
+ if (ret)
+ return ret;
+
+ 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;
+
+ /*
+ * We will not register for cpu's cpuidle device here,
+ * they will be registered as and when their power controllers
+ * are ready.
+ */
+ return cpuidle_register_driver(drv);
+}
+
+static struct platform_driver qcom_cpuidle = {
+ .probe = qcom_cpuidle_probe,
+ .driver = {
+ .name = "qcom_cpuidle",
+ },
+};
+
+/*
+ * Register the driver early so the we have a successul registration
+ * when the device shows up.
+ * This way the cpuidle driver could be registered before the cpuidle
+ * devices are registered.
+ */
+static int __init qcom_cpuidle_driver_init(void)
+{
+ return platform_driver_register(&qcom_cpuidle);
+}
+core_initcall(qcom_cpuidle_driver_init);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("CPUIDLE driver for QCOM SoC");
+MODULE_ALIAS("platform:qcom-cpuidle");
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH v14 08/10] arm: dts: qcom: Add idle states device nodes for 8074
2014-12-02 17:39 ` Lina Iyer
@ 2014-12-02 17:39 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: daniel.lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel
Cc: lorenzo.pieralisi, msivasub, devicetree, Lina Iyer
Add ARM common idle states device bindings for cpuidle support for APQ
8074/8974.
Support Standby and Standalone power collapse (power down that does not
affect any SoC idle states) for each cpu.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/boot/dts/qcom-msm8974.dtsi | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 5a41f44..ea31c3a 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -22,6 +22,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc0>;
qcom,saw = <&saw0>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu@1 {
@@ -32,6 +33,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc1>;
qcom,saw = <&saw1>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu@2 {
@@ -42,6 +44,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc2>;
qcom,saw = <&saw2>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu@3 {
@@ -52,6 +55,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc3>;
qcom,saw = <&saw3>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
L2: l2-cache {
@@ -59,6 +63,24 @@
cache-level = <2>;
qcom,saw = <&saw_l2>;
};
+
+ idle-states {
+ CPU_STBY: standby {
+ compatible = "qcom,idle-state-stby",
+ "arm,idle-state";
+ entry-latency-us = <1>;
+ exit-latency-us = <1>;
+ min-residency-us = <2>;
+ };
+
+ CPU_SPC: spc {
+ compatible = "qcom,idle-state-spc",
+ "arm,idle-state";
+ entry-latency-us = <150>;
+ exit-latency-us = <200>;
+ min-residency-us = <2000>;
+ };
+ };
};
cpu-pmu {
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread* [PATCH v14 08/10] arm: dts: qcom: Add idle states device nodes for 8074
@ 2014-12-02 17:39 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: linux-arm-kernel
Add ARM common idle states device bindings for cpuidle support for APQ
8074/8974.
Support Standby and Standalone power collapse (power down that does not
affect any SoC idle states) for each cpu.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/boot/dts/qcom-msm8974.dtsi | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 5a41f44..ea31c3a 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -22,6 +22,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc0>;
qcom,saw = <&saw0>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu at 1 {
@@ -32,6 +33,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc1>;
qcom,saw = <&saw1>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu at 2 {
@@ -42,6 +44,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc2>;
qcom,saw = <&saw2>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu at 3 {
@@ -52,6 +55,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc3>;
qcom,saw = <&saw3>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
L2: l2-cache {
@@ -59,6 +63,24 @@
cache-level = <2>;
qcom,saw = <&saw_l2>;
};
+
+ idle-states {
+ CPU_STBY: standby {
+ compatible = "qcom,idle-state-stby",
+ "arm,idle-state";
+ entry-latency-us = <1>;
+ exit-latency-us = <1>;
+ min-residency-us = <2>;
+ };
+
+ CPU_SPC: spc {
+ compatible = "qcom,idle-state-spc",
+ "arm,idle-state";
+ entry-latency-us = <150>;
+ exit-latency-us = <200>;
+ min-residency-us = <2000>;
+ };
+ };
};
cpu-pmu {
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH v14 09/10] arm: dts: qcom: Add idle states device nodes for 8084
2014-12-02 17:39 ` Lina Iyer
@ 2014-12-02 17:39 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: daniel.lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel
Cc: lorenzo.pieralisi, msivasub, devicetree, Lina Iyer
Add ARM common idle states device bindings for cpuidle support for APQ
8084.
Support standby and standalone power collapse (power down that does not
affect any SoC idle states) for each cpu.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/boot/dts/qcom-apq8084.dtsi | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
index 71182bf..a3c24ff 100644
--- a/arch/arm/boot/dts/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
@@ -22,6 +22,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc0>;
qcom,saw = <&saw0>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu@1 {
@@ -32,6 +33,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc1>;
qcom,saw = <&saw1>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu@2 {
@@ -42,6 +44,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc2>;
qcom,saw = <&saw2>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu@3 {
@@ -52,6 +55,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc3>;
qcom,saw = <&saw3>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
L2: l2-cache {
@@ -59,6 +63,24 @@
cache-level = <2>;
qcom,saw = <&saw_l2>;
};
+
+ idle-states {
+ CPU_STBY: standby {
+ compatible = "qcom,idle-state-stby",
+ "arm,idle-state";
+ entry-latency-us = <1>;
+ exit-latency-us = <1>;
+ min-residency-us = <2>;
+ };
+
+ CPU_SPC: spc {
+ compatible = "qcom,idle-state-spc",
+ "arm,idle-state";
+ entry-latency-us = <150>;
+ exit-latency-us = <200>;
+ min-residency-us = <2000>;
+ };
+ };
};
cpu-pmu {
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread* [PATCH v14 09/10] arm: dts: qcom: Add idle states device nodes for 8084
@ 2014-12-02 17:39 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: linux-arm-kernel
Add ARM common idle states device bindings for cpuidle support for APQ
8084.
Support standby and standalone power collapse (power down that does not
affect any SoC idle states) for each cpu.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/boot/dts/qcom-apq8084.dtsi | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
index 71182bf..a3c24ff 100644
--- a/arch/arm/boot/dts/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
@@ -22,6 +22,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc0>;
qcom,saw = <&saw0>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu at 1 {
@@ -32,6 +33,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc1>;
qcom,saw = <&saw1>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu at 2 {
@@ -42,6 +44,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc2>;
qcom,saw = <&saw2>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu at 3 {
@@ -52,6 +55,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc3>;
qcom,saw = <&saw3>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
L2: l2-cache {
@@ -59,6 +63,24 @@
cache-level = <2>;
qcom,saw = <&saw_l2>;
};
+
+ idle-states {
+ CPU_STBY: standby {
+ compatible = "qcom,idle-state-stby",
+ "arm,idle-state";
+ entry-latency-us = <1>;
+ exit-latency-us = <1>;
+ min-residency-us = <2>;
+ };
+
+ CPU_SPC: spc {
+ compatible = "qcom,idle-state-spc",
+ "arm,idle-state";
+ entry-latency-us = <150>;
+ exit-latency-us = <200>;
+ min-residency-us = <2000>;
+ };
+ };
};
cpu-pmu {
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH v14 10/10] arm: dts: qcom: Add idle state device nodes for 8064
2014-12-02 17:39 ` Lina Iyer
@ 2014-12-02 17:39 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: daniel.lezcano, khilman, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel
Cc: lorenzo.pieralisi, msivasub, devicetree, Lina Iyer
Add ARM common idle state device bindings for cpuidle support for APQ
8064.
Support Standby and Standalone power collapse (power down that does not
affect any SoC idle states) for each cpu.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/boot/dts/qcom-apq8064.dtsi | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index 9fd24bc..08893fd 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -23,6 +23,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc0>;
qcom,saw = <&saw0>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu@1 {
@@ -33,6 +34,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc1>;
qcom,saw = <&saw1>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu@2 {
@@ -43,6 +45,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc2>;
qcom,saw = <&saw2>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu@3 {
@@ -53,12 +56,31 @@
next-level-cache = <&L2>;
qcom,acc = <&acc3>;
qcom,saw = <&saw3>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
L2: l2-cache {
compatible = "cache";
cache-level = <2>;
};
+
+ idle-states {
+ CPU_STBY: standby {
+ compatible = "qcom,idle-state-stby",
+ "arm,idle-state";
+ entry-latency-us = <1>;
+ exit-latency-us = <1>;
+ min-residency-us = <2>;
+ };
+
+ CPU_SPC: spc {
+ compatible = "qcom,idle-state-spc",
+ "arm,idle-state";
+ entry-latency-us = <400>;
+ exit-latency-us = <900>;
+ min-residency-us = <3000>;
+ };
+ };
};
cpu-pmu {
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread* [PATCH v14 10/10] arm: dts: qcom: Add idle state device nodes for 8064
@ 2014-12-02 17:39 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-02 17:39 UTC (permalink / raw)
To: linux-arm-kernel
Add ARM common idle state device bindings for cpuidle support for APQ
8064.
Support Standby and Standalone power collapse (power down that does not
affect any SoC idle states) for each cpu.
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
arch/arm/boot/dts/qcom-apq8064.dtsi | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index 9fd24bc..08893fd 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -23,6 +23,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc0>;
qcom,saw = <&saw0>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu at 1 {
@@ -33,6 +34,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc1>;
qcom,saw = <&saw1>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu at 2 {
@@ -43,6 +45,7 @@
next-level-cache = <&L2>;
qcom,acc = <&acc2>;
qcom,saw = <&saw2>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
cpu at 3 {
@@ -53,12 +56,31 @@
next-level-cache = <&L2>;
qcom,acc = <&acc3>;
qcom,saw = <&saw3>;
+ cpu-idle-states = <&CPU_STBY &CPU_SPC>;
};
L2: l2-cache {
compatible = "cache";
cache-level = <2>;
};
+
+ idle-states {
+ CPU_STBY: standby {
+ compatible = "qcom,idle-state-stby",
+ "arm,idle-state";
+ entry-latency-us = <1>;
+ exit-latency-us = <1>;
+ min-residency-us = <2>;
+ };
+
+ CPU_SPC: spc {
+ compatible = "qcom,idle-state-spc",
+ "arm,idle-state";
+ entry-latency-us = <400>;
+ exit-latency-us = <900>;
+ min-residency-us = <3000>;
+ };
+ };
};
cpu-pmu {
--
2.1.0
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH v14 00/10] cpuidle driver for QCOM SoCs: 8064, 8074, 8084
2014-12-02 17:39 ` Lina Iyer
@ 2014-12-17 18:14 ` Kevin Hilman
-1 siblings, 0 replies; 62+ messages in thread
From: Kevin Hilman @ 2014-12-17 18:14 UTC (permalink / raw)
To: Lina Iyer
Cc: daniel.lezcano, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel, lorenzo.pieralisi, msivasub, devicetree
Lina Iyer <lina.iyer@linaro.org> writes:
> 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 v13:
> - Return values for idle states propagated back to the cpuidle driver.
> - Remove static bool cpuidle_drv_init.
> - Register cpuidle driver and cpuidle device separately.
> - cpuidle device registered only when the SPM for the cpu is probed.
> - Initialization changes to ensure dynamic cpuidle devices are registered only
> after the cpuidle driver is registered.
> - Removed wmb, replaced with a poll loop to ensure that the SPM registers are
> written before executing wfi.
> - Added spm_register_write_sync() for write guarantees.
> - Removed irrelevant return value for spm_set_low_power_mode().
> - Added comments, updated module description.
> - Removed Reviewed-by and Acked-by on the spm and cpuidle-qcom patches.
I think this series should also include updates to qcom_defconfig that
enable QCOM_PM and the cpuidle driver.
Then it will be enabled by default and get more broad testing.
Kevin
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v14 00/10] cpuidle driver for QCOM SoCs: 8064, 8074, 8084
@ 2014-12-17 18:14 ` Kevin Hilman
0 siblings, 0 replies; 62+ messages in thread
From: Kevin Hilman @ 2014-12-17 18:14 UTC (permalink / raw)
To: linux-arm-kernel
Lina Iyer <lina.iyer@linaro.org> writes:
> 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 v13:
> - Return values for idle states propagated back to the cpuidle driver.
> - Remove static bool cpuidle_drv_init.
> - Register cpuidle driver and cpuidle device separately.
> - cpuidle device registered only when the SPM for the cpu is probed.
> - Initialization changes to ensure dynamic cpuidle devices are registered only
> after the cpuidle driver is registered.
> - Removed wmb, replaced with a poll loop to ensure that the SPM registers are
> written before executing wfi.
> - Added spm_register_write_sync() for write guarantees.
> - Removed irrelevant return value for spm_set_low_power_mode().
> - Added comments, updated module description.
> - Removed Reviewed-by and Acked-by on the spm and cpuidle-qcom patches.
I think this series should also include updates to qcom_defconfig that
enable QCOM_PM and the cpuidle driver.
Then it will be enabled by default and get more broad testing.
Kevin
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v14 00/10] cpuidle driver for QCOM SoCs: 8064, 8074, 8084
2014-12-17 18:14 ` Kevin Hilman
@ 2014-12-17 18:25 ` Lina Iyer
-1 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-17 18:25 UTC (permalink / raw)
To: Kevin Hilman
Cc: daniel.lezcano, sboyd, galak, linux-arm-msm, linux-pm,
linux-arm-kernel, lorenzo.pieralisi, msivasub, devicetree
On Wed, Dec 17 2014 at 11:14 -0700, Kevin Hilman wrote:
>Lina Iyer <lina.iyer@linaro.org> writes:
>
>> 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 v13:
>> - Return values for idle states propagated back to the cpuidle driver.
>> - Remove static bool cpuidle_drv_init.
>> - Register cpuidle driver and cpuidle device separately.
>> - cpuidle device registered only when the SPM for the cpu is probed.
>> - Initialization changes to ensure dynamic cpuidle devices are registered only
>> after the cpuidle driver is registered.
>> - Removed wmb, replaced with a poll loop to ensure that the SPM registers are
>> written before executing wfi.
>> - Added spm_register_write_sync() for write guarantees.
>> - Removed irrelevant return value for spm_set_low_power_mode().
>> - Added comments, updated module description.
>> - Removed Reviewed-by and Acked-by on the spm and cpuidle-qcom patches.
>
>I think this series should also include updates to qcom_defconfig that
>enable QCOM_PM and the cpuidle driver.
>
>Then it will be enabled by default and get more broad testing.
Sure. I will be enabling them in the next revision of the series.
>
>Kevin
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v14 00/10] cpuidle driver for QCOM SoCs: 8064, 8074, 8084
@ 2014-12-17 18:25 ` Lina Iyer
0 siblings, 0 replies; 62+ messages in thread
From: Lina Iyer @ 2014-12-17 18:25 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Dec 17 2014 at 11:14 -0700, Kevin Hilman wrote:
>Lina Iyer <lina.iyer@linaro.org> writes:
>
>> 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 v13:
>> - Return values for idle states propagated back to the cpuidle driver.
>> - Remove static bool cpuidle_drv_init.
>> - Register cpuidle driver and cpuidle device separately.
>> - cpuidle device registered only when the SPM for the cpu is probed.
>> - Initialization changes to ensure dynamic cpuidle devices are registered only
>> after the cpuidle driver is registered.
>> - Removed wmb, replaced with a poll loop to ensure that the SPM registers are
>> written before executing wfi.
>> - Added spm_register_write_sync() for write guarantees.
>> - Removed irrelevant return value for spm_set_low_power_mode().
>> - Added comments, updated module description.
>> - Removed Reviewed-by and Acked-by on the spm and cpuidle-qcom patches.
>
>I think this series should also include updates to qcom_defconfig that
>enable QCOM_PM and the cpuidle driver.
>
>Then it will be enabled by default and get more broad testing.
Sure. I will be enabling them in the next revision of the series.
>
>Kevin
^ permalink raw reply [flat|nested] 62+ messages in thread