| Message ID | 1412718106-17049-6-git-send-email-lina.iyer@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 10/07/2014 02:41 PM, Lina Iyer wrote: > + > +static int qcom_cpuidle_probe(struct platform_device *pdev) > +{ > + struct cpuidle_driver *drv = &qcom_cpuidle_driver; > + int ret; > + > + qcom_idle_enter = pdev->dev.platform_data; > + if (!qcom_idle_enter) > + return -EFAULT; Is this ever true? Let's just drop the whole check.
On 10/07/2014 11:41 PM, Lina Iyer wrote: > Add cpuidle driver interface to allow cpus to go into C-States. Use the > cpuidle DT interface, common across ARM architectures, to provide the > C-State information to the cpuidle framework. > > Supported modes at this time are Standby and Standalone Power Collapse. Why not the retention mode which is in between the standby and the retention ? > 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 | 79 +++++++++++++++++++++ > 4 files changed, 168 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..87f1742 > --- /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 > +heirarchy to enter standby states, when all cpus are idle. An interrupt brings s/heirarchy/hierarchy/ > +the SPM state machine out of its wait, the next step is to ensure that the > +cache heirarchy is also out of standby, and then the cpu is allowed to resume s/heirarchy/hierarchy/ > +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 ^^ extra space > +system state including cache hierarchy is ready before allowing core to > +resume. Applying power and resetting the core causes the core to warmboot ^^ extra space > +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 ^^ extra space > +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 ^^ extra space > +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 ^^ extra space > +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 38cff69..6a9ee12 100644 > --- a/drivers/cpuidle/Kconfig.arm > +++ b/drivers/cpuidle/Kconfig.arm > @@ -62,3 +62,10 @@ config ARM_MVEBU_V7_CPUIDLE > depends on ARCH_MVEBU > help > Select this to enable cpuidle on Armada 370, 38x and XP processors. > + > +config ARM_QCOM_CPUIDLE > + bool "CPU Idle drivers for Qualcomm processors" > + depends on QCOM_PM + 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..0a65065 > --- /dev/null > +++ b/drivers/cpuidle/cpuidle-qcom.c > @@ -0,0 +1,79 @@ > +/* > + * 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/cpu_pm.h> > +#include <linux/cpuidle.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > + > +#include <soc/qcom/pm.h> > +#include "dt_idle_states.h" > + > +static void (*qcom_idle_enter)(enum pm_sleep_mode); > + > +static int qcom_lpm_enter_stby(struct cpuidle_device *dev, > + struct cpuidle_driver *drv, int index) > +{ > + qcom_idle_enter(PM_SLEEP_MODE_STBY); Could you replace this function by a generic one ? It would be nice to have qcom_cpu_standby(void) and qcom_cpu_powerdown(void) and let behind the mysterious words 'Single Power Collapse' in the low level code which is qcom specific :) I guess you had to create a single "qcom_idle_enter" because of a single pointer in the platform data. I am working on a common structure to be shared across the drivers as a common way to pass the different callbacks without including a soc specific header. struct cpuidle_ops { int (*standby)(void *data); int (*retention)(void *data); int (*poweroff)(void *data); }; So maybe you can either: 1. wait I post this structure and provide the driver with this one 2. create a similar structure and I will take care to upgrade when I post the patchset with the common structure. The issue I see with this common structure is the initialization of the qcom_idle_state_match array. > + local_irq_enable(); local_irq_enable() is handled by the cpuidle framework. Please remove all occurrences of this function in the driver otherwise time measurement will include irq time processing and will no longer be valid. > + return index; > +} > + > +static int qcom_lpm_enter_spc(struct cpuidle_device *dev, > + struct cpuidle_driver *drv, int index) > +{ > + cpu_pm_enter(); > + qcom_idle_enter(PM_SLEEP_MODE_SPC); Where is cpu_suspend ? > + cpu_pm_exit(); > + local_irq_enable(); > + > + 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_lpm_enter_stby}, > + { .compatible = "qcom,idle-state-spc", .data = qcom_lpm_enter_spc }, > + { }, > +}; > + > +static int qcom_cpuidle_probe(struct platform_device *pdev) > +{ > + struct cpuidle_driver *drv = &qcom_cpuidle_driver; > + int ret; > + > + qcom_idle_enter = pdev->dev.platform_data; > + if (!qcom_idle_enter) > + return -EFAULT; It shouldn't fail because if the probe is called then the cpuidle device was registered with its callback which is hardcoded. > + /* Probe for other states, including standby */ > + ret = dt_init_idle_driver(drv, qcom_idle_state_match, 0); Are you sure it is not worth to add the simple WFI state ? It may have less latency than the standby mode, no ? > + if (ret < 0) > + return ret; > + > + return cpuidle_register(drv, NULL); > +} > + > +static struct platform_driver qcom_cpuidle_plat_driver = { > + .probe = qcom_cpuidle_probe, > + .driver = { > + .name = "qcom_cpuidle", > + }, > +}; > + > +module_platform_driver(qcom_cpuidle_plat_driver);
On Thu, Oct 23, 2014 at 12:05:39PM +0100, Daniel Lezcano wrote: [...] > > diff --git a/drivers/cpuidle/cpuidle-qcom.c b/drivers/cpuidle/cpuidle-qcom.c > > new file mode 100644 > > index 0000000..0a65065 > > --- /dev/null > > +++ b/drivers/cpuidle/cpuidle-qcom.c > > @@ -0,0 +1,79 @@ > > +/* > > + * 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/cpu_pm.h> > > +#include <linux/cpuidle.h> > > +#include <linux/module.h> > > +#include <linux/platform_device.h> > > + > > +#include <soc/qcom/pm.h> > > +#include "dt_idle_states.h" > > + > > +static void (*qcom_idle_enter)(enum pm_sleep_mode); > > + > > +static int qcom_lpm_enter_stby(struct cpuidle_device *dev, > > + struct cpuidle_driver *drv, int index) > > +{ > > + qcom_idle_enter(PM_SLEEP_MODE_STBY); > > Could you replace this function by a generic one ? > > It would be nice to have qcom_cpu_standby(void) and > qcom_cpu_powerdown(void) and let behind the mysterious words 'Single > Power Collapse' in the low level code which is qcom specific :) > > I guess you had to create a single "qcom_idle_enter" because of a single > pointer in the platform data. I am working on a common structure to be > shared across the drivers as a common way to pass the different > callbacks without including a soc specific header. > > struct cpuidle_ops { > int (*standby)(void *data); > int (*retention)(void *data); > int (*poweroff)(void *data); > }; > > So maybe you can either: > > 1. wait I post this structure and provide the driver with this one > 2. create a similar structure and I will take care to upgrade when I > post the patchset with the common structure. > > The issue I see with this common structure is the initialization of the > qcom_idle_state_match array. Because you do not know which function to call when right ? That's why I think it is up to the CPUidle back-end to make that decision and why I think that the contract between the CPUidle driver and the back-end should be the idle index. Even if you have pointers to functions, the CPUidle driver do not know what parameter it has to chuck into the void data, which is likely to be platform specific too. Granted, you can make those cpuidle_ops a set of pairs, {function, parameter} associated with an idle index, but then you will end up implementing what I suggest below. If you have a look at how the MCPM wrapper works on bL driver, that's exactly the same problem. At the moment we are supporting only cluster shutdown. If we were to add a core gating idle state, how could the MCPM back-end differentiate between core and cluster states ? At the moment the only way would consist in passing the residency through mcpm_suspend() parameter. We could pass the idle state index, it is the same story. That's the reasoning behind cpu_suspend on ARM64, the index determines what should be done, it is up to the platform back end to execute the required actions (and it is not because we have PSCI on ARM64, that concept is generic and should be made similar on ARM32 IMHO). DT idle states are sorted by definition, and they can be parsed by the arch back-end too to determine the actions required by an idle state index (eg most likely information coming from power domains). The glue code on arm64 is cpu_init_idle(), which takes charge of initializing the idle state platform specific actions/parameters/etc. Everything is open to debate, as long as we nail down an interface on arm32 and we stick to that from now onwards, I do not think you are far from achieving that at all. Cheers, Lorenzo -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 23 2014 at 06:43 -0600, Lorenzo Pieralisi wrote: >On Thu, Oct 23, 2014 at 12:05:39PM +0100, Daniel Lezcano wrote: > >[...] > >> > diff --git a/drivers/cpuidle/cpuidle-qcom.c b/drivers/cpuidle/cpuidle-qcom.c >> > new file mode 100644 >> > index 0000000..0a65065 >> > --- /dev/null >> > +++ b/drivers/cpuidle/cpuidle-qcom.c >> > @@ -0,0 +1,79 @@ >> > +/* >> > + * 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/cpu_pm.h> >> > +#include <linux/cpuidle.h> >> > +#include <linux/module.h> >> > +#include <linux/platform_device.h> >> > + >> > +#include <soc/qcom/pm.h> >> > +#include "dt_idle_states.h" >> > + >> > +static void (*qcom_idle_enter)(enum pm_sleep_mode); >> > + >> > +static int qcom_lpm_enter_stby(struct cpuidle_device *dev, >> > + struct cpuidle_driver *drv, int index) >> > +{ >> > + qcom_idle_enter(PM_SLEEP_MODE_STBY); >> >> Could you replace this function by a generic one ? >> >> It would be nice to have qcom_cpu_standby(void) and >> qcom_cpu_powerdown(void) and let behind the mysterious words 'Single >> Power Collapse' in the low level code which is qcom specific :) >> >> I guess you had to create a single "qcom_idle_enter" because of a single >> pointer in the platform data. I am working on a common structure to be >> shared across the drivers as a common way to pass the different >> callbacks without including a soc specific header. >> >> struct cpuidle_ops { >> int (*standby)(void *data); >> int (*retention)(void *data); >> int (*poweroff)(void *data); >> }; >> >> So maybe you can either: >> >> 1. wait I post this structure and provide the driver with this one >> 2. create a similar structure and I will take care to upgrade when I >> post the patchset with the common structure. >> >> The issue I see with this common structure is the initialization of the >> qcom_idle_state_match array. > >Because you do not know which function to call when right ? That's why >I think it is up to the CPUidle back-end to make that decision and why >I think that the contract between the CPUidle driver and the back-end >should be the idle index. Even if you have pointers to functions, >the CPUidle driver do not know what parameter it has to chuck into >the void data, which is likely to be platform specific too. Granted, >you can make those cpuidle_ops a set of pairs, {function, parameter} >associated with an idle index, but then you will end up implementing >what I suggest below. > >If you have a look at how the MCPM wrapper works on bL driver, that's >exactly the same problem. At the moment we are supporting only cluster >shutdown. If we were to add a core gating idle state, how could the >MCPM back-end differentiate between core and cluster states ? At the >moment the only way would consist in passing the residency through >mcpm_suspend() parameter. We could pass the idle state index, it is the >same story. > >That's the reasoning behind cpu_suspend on ARM64, the index determines >what should be done, it is up to the platform back end to execute the >required actions (and it is not because we have PSCI on ARM64, that >concept is generic and should be made similar on ARM32 IMHO). > >DT idle states are sorted by definition, and they can be parsed by >the arch back-end too to determine the actions required by an idle >state index (eg most likely information coming from power domains). This is where it makes it difficult for me. QCOM SoC's can differ from each other in the states supported. For example here, I dont have retention state, while other SoC's would have it. And to have cpuidle states with increasing order of power saving (or latency), retention would creep in between standby and power collpase. This makes the mapping, a real issue to figure out what the SoC cpuidle driver chose to define and to what functions they map to. I was thinking in lines of the ops structure, which could work for PSCI as well. > >The glue code on arm64 is cpu_init_idle(), which takes charge of >initializing the idle state platform specific actions/parameters/etc. > >Everything is open to debate, as long as we nail down an interface on >arm32 and we stick to that from now onwards, I do not think you are far >from achieving that at all. > Thanks, I will try and get the QCOM idle drivers conform to the common ideas. >Cheers, >Lorenzo > -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 23 2014 at 05:05 -0600, Daniel Lezcano wrote: >On 10/07/2014 11:41 PM, Lina Iyer wrote: >>Add cpuidle driver interface to allow cpus to go into C-States. Use the >>cpuidle DT interface, common across ARM architectures, to provide the >>C-State information to the cpuidle framework. >> >>Supported modes at this time are Standby and Standalone Power Collapse. > >Why not the retention mode which is in between the standby and the >retention ? > Retention would appear 'hacky' in comparision to these code, and has dependencies on clocks. So at some point, yes, I will submit a patch to address this deficiency. >diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm >>index 38cff69..6a9ee12 100644 >>--- a/drivers/cpuidle/Kconfig.arm >>+++ b/drivers/cpuidle/Kconfig.arm >>@@ -62,3 +62,10 @@ config ARM_MVEBU_V7_CPUIDLE >> depends on ARCH_MVEBU >> help >> Select this to enable cpuidle on Armada 370, 38x and XP processors. >>+ >>+config ARM_QCOM_CPUIDLE >>+ bool "CPU Idle drivers for Qualcomm processors" >>+ depends on QCOM_PM > >+ depends on ARCH_QCOM > I may have removed it, which I will check, QCOM_PM used to depend on ARCH_QCOM >+ >>+static void (*qcom_idle_enter)(enum pm_sleep_mode); >>+ >>+static int qcom_lpm_enter_stby(struct cpuidle_device *dev, >>+ struct cpuidle_driver *drv, int index) >>+{ >>+ qcom_idle_enter(PM_SLEEP_MODE_STBY); > >Could you replace this function by a generic one ? > >It would be nice to have qcom_cpu_standby(void) and >qcom_cpu_powerdown(void) and let behind the mysterious words 'Single >Power Collapse' in the low level code which is qcom specific :) > >I guess you had to create a single "qcom_idle_enter" because of a >single pointer in the platform data. I am working on a common >structure to be shared across the drivers as a common way to pass the >different callbacks without including a soc specific header. > >struct cpuidle_ops { > int (*standby)(void *data); > int (*retention)(void *data); > int (*poweroff)(void *data); >}; > >So maybe you can either: > >1. wait I post this structure and provide the driver with this one >2. create a similar structure and I will take care to upgrade when I >post the patchset with the common structure. > >The issue I see with this common structure is the initialization of >the qcom_idle_state_match array. > >>+ local_irq_enable(); > >local_irq_enable() is handled by the cpuidle framework. >Please remove all occurrences of this function in the driver otherwise >time measurement will include irq time processing and will no longer >be valid. OK. Thanks for pointing that out. > >>+ return index; >>+} >>+ >>+static int qcom_lpm_enter_spc(struct cpuidle_device *dev, >>+ struct cpuidle_driver *drv, int index) >>+{ >>+ cpu_pm_enter(); >>+ qcom_idle_enter(PM_SLEEP_MODE_SPC); > >Where is cpu_suspend ? > Inside that function qcom_idle_enter() in the SoC layer (pm.c) >>+ cpu_pm_exit(); >>+ local_irq_enable(); >>+ >>+ 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_lpm_enter_stby}, >>+ { .compatible = "qcom,idle-state-spc", .data = qcom_lpm_enter_spc }, >>+ { }, >>+}; >>+ >>+static int qcom_cpuidle_probe(struct platform_device *pdev) >>+{ >>+ struct cpuidle_driver *drv = &qcom_cpuidle_driver; >>+ int ret; >>+ >>+ qcom_idle_enter = pdev->dev.platform_data; >>+ if (!qcom_idle_enter) >>+ return -EFAULT; > >It shouldn't fail because if the probe is called then the cpuidle >device was registered with its callback which is hardcoded. > Yeah, I see the paradigm shift. Even though I know how the caller is, I am always backing up the expectation with an error check. Will remove that. >>+ /* Probe for other states, including standby */ >>+ ret = dt_init_idle_driver(drv, qcom_idle_state_match, 0); > >Are you sure it is not worth to add the simple WFI state ? It may have >less latency than the standby mode, no ? > May be. But it would split the bucket between wfi and the cpu plus allowing the L2 and the power hierarachy to enter their standby states. This could very well be useful and possible, when there is a QoS request that disallows power down and high latency states. IMO, the benefit of the possible heirarchical standby seems to outweigh the latency gain we may get by just doing a core's clock gating. >>+ if (ret < 0) >>+ return ret; >>+ >>+ return cpuidle_register(drv, NULL); >>+} >>+ >>+static struct platform_driver qcom_cpuidle_plat_driver = { >>+ .probe = qcom_cpuidle_probe, >>+ .driver = { >>+ .name = "qcom_cpuidle", >>+ }, >>+}; >>+ >>+module_platform_driver(qcom_cpuidle_plat_driver); > > >-- > <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs > >Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | ><http://twitter.com/#!/linaroorg> Twitter | ><http://www.linaro.org/linaro-blog/> Blog > -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/23/2014 06:58 PM, Lina Iyer wrote: > On Thu, Oct 23 2014 at 05:05 -0600, Daniel Lezcano wrote: >> On 10/07/2014 11:41 PM, Lina Iyer wrote: >>> Add cpuidle driver interface to allow cpus to go into C-States. Use the >>> cpuidle DT interface, common across ARM architectures, to provide the >>> C-State information to the cpuidle framework. >>> >>> Supported modes at this time are Standby and Standalone Power Collapse. >> >> Why not the retention mode which is in between the standby and the >> retention ? >> > Retention would appear 'hacky' in comparision to these code, and has > dependencies on clocks. So at some point, yes, I will submit a patch to > address this deficiency. >> diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm >>> index 38cff69..6a9ee12 100644 >>> --- a/drivers/cpuidle/Kconfig.arm >>> +++ b/drivers/cpuidle/Kconfig.arm >>> @@ -62,3 +62,10 @@ config ARM_MVEBU_V7_CPUIDLE >>> depends on ARCH_MVEBU >>> help >>> Select this to enable cpuidle on Armada 370, 38x and XP >>> processors. >>> + >>> +config ARM_QCOM_CPUIDLE >>> + bool "CPU Idle drivers for Qualcomm processors" >>> + depends on QCOM_PM >> >> + depends on ARCH_QCOM >> > I may have removed it, which I will check, QCOM_PM used to depend on > ARCH_QCOM If QCOM_PM depends on ARCH_QCOM, then yes you can remove the QCOM_PM dep. >>> +static void (*qcom_idle_enter)(enum pm_sleep_mode); >>> + >>> +static int qcom_lpm_enter_stby(struct cpuidle_device *dev, >>> + struct cpuidle_driver *drv, int index) >>> +{ >>> + qcom_idle_enter(PM_SLEEP_MODE_STBY); >> >> Could you replace this function by a generic one ? >> >> It would be nice to have qcom_cpu_standby(void) and >> qcom_cpu_powerdown(void) and let behind the mysterious words 'Single >> Power Collapse' in the low level code which is qcom specific :) >> >> I guess you had to create a single "qcom_idle_enter" because of a >> single pointer in the platform data. I am working on a common >> structure to be shared across the drivers as a common way to pass the >> different callbacks without including a soc specific header. >> >> struct cpuidle_ops { >> int (*standby)(void *data); >> int (*retention)(void *data); >> int (*poweroff)(void *data); >> }; >> >> So maybe you can either: >> >> 1. wait I post this structure and provide the driver with this one >> 2. create a similar structure and I will take care to upgrade when I >> post the patchset with the common structure. >> >> The issue I see with this common structure is the initialization of >> the qcom_idle_state_match array. >> >>> + local_irq_enable(); >> >> local_irq_enable() is handled by the cpuidle framework. >> Please remove all occurrences of this function in the driver otherwise >> time measurement will include irq time processing and will no longer >> be valid. > OK. Thanks for pointing that out. > >> >>> + return index; >>> +} >>> + >>> +static int qcom_lpm_enter_spc(struct cpuidle_device *dev, >>> + struct cpuidle_driver *drv, int index) >>> +{ >>> + cpu_pm_enter(); >>> + qcom_idle_enter(PM_SLEEP_MODE_SPC); >> >> Where is cpu_suspend ? >> > Inside that function qcom_idle_enter() in the SoC layer (pm.c) It would be preferable to group cpu_suspend with cpu_pm_enter/exit in this function. >>> + cpu_pm_exit(); >>> + local_irq_enable(); >>> + >>> + 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_lpm_enter_stby}, >>> + { .compatible = "qcom,idle-state-spc", .data = >>> qcom_lpm_enter_spc }, >>> + { }, >>> +}; >>> + >>> +static int qcom_cpuidle_probe(struct platform_device *pdev) >>> +{ >>> + struct cpuidle_driver *drv = &qcom_cpuidle_driver; >>> + int ret; >>> + >>> + qcom_idle_enter = pdev->dev.platform_data; >>> + if (!qcom_idle_enter) >>> + return -EFAULT; >> >> It shouldn't fail because if the probe is called then the cpuidle >> device was registered with its callback which is hardcoded. >> > Yeah, I see the paradigm shift. Even though I know how the caller is, I > am always backing up the expectation with an error check. Will remove > that. > >>> + /* Probe for other states, including standby */ >>> + ret = dt_init_idle_driver(drv, qcom_idle_state_match, 0); >> >> Are you sure it is not worth to add the simple WFI state ? It may have >> less latency than the standby mode, no ? >> > May be. But it would split the bucket between wfi and the cpu plus > allowing the L2 and the power hierarachy to enter their standby states. > This could very well be useful and possible, when there is a QoS request > that disallows power down and high latency states. Not necessarly a QoS, it could be a state selection from the governor with very short latencies. > IMO, the benefit of the possible heirarchical standby seems to outweigh the > latency gain we may get by just doing a core's clock gating. It is up to the governor/scheduler to figure out this :) >>> + if (ret < 0) >>> + return ret; >>> + >>> + return cpuidle_register(drv, NULL); >>> +} >>> + >>> +static struct platform_driver qcom_cpuidle_plat_driver = { >>> + .probe = qcom_cpuidle_probe, >>> + .driver = { >>> + .name = "qcom_cpuidle", >>> + }, >>> +}; >>> + >>> +module_platform_driver(qcom_cpuidle_plat_driver); >> >> >> -- >> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs >> >> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | >> <http://twitter.com/#!/linaroorg> Twitter | >> <http://www.linaro.org/linaro-blog/> Blog >>
On 10/23/2014 02:43 PM, Lorenzo Pieralisi wrote: > On Thu, Oct 23, 2014 at 12:05:39PM +0100, Daniel Lezcano wrote: > > [...] > >>> diff --git a/drivers/cpuidle/cpuidle-qcom.c b/drivers/cpuidle/cpuidle-qcom.c >>> new file mode 100644 >>> index 0000000..0a65065 >>> --- /dev/null >>> +++ b/drivers/cpuidle/cpuidle-qcom.c >>> @@ -0,0 +1,79 @@ >>> +/* >>> + * 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/cpu_pm.h> >>> +#include <linux/cpuidle.h> >>> +#include <linux/module.h> >>> +#include <linux/platform_device.h> >>> + >>> +#include <soc/qcom/pm.h> >>> +#include "dt_idle_states.h" >>> + >>> +static void (*qcom_idle_enter)(enum pm_sleep_mode); >>> + >>> +static int qcom_lpm_enter_stby(struct cpuidle_device *dev, >>> + struct cpuidle_driver *drv, int index) >>> +{ >>> + qcom_idle_enter(PM_SLEEP_MODE_STBY); >> >> Could you replace this function by a generic one ? >> >> It would be nice to have qcom_cpu_standby(void) and >> qcom_cpu_powerdown(void) and let behind the mysterious words 'Single >> Power Collapse' in the low level code which is qcom specific :) >> >> I guess you had to create a single "qcom_idle_enter" because of a single >> pointer in the platform data. I am working on a common structure to be >> shared across the drivers as a common way to pass the different >> callbacks without including a soc specific header. >> >> struct cpuidle_ops { >> int (*standby)(void *data); >> int (*retention)(void *data); >> int (*poweroff)(void *data); >> }; >> >> So maybe you can either: >> >> 1. wait I post this structure and provide the driver with this one >> 2. create a similar structure and I will take care to upgrade when I >> post the patchset with the common structure. >> >> The issue I see with this common structure is the initialization of the >> qcom_idle_state_match array. > > Because you do not know which function to call when right ? Hi Lorenzo, that's correct. Perhaps an alternative could be to use the 'weak' attribute instead of a structure and define the different ops like the cpu_do_idle() does. > That's why > I think it is up to the CPUidle back-end to make that decision and why > I think that the contract between the CPUidle driver and the back-end > should be the idle index. Even if you have pointers to functions, > the CPUidle driver do not know what parameter it has to chuck into > the void data, which is likely to be platform specific too. Granted, > you can make those cpuidle_ops a set of pairs, {function, parameter} > associated with an idle index, but then you will end up implementing > what I suggest below. I don't see where is the problem if the backend driver passes the different CPU PM ops. If the cpuidle driver and the backend PM code want to pass an index as 'data', I don't like that but it is ok. But I don't want to provide the index as part of the API. We can see the mess when the code began to play with the indexes as a specific idle state (cf. CPUIDLE_DRIVER_STATE_START magic). > If you have a look at how the MCPM wrapper works on bL driver, that's > exactly the same problem. At the moment we are supporting only cluster > shutdown. If we were to add a core gating idle state, how could the > MCPM back-end differentiate between core and cluster states ? At the > moment the only way would consist in passing the residency through > mcpm_suspend() parameter. We could pass the idle state index, it is the > same story. > > That's the reasoning behind cpu_suspend on ARM64, the index determines > what should be done, it is up to the platform back end to execute the > required actions (and it is not because we have PSCI on ARM64, that > concept is generic and should be made similar on ARM32 IMHO). > > DT idle states are sorted by definition, and they can be parsed by > the arch back-end too to determine the actions required by an idle > state index (eg most likely information coming from power domains). > > The glue code on arm64 is cpu_init_idle(), which takes charge of > initializing the idle state platform specific actions/parameters/etc. > > Everything is open to debate, as long as we nail down an interface on > arm32 and we stick to that from now onwards, I do not think you are far > from achieving that at all.
On Fri, Oct 24, 2014 at 09:56:35AM +0100, Daniel Lezcano wrote: > On 10/23/2014 02:43 PM, Lorenzo Pieralisi wrote: > > On Thu, Oct 23, 2014 at 12:05:39PM +0100, Daniel Lezcano wrote: > > > > [...] > > > >>> diff --git a/drivers/cpuidle/cpuidle-qcom.c b/drivers/cpuidle/cpuidle-qcom.c > >>> new file mode 100644 > >>> index 0000000..0a65065 > >>> --- /dev/null > >>> +++ b/drivers/cpuidle/cpuidle-qcom.c > >>> @@ -0,0 +1,79 @@ > >>> +/* > >>> + * 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/cpu_pm.h> > >>> +#include <linux/cpuidle.h> > >>> +#include <linux/module.h> > >>> +#include <linux/platform_device.h> > >>> + > >>> +#include <soc/qcom/pm.h> > >>> +#include "dt_idle_states.h" > >>> + > >>> +static void (*qcom_idle_enter)(enum pm_sleep_mode); > >>> + > >>> +static int qcom_lpm_enter_stby(struct cpuidle_device *dev, > >>> + struct cpuidle_driver *drv, int index) > >>> +{ > >>> + qcom_idle_enter(PM_SLEEP_MODE_STBY); > >> > >> Could you replace this function by a generic one ? > >> > >> It would be nice to have qcom_cpu_standby(void) and > >> qcom_cpu_powerdown(void) and let behind the mysterious words 'Single > >> Power Collapse' in the low level code which is qcom specific :) > >> > >> I guess you had to create a single "qcom_idle_enter" because of a single > >> pointer in the platform data. I am working on a common structure to be > >> shared across the drivers as a common way to pass the different > >> callbacks without including a soc specific header. > >> > >> struct cpuidle_ops { > >> int (*standby)(void *data); > >> int (*retention)(void *data); > >> int (*poweroff)(void *data); > >> }; > >> > >> So maybe you can either: > >> > >> 1. wait I post this structure and provide the driver with this one > >> 2. create a similar structure and I will take care to upgrade when I > >> post the patchset with the common structure. > >> > >> The issue I see with this common structure is the initialization of the > >> qcom_idle_state_match array. > > > > Because you do not know which function to call when right ? > > Hi Lorenzo, > > that's correct. > > Perhaps an alternative could be to use the 'weak' attribute instead of a > structure and define the different ops like the cpu_do_idle() does. Do you mean arch_cpu_idle() ? I still do not see how the CPUidle driver can make out what function to call when, unless it is the driver that initializes those functions itself, I think we should see how it works with an example. > > That's why > > I think it is up to the CPUidle back-end to make that decision and why > > I think that the contract between the CPUidle driver and the back-end > > should be the idle index. Even if you have pointers to functions, > > the CPUidle driver do not know what parameter it has to chuck into > > the void data, which is likely to be platform specific too. Granted, > > you can make those cpuidle_ops a set of pairs, {function, parameter} > > associated with an idle index, but then you will end up implementing > > what I suggest below. > > I don't see where is the problem if the backend driver passes the > different CPU PM ops. If the cpuidle driver and the backend PM code want > to pass an index as 'data', I don't like that but it is ok. But I don't > want to provide the index as part of the API. > > We can see the mess when the code began to play with the indexes as a > specific idle state (cf. CPUIDLE_DRIVER_STATE_START magic). I think that it can be done, but I also agree that there is a lot of legacy and you are looking for an interface to unify the idle drivers, inclusive of legacy ones so that's convoluted. As I mentioned below, we need a solution that works well on the majority of idle drivers, inclusive of bL one MCPM based, otherwise things get out of control. I already have to update the bL driver for core gating states, and in the process that requires a clean interface between MCPM calls and the backend, which at the moment is based on passing the state residency. We can't deploy different solutions to the same problem, that's my point. Thanks, Lorenzo > > If you have a look at how the MCPM wrapper works on bL driver, that's > > exactly the same problem. At the moment we are supporting only cluster > > shutdown. If we were to add a core gating idle state, how could the > > MCPM back-end differentiate between core and cluster states ? At the > > moment the only way would consist in passing the residency through > > mcpm_suspend() parameter. We could pass the idle state index, it is the > > same story. > > > > That's the reasoning behind cpu_suspend on ARM64, the index determines > > what should be done, it is up to the platform back end to execute the > > required actions (and it is not because we have PSCI on ARM64, that > > concept is generic and should be made similar on ARM32 IMHO). > > > > DT idle states are sorted by definition, and they can be parsed by > > the arch back-end too to determine the actions required by an idle > > state index (eg most likely information coming from power domains). > > > > The glue code on arm64 is cpu_init_idle(), which takes charge of > > initializing the idle state platform specific actions/parameters/etc. > > > > Everything is open to debate, as long as we nail down an interface on > > arm32 and we stick to that from now onwards, I do not think you are far > > from achieving that at all. > > > > > -- > <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs > > Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | > <http://twitter.com/#!/linaroorg> Twitter | > <http://www.linaro.org/linaro-blog/> Blog > > -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 24 2014 at 02:42 -0600, Daniel Lezcano wrote: >On 10/23/2014 06:58 PM, Lina Iyer wrote: >>On Thu, Oct 23 2014 at 05:05 -0600, Daniel Lezcano wrote: >>>On 10/07/2014 11:41 PM, Lina Iyer wrote: >>>+static int qcom_lpm_enter_spc(struct cpuidle_device *dev, >>>>+ struct cpuidle_driver *drv, int index) >>>>+{ >>>>+ cpu_pm_enter(); >>>>+ qcom_idle_enter(PM_SLEEP_MODE_SPC); >>> >>>Where is cpu_suspend ? >>> >>Inside that function qcom_idle_enter() in the SoC layer (pm.c) > >It would be preferable to group cpu_suspend with cpu_pm_enter/exit in >this function. > OK. Sure. >>>+ /* Probe for other states, including standby */ >>>>+ ret = dt_init_idle_driver(drv, qcom_idle_state_match, 0); >>> >>>Are you sure it is not worth to add the simple WFI state ? It may have >>>less latency than the standby mode, no ? >>> >>May be. But it would split the bucket between wfi and the cpu plus >>allowing the L2 and the power hierarachy to enter their standby states. >>This could very well be useful and possible, when there is a QoS request >>that disallows power down and high latency states. > >Not necessarly a QoS, it could be a state selection from the governor >with very short latencies. > >>IMO, the benefit of the possible heirarchical standby seems to outweigh the >>latency gain we may get by just doing a core's clock gating. > >It is up to the governor/scheduler to figure out this :) > There is a bit more problem than that, which I should have mentioned earlier. If we have SPM available and configured for the SoC, then unless we explictly disable the SPM, the WFI instruction executed by the cpu core would trigger SPM state machine and since we decided to set up the SPM state machine to do its SPM sequence for standby only when we choose standby in cpuidle, SPM might remain configured to do the previous idle state and as such the WFI instruction from the core would do that (possibly deeper), uintentionally, even when the scheduler decided only to do cpu clock gating. So we have to disable SPM everytime, per our pattern, if we want to do that, that means, we still have to enter the SoC specific code, and cannot get-by with the WFI instruction alone. Lina -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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..87f1742 --- /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 +heirarchy 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 heirarchy 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 38cff69..6a9ee12 100644 --- a/drivers/cpuidle/Kconfig.arm +++ b/drivers/cpuidle/Kconfig.arm @@ -62,3 +62,10 @@ config ARM_MVEBU_V7_CPUIDLE depends on ARCH_MVEBU help Select this to enable cpuidle on Armada 370, 38x and XP processors. + +config ARM_QCOM_CPUIDLE + bool "CPU Idle drivers for Qualcomm processors" + depends on QCOM_PM + select DT_IDLE_STATES + help + Select this to enable cpuidle for QCOM processors 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..0a65065 --- /dev/null +++ b/drivers/cpuidle/cpuidle-qcom.c @@ -0,0 +1,79 @@ +/* + * 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/cpu_pm.h> +#include <linux/cpuidle.h> +#include <linux/module.h> +#include <linux/platform_device.h> + +#include <soc/qcom/pm.h> +#include "dt_idle_states.h" + +static void (*qcom_idle_enter)(enum pm_sleep_mode); + +static int qcom_lpm_enter_stby(struct cpuidle_device *dev, + struct cpuidle_driver *drv, int index) +{ + qcom_idle_enter(PM_SLEEP_MODE_STBY); + local_irq_enable(); + + return index; +} + +static int qcom_lpm_enter_spc(struct cpuidle_device *dev, + struct cpuidle_driver *drv, int index) +{ + cpu_pm_enter(); + qcom_idle_enter(PM_SLEEP_MODE_SPC); + cpu_pm_exit(); + local_irq_enable(); + + 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_lpm_enter_stby}, + { .compatible = "qcom,idle-state-spc", .data = qcom_lpm_enter_spc }, + { }, +}; + +static int qcom_cpuidle_probe(struct platform_device *pdev) +{ + struct cpuidle_driver *drv = &qcom_cpuidle_driver; + int ret; + + qcom_idle_enter = pdev->dev.platform_data; + if (!qcom_idle_enter) + return -EFAULT; + + /* Probe for other states, including standby */ + ret = dt_init_idle_driver(drv, qcom_idle_state_match, 0); + if (ret < 0) + return ret; + + return cpuidle_register(drv, NULL); +} + +static struct platform_driver qcom_cpuidle_plat_driver = { + .probe = qcom_cpuidle_probe, + .driver = { + .name = "qcom_cpuidle", + }, +}; + +module_platform_driver(qcom_cpuidle_plat_driver);
Add cpuidle driver interface to allow cpus to go into C-States. Use the cpuidle DT interface, common across ARM architectures, to provide the C-State information to the cpuidle framework. Supported modes at this time are Standby and Standalone Power Collapse. Signed-off-by: Lina Iyer <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 | 79 +++++++++++++++++++++ 4 files changed, 168 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/msm/qcom,idle-state.txt create mode 100644 drivers/cpuidle/cpuidle-qcom.c