Re: [PATCH v8 4/8] ARM: davinci: Consolidate time keeping and irq enable
Sekhar tested this patch on Davinci last night and found a problem. I
looked at the code again and found a mindless omission on my part (see
below). Fix is trivial. I've check all other platforms and confirmed
this problem does not exist for those. Will resend a v9 of the
patchset shortly.
On Tue, Mar 20, 2012 at 3:22 PM, Robert Lee rob@linaro.org wrote:
Enable core cpuidle timekeeping and irq enabling and remove that
handling from this code.
Signed-off-by: Robert Lee rob@linaro.org
Reviewed-by: Kevin Hilman khil...@ti.com
Reviewed-by: Daniel Lezcano daniel.lezc...@linaro.org
Acked-by: Jean Pihetj-pi...@ti.com
---
arch/arm/mach-davinci/cpuidle.c | 82
---
1 file changed, 33 insertions(+), 49 deletions(-)
diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index a30c7c5..93ae096 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -18,6 +18,7 @@
#include linux/io.h
#include linux/export.h
#include asm/proc-fns.h
+#include asm/cpuidle.h
#include mach/cpuidle.h
#include mach/ddr2.h
@@ -30,12 +31,42 @@ struct davinci_ops {
u32 flags;
};
+/* Actual code that puts the SoC in different idle states */
+static int davinci_enter_idle(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv,
+ int index)
+{
+ struct cpuidle_state_usage *state_usage = dev-states_usage[index];
+ struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
+
+ if (ops ops-enter)
+ ops-enter(ops-flags);
+
+ index = cpuidle_wrap_enter(dev, drv, index,
+ arm_cpuidle_simple_enter);
+
+ if (ops ops-exit)
+ ops-exit(ops-flags);
+
+ return index;
+}
+
/* fields in davinci_ops.flags */
#define DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN BIT(0)
static struct cpuidle_driver davinci_idle_driver = {
- .name = cpuidle-davinci,
- .owner = THIS_MODULE,
+ .name = cpuidle-davinci,
+ .owner = THIS_MODULE,
Argh! I am missing the .en_core_tk_irqen = 1 here.
+ .states[0] = ARM_CPUIDLE_WFI_STATE,
+ .states[1] = {
+ .enter = davinci_enter_idle,
+ .exit_latency = 10,
+ .target_residency = 10,
+ .flags = CPUIDLE_FLAG_TIME_VALID,
+ .name = DDR SR,
+ .desc = WFI and DDR Self Refresh,
+ },
+ .state_count = DAVINCI_CPUIDLE_MAX_STATES,
};
static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device);
@@ -77,41 +108,10 @@ static struct davinci_ops
davinci_states[DAVINCI_CPUIDLE_MAX_STATES] = {
},
};
-/* Actual code that puts the SoC in different idle states */
-static int davinci_enter_idle(struct cpuidle_device *dev,
- struct cpuidle_driver *drv,
- int index)
-{
- struct cpuidle_state_usage *state_usage = dev-states_usage[index];
- struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
- struct timeval before, after;
- int idle_time;
-
- local_irq_disable();
- do_gettimeofday(before);
-
- if (ops ops-enter)
- ops-enter(ops-flags);
- /* Wait for interrupt state */
- cpu_do_idle();
- if (ops ops-exit)
- ops-exit(ops-flags);
-
- do_gettimeofday(after);
- local_irq_enable();
- idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
- (after.tv_usec - before.tv_usec);
-
- dev-last_residency = idle_time;
-
- return index;
-}
-
static int __init davinci_cpuidle_probe(struct platform_device *pdev)
{
int ret;
struct cpuidle_device *device;
- struct cpuidle_driver *driver = davinci_idle_driver;
struct davinci_cpuidle_config *pdata = pdev-dev.platform_data;
device = per_cpu(davinci_cpuidle_device, smp_processor_id());
@@ -123,27 +123,11 @@ static int __init davinci_cpuidle_probe(struct
platform_device *pdev)
ddr2_reg_base = pdata-ddr2_ctlr_base;
- /* Wait for interrupt state */
- driver-states[0].enter = davinci_enter_idle;
- driver-states[0].exit_latency = 1;
- driver-states[0].target_residency = 1;
- driver-states[0].flags = CPUIDLE_FLAG_TIME_VALID;
- strcpy(driver-states[0].name, WFI);
- strcpy(driver-states[0].desc, Wait for interrupt);
-
- /* Wait for interrupt and DDR self refresh state */
- driver-states[1].enter = davinci_enter_idle;
- driver-states[1].exit_latency = 10;
- driver-states[1].target_residency = 1;
-
Re: [PATCH v8 4/8] ARM: davinci: Consolidate time keeping and irq enable
On Wed, Mar 21, 2012 at 9:28 AM, Rob Lee rob@linaro.org wrote:
Sekhar tested this patch on Davinci last night and found a problem. I
looked at the code again and found a mindless omission on my part (see
below). Fix is trivial. I've check all other platforms and confirmed
this problem does not exist for those. Will resend a v9 of the
patchset shortly.
Kevin just informed me that Len already has v8 patchset queued so I'll
just send this fix out as a separate patch.
On Tue, Mar 20, 2012 at 3:22 PM, Robert Lee rob@linaro.org wrote:
Enable core cpuidle timekeeping and irq enabling and remove that
handling from this code.
Signed-off-by: Robert Lee rob@linaro.org
Reviewed-by: Kevin Hilman khil...@ti.com
Reviewed-by: Daniel Lezcano daniel.lezc...@linaro.org
Acked-by: Jean Pihetj-pi...@ti.com
---
arch/arm/mach-davinci/cpuidle.c | 82
---
1 file changed, 33 insertions(+), 49 deletions(-)
diff --git a/arch/arm/mach-davinci/cpuidle.c
b/arch/arm/mach-davinci/cpuidle.c
index a30c7c5..93ae096 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -18,6 +18,7 @@
#include linux/io.h
#include linux/export.h
#include asm/proc-fns.h
+#include asm/cpuidle.h
#include mach/cpuidle.h
#include mach/ddr2.h
@@ -30,12 +31,42 @@ struct davinci_ops {
u32 flags;
};
+/* Actual code that puts the SoC in different idle states */
+static int davinci_enter_idle(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv,
+ int index)
+{
+ struct cpuidle_state_usage *state_usage = dev-states_usage[index];
+ struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
+
+ if (ops ops-enter)
+ ops-enter(ops-flags);
+
+ index = cpuidle_wrap_enter(dev, drv, index,
+ arm_cpuidle_simple_enter);
+
+ if (ops ops-exit)
+ ops-exit(ops-flags);
+
+ return index;
+}
+
/* fields in davinci_ops.flags */
#define DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN BIT(0)
static struct cpuidle_driver davinci_idle_driver = {
- .name = cpuidle-davinci,
- .owner = THIS_MODULE,
+ .name = cpuidle-davinci,
+ .owner = THIS_MODULE,
Argh! I am missing the .en_core_tk_irqen = 1 here.
+ .states[0] = ARM_CPUIDLE_WFI_STATE,
+ .states[1] = {
+ .enter = davinci_enter_idle,
+ .exit_latency = 10,
+ .target_residency = 10,
+ .flags = CPUIDLE_FLAG_TIME_VALID,
+ .name = DDR SR,
+ .desc = WFI and DDR Self Refresh,
+ },
+ .state_count = DAVINCI_CPUIDLE_MAX_STATES,
};
static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device);
@@ -77,41 +108,10 @@ static struct davinci_ops
davinci_states[DAVINCI_CPUIDLE_MAX_STATES] = {
},
};
-/* Actual code that puts the SoC in different idle states */
-static int davinci_enter_idle(struct cpuidle_device *dev,
- struct cpuidle_driver *drv,
- int index)
-{
- struct cpuidle_state_usage *state_usage = dev-states_usage[index];
- struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
- struct timeval before, after;
- int idle_time;
-
- local_irq_disable();
- do_gettimeofday(before);
-
- if (ops ops-enter)
- ops-enter(ops-flags);
- /* Wait for interrupt state */
- cpu_do_idle();
- if (ops ops-exit)
- ops-exit(ops-flags);
-
- do_gettimeofday(after);
- local_irq_enable();
- idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
- (after.tv_usec - before.tv_usec);
-
- dev-last_residency = idle_time;
-
- return index;
-}
-
static int __init davinci_cpuidle_probe(struct platform_device *pdev)
{
int ret;
struct cpuidle_device *device;
- struct cpuidle_driver *driver = davinci_idle_driver;
struct davinci_cpuidle_config *pdata = pdev-dev.platform_data;
device = per_cpu(davinci_cpuidle_device, smp_processor_id());
@@ -123,27 +123,11 @@ static int __init davinci_cpuidle_probe(struct
platform_device *pdev)
ddr2_reg_base = pdata-ddr2_ctlr_base;
- /* Wait for interrupt state */
- driver-states[0].enter = davinci_enter_idle;
- driver-states[0].exit_latency = 1;
- driver-states[0].target_residency = 1;
- driver-states[0].flags = CPUIDLE_FLAG_TIME_VALID;
- strcpy(driver-states[0].name, WFI);
- strcpy(driver-states[0].desc, Wait for interrupt);
-
- /* Wait for interrupt and DDR self
Re: [PATCH v8 0/8] Consolidate cpuidle functionality
On Tue, Mar 20, 2012 at 7:10 PM, Kevin Hilman khil...@ti.com wrote: Hi Rob, Robert Lee rob@linaro.org writes: This patch series moves various functionality duplicated in platform cpuidle drivers to the core cpuidle driver. Also, the platform irq disabling was removed as it appears that all calls into cpuidle_call_idle will have already called local_irq_disable(). These changes have been pulled into linux-next. Len, Andrew, can a request be made for Linus to pull these changes? Acked-by: Jean Pihetj-pi...@ti.com (v6) Tested-by: Jean Pihetj-pi...@ti.com (v6, omap3) Tested-by: Amit Danielamit.kach...@linaro.org (v6, Exynos4) Tested-by: Robert Leerob@linaro.org (imx51, imx6q) Note that there's a space missing between the name and email in these tags (and for Deepthi's below also.) That seems to exist in all the patches. Thanks. This is now fixed in my pull tree. Reviewed-by: Kevin Hilman khil...@ti.com For my Reviewed-by, it only applies to the core code and the OMAP changes. I haven't reviewed the other platform-specific drivers. I believe the same applies to Jean Pihet who works with me on OMAP. Reviewed-by: Daniel Lezcano daniel.lezc...@linaro.org Reviewed-by: Deepthi Dharwardeep...@linux.vnet.ibm.com (core cpuidle only) Looks like you never heard from anyone actively working on at91, shmobile, kirwood or davinci. I'm not sure we should merge those platform-specific changes without an ack from those platform maintainers. For 3.4, maybe we should just merge the core code and the platforms that have been reviewed/ack'd, and for 3.5, spent some time nagging the other platform maintainers to review and test. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git pull] Consolidate cpuidle functionality
Len and Andrew, Please consider this an official merge request of this cpuidle patchset for v3.4. There were two small conflicts Stephen Rothwell found when merging to linux-next. The first conflict is with patch 1/9 in the drivers/cpuidle/cpuidle.c file which is trivially to resolve. I'm told this fixup is trivially enough to not require anyone to carry it. The second is with patch 2/9 in mach-at91/cpuidle.c due to some minor cleanup changes. The fix is also pretty simple but I'm waiting to hear back about who will carry it. Please let me know if you need any other action or information on my part. Thanks, Rob On Fri, Mar 9, 2012 at 12:40 AM, Stephen Rothwell s...@canb.auug.org.au wrote: Hi Rob, On Thu, 8 Mar 2012 19:58:23 -0600 Rob Lee rob@linaro.org wrote: git://git.linaro.org/people/rob_lee/linux.git cpuidle_consol_pull These changes move various functionality duplicated in platform cpuidle drivers to the core cpuidle driver and common arch arm code. Also, the irq disabling in the platform code was removed as all calls into cpuidle_call_idle() will have already called local_irq_disable(). This patchset is bisect safe. Also, the core cpuidle and arch changes of the first commit do not require any changes to the arch and platform cpuidle drivers, though those arch and platform change should be made to take advantage of the new consolidation function. Stephen, this patch has been reviewed, tested, and ACK'd per the list above but cpuidle maintainer Len Brown has been out on vacation for a couple of weeks so I am sending you this pull request as time is running out to get this into v3.4. I've had a brief communication with Andrew Morton about this as well so he is aware of this situation. I am fairly new to the community so please let me know if you see anything that needs my attention or anything I should be doing differently. I will add this tree from today. Lets see if anyone screams. Thanks for adding your subsystem tree as a participant of linux-next. As you may know, this is not a judgment of your code. The purpose of linux-next is for integration testing and to lower the impact of conflicts between subsystems in the next merge window. You will need to ensure that the patches/commits in your tree/series have been: * submitted under GPL v2 (or later) and include the Contributor's Signed-off-by, * posted to the relevant mailing list, * reviewed by you (or another maintainer of your subsystem tree), * successfully unit tested, and * destined for the current or next Linux merge window. Basically, this should be just what you would send to Linus (or ask him to fetch). It is allowed to be rebased if you deem it necessary. -- Cheers, Stephen Rothwell s...@canb.auug.org.au Legal Stuff: By participating in linux-next, your subsystem tree contributions are public and will be included in the linux-next trees. You may be sent e-mail messages indicating errors or other issues when the patches/commits from your subsystem tree are merged and tested in linux-next. These messages may also be cross-posted to the linux-next mailing list, the linux-kernel mailing list, etc. The linux-next tree project and IBM (my employer) make no warranties regarding the linux-next project, the testing procedures, the results, the e-mails, etc. If you don't agree to these ground rules, let me know and I'll remove your tree from participation in linux-next. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[git pull] Consolidate cpuidle functionality
Hello Stephen, The following changes since commit 192cfd58774b4d17b2fe8bdc77d89c2ef4e0591d: Linux 3.3-rc6 (2012-03-03 17:08:09 -0800) are available in the git repository at: git://git.linaro.org/people/rob_lee/linux.git cpuidle_consol_pull Robert Lee (8): cpuidle: Add common time keeping and irq enabling ARM: at91: Consolidate time keeping and irq enable ARM: kirkwood: Consolidate time keeping and irq enable ARM: davinci: Consolidate time keeping and irq enable ARM: omap: Consolidate OMAP3 time keeping and irq enable ARM: omap: Consolidate OMAP4 time keeping and irq enable ARM: shmobile: Consolidate time keeping and irq enable SH: shmobile: Consolidate time keeping and irq enable arch/arm/include/asm/cpuidle.h| 22 + arch/arm/kernel/Makefile |2 +- arch/arm/kernel/cpuidle.c | 21 arch/arm/mach-at91/cpuidle.c | 67 ++- arch/arm/mach-davinci/cpuidle.c | 82 +--- arch/arm/mach-kirkwood/cpuidle.c | 72 arch/arm/mach-omap2/cpuidle34xx.c | 42 +++-- arch/arm/mach-omap2/cpuidle44xx.c | 21 +--- arch/arm/mach-shmobile/cpuidle.c | 31 +++-- arch/sh/kernel/cpu/shmobile/cpuidle.c | 10 +--- drivers/cpuidle/cpuidle.c | 79 +-- include/linux/cpuidle.h | 13 +- 12 files changed, 227 insertions(+), 235 deletions(-) create mode 100644 arch/arm/include/asm/cpuidle.h create mode 100644 arch/arm/kernel/cpuidle.c The top-commit id is bd7fd2ab3ee38df371ece6cd4997d6660b3e1c95 Acked-by: Jean Pihetj-pi...@ti.com (v6) Tested-by: Jean Pihetj-pi...@ti.com (v6, omap3) Tested-by: Amit Danielamit.kach...@linaro.org (v6, Exynos4) Tested-by: Robert Leerob@linaro.org (imx51, imx6q) Reviewed-by: Kevin Hilman khil...@ti.com Reviewed-by: Daniel Lezcano daniel.lezc...@linaro.org Reviewed-by: Deepthi Dharwardeep...@linux.vnet.ibm.com (core cpuidle only) These changes move various functionality duplicated in platform cpuidle drivers to the core cpuidle driver and common arch arm code. Also, the irq disabling in the platform code was removed as all calls into cpuidle_call_idle() will have already called local_irq_disable(). This patchset is bisect safe. Also, the core cpuidle and arch changes of the first commit do not require any changes to the arch and platform cpuidle drivers, though those arch and platform change should be made to take advantage of the new consolidation function. Stephen, this patch has been reviewed, tested, and ACK'd per the list above but cpuidle maintainer Len Brown has been out on vacation for a couple of weeks so I am sending you this pull request as time is running out to get this into v3.4. I've had a brief communication with Andrew Morton about this as well so he is aware of this situation. I am fairly new to the community so please let me know if you see anything that needs my attention or anything I should be doing differently. I've checked these changes against the recent linux-next tags and there is currently one small conflict due to a small change in arch/arm/mach-at91/cpuidle.c. I'd be happy to assist in resolving this issue if needed. Also, in my patchset submissions I include changes for a Samsung Exynos cpuidle platform driver patch that was accepted a couple of weeks ago but it hasn't yet made it to an rc so I am not including it in this request. --- patchset submission history: v7 submission can be found here: http://www.spinics.net/lists/arm-kernel/msg162290.html v7 changes: * Made some struct whitespace alignment changes. * Fixed a coding style violation (thanks Jean Pihet) * Fixed a bug in davinci cpuidle (thanks Jean Pihet) * Corrected the common ARM cpuidle WFI state description to be ARM platform agnostic (thanks Kevin Hilman) * Fixed the problem causing x86 and PPC builds to fail (thanks Deepthi) * Re-added a line of code that was mistakenly removed (thanks Deepthi) v6 submission can be found here: http://www.spinics.net/lists/arm-kernel/msg162018.html v6 changes: * Fixed mindless bug in CONFIG_ARCH_HAS_RELAX code in drivers/cpuidle/cpuidle.c * Removed inline from wrapper function (thanks Mike Turquette and Rob Herring) * Add zeroing out of last_residency if error value is returned (thanks Mike) * Made drivers/cpuidle/cpuidle.c more intelligently handle en_core_tk_irqen flag allowing removal of the if (en_core_tk_irqen) in cpuidle_idle_call (thanks Daniel Lezcano) * Moved CPUIDLE_ARM_WFI_STATE macro to arch/arm/include/asm/cpuidle.h (thanks Jean Pihet) * Cleaned up some comments and a stray change (thanks Jean) v5 submission can be found here: http://www.spinics.net/lists/arm-kernel/msg161596.html v5 changes: * Added common cpu_do_idle function to core cpuidle * Added time keep irq en wrapper to core cpuidle *
Re: [PATCH v7 1/9] cpuidle: Add common time keeping and irq enabling
Hello Deepthi, On Wed, Feb 29, 2012 at 10:15 PM, Deepthi Dharwar deep...@linux.vnet.ibm.com wrote: Hi Rob, On 03/01/2012 06:12 AM, Robert Lee wrote: Make necessary changes to implement time keeping and irq enabling in the core cpuidle code. This will allow the removal of these functionalities from various platform cpuidle implementations whose timekeeping and irq enabling follows the form in this common code. The generic cpuidle changes look good, but is there a reason as to why these changes are enabled only for ARM and not other archs ? Besides ARM, this patchset also enables some of this new consolidation functionality on arch/SH and for archs that use the CONFIG_ARCH_HAS_CPU_RELAX (maybe x86 uses this?). For the powerpc P-series, it could probably could be modified to use the consolidated timekeeping but I didn't feel comfortable making that change myself for a couple of reasons. First, the common wrapper also includes the local_irq_enable() call, but the p-series cpuidle code doesn't include this call, as instead, it relies on the local_irq_enable() call in the cpu_idle() function in arch/powerpc/kernel/idle.c. Is it OK to remove this local_irq_enable() once the wrapper is used? Second, is there any special coordination needed with the timekeeping functions and the mfspr() calls? Looking at the intel and acpi cpuidle implementations, their current organization does seem to be able to use the common time keeping / irq enabling wrapper. Upon first glance, it appears that there are special timer/timekeeping requirements for x86 that aren't required by other platforms. But that may not be correct. If you look back at v4 of this patch series, you'll see an attempt at a common timekeeping that could be used by x86 and acpi , but it causes other compromises that to me aren't worth the extra gain from a 100% common timekeeping / irq enable solution. I requested feedback/opinions on this issue after v4 but didn't hear anything about changes made to the intel or acpi implementations. So I continued on with the common wrapper direction from v3 when making v5. Ultimately, even if the consolidated code only can be used by most and not all arch or platform cpuidle implementations, it still reduces some platform cpuidle fragmentation and duplicated code and hopefully improves the maintainability of the core cpuidle. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 0/9] Consolidate cpuidle functionality
On Wed, Feb 29, 2012 at 6:42 PM, Robert Lee rob@linaro.org wrote: This patch series moves various functionality duplicated in platform cpuidle drivers to the core cpuidle driver. Also, the platform irq disabling was removed as it appears that all calls into cpuidle_call_idle will have already called local_irq_disable(). I'm told that I forgot to add the Acks from the previous v6 to this version: Acked-by: Jean Pihet j-pi...@ti.com (v6) Tested-by: Jean Pihet j-pi...@ti.com (v6, omap3) Tested-by: Amit Daniel amit.kach...@linaro.org (v6, Exynos4) For the generic cpuidle changes: Reviewed-by: Deepthi Dharwar deep...@linux.vnet.ibm.com If anyone sees other omissions or has any suggested changes or improvements in my patch submissions semantics, please let me know. Thanks, Rob Rafael, Could you review this patchset and merge patch 1/9 once its ready? It seems pretty close to being acceptable. The get_maintainer script shows Len Brown as the cpuidle maintainer but I've been unable to get a response from him so far. If you are not the right person, could you suggest who I can make this request to? Thanks. Note to platform maintainers: Platform patches (2/9 to 9/9) in this patchset are not required to work with patch 1/9 but please review and push these platform changes as possible to allow this consolidation to occur. Based on 3.3-rc5 plus recent exynos cpuidle patch (affects exynos cpuidle only): http://www.spinics.net/lists/linux-samsung-soc/msg09467.html v6 submission tested successfully on Exynos (thanks Amit Kacchap) and OMAP3 (thanks Jean Pihet) platforms. v6 submission can be found here: http://www.spinics.net/lists/arm-kernel/msg162018.html Changes since v6: * Made some struct whitespace alignment changes. * Fixed a coding style violation (thanks Jean Pihet) * Fixed a bug in davinci cpuidle (thanks Jean Pihet) * Corrected the common ARM cpuidle WFI state description to be ARM platform agnostic (thanks Kevin Hilman) * Fixed the problem causing x86 and PPC builds to fail (thanks Deepthi) * Re-added a line of code that was mistakenly removed (thanks Deepthi) Robert Lee (9): cpuidle: Add common time keeping and irq enabling ARM: at91: Consolidate time keeping and irq enable ARM: exynos: Consolidate time keeping and irq enable ARM: kirkwood: Consolidate time keeping and irq enable ARM: davinci: Consolidate time keeping and irq enable ARM: omap: Consolidate OMAP3 time keeping and irq enable ARM: omap: Consolidate OMAP4 time keeping and irq enable ARM: shmobile: Consolidate time keeping and irq enable SH: shmobile: Consolidate time keeping and irq enable arch/arm/include/asm/cpuidle.h | 22 + arch/arm/kernel/Makefile | 2 +- arch/arm/kernel/cpuidle.c | 21 arch/arm/mach-at91/cpuidle.c | 67 ++- arch/arm/mach-davinci/cpuidle.c | 82 +--- arch/arm/mach-exynos/cpuidle.c | 53 ++--- arch/arm/mach-kirkwood/cpuidle.c | 72 arch/arm/mach-omap2/cpuidle34xx.c | 42 +++-- arch/arm/mach-omap2/cpuidle44xx.c | 21 +--- arch/arm/mach-shmobile/cpuidle.c | 31 +++-- arch/sh/kernel/cpu/shmobile/cpuidle.c | 10 +--- drivers/cpuidle/cpuidle.c | 79 +-- include/linux/cpuidle.h | 13 +- 13 files changed, 233 insertions(+), 282 deletions(-) create mode 100644 arch/arm/include/asm/cpuidle.h create mode 100644 arch/arm/kernel/cpuidle.c -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
On Wed, Feb 29, 2012 at 2:30 AM, Jean Pihet jean.pi...@newoldbits.com wrote: Hi Rob, On Wed, Feb 29, 2012 at 4:11 AM, Robert Lee rob@linaro.org wrote: Make necessary changes to implement time keeping and irq enabling in the core cpuidle code. This will allow the removal of these functionalities from various platform cpuidle implementations whose timekeeping and irq enabling follows the form in this common code. Signed-off-by: Robert Lee rob@linaro.org --- arch/arm/include/asm/cpuidle.h | 14 ++ drivers/cpuidle/cpuidle.c | 90 include/linux/cpuidle.h | 13 ++ 3 files changed, 99 insertions(+), 18 deletions(-) create mode 100644 arch/arm/include/asm/cpuidle.h ... diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index 59f4261..00b02f5 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c ... @@ -110,7 +126,8 @@ int cpuidle_idle_call(void) dev-states_usage[entered_state].time += (unsigned long long)dev-last_residency; dev-states_usage[entered_state].usage++; - } + } else + dev-last_residency = 0; Braces are required here, according to the coding style doc. Thanks. ... Regards, Jean -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
Hello Deepthi,
On Wed, Feb 29, 2012 at 5:13 AM, Deepthi Dharwar
deep...@linux.vnet.ibm.com wrote:
Hi Rob,
On 02/29/2012 08:41 AM, Robert Lee wrote:
Make necessary changes to implement time keeping and irq enabling
in the core cpuidle code. This will allow the removal of these
functionalities from various platform cpuidle implementations whose
timekeeping and irq enabling follows the form in this common code.
Signed-off-by: Robert Lee rob@linaro.org
---
arch/arm/include/asm/cpuidle.h | 14 ++
drivers/cpuidle/cpuidle.c | 90
include/linux/cpuidle.h | 13 ++
3 files changed, 99 insertions(+), 18 deletions(-)
create mode 100644 arch/arm/include/asm/cpuidle.h
diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
new file mode 100644
index 000..1d2075b
--- /dev/null
+++ b/arch/arm/include/asm/cpuidle.h
@@ -0,0 +1,14 @@
+#ifndef __ASM_ARM_CPUIDLE_H
+#define __ASM_ARM_CPUIDLE_H
+
+/* Common ARM WFI state */
+#define CPUIDLE_ARM_WFI_STATE {\
+ .enter = cpuidle_simple_enter,\
+ .exit_latency = 1,\
+ .target_residency = 1,\
+ .flags = CPUIDLE_FLAG_TIME_VALID,\
+ .name = WFI,\
+ .desc = ARM core clock gating (WFI),\
+}
+
+#endif
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 59f4261..00b02f5 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -18,6 +18,7 @@
#include linux/ktime.h
#include linux/hrtimer.h
#include linux/module.h
+#include asm/proc-fns.h
#include trace/events/power.h
#include cpuidle.h
@@ -53,6 +54,24 @@ static void cpuidle_kick_cpus(void) {}
static int __cpuidle_register_device(struct cpuidle_device *dev);
+static inline int cpuidle_enter(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int index)
+{
+ struct cpuidle_state *target_state = drv-states[index];
+ return target_state-enter(dev, drv, index);
+}
+
+static inline int cpuidle_enter_tk(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int index)
+{
+ return cpuidle_wrap_enter(dev, drv, index, cpuidle_enter);
+}
+
+typedef int (*cpuidle_enter_t)(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int index);
+
+static cpuidle_enter_t cpuidle_enter_ops;
+
/**
* cpuidle_idle_call - the main idle loop
*
@@ -63,7 +82,6 @@ int cpuidle_idle_call(void)
{
struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
struct cpuidle_driver *drv = cpuidle_get_driver();
- struct cpuidle_state *target_state;
int next_state, entered_state;
if (off)
@@ -92,12 +110,10 @@ int cpuidle_idle_call(void)
return 0;
}
- target_state = drv-states[next_state];
-
trace_power_start(POWER_CSTATE, next_state, dev-cpu);
trace_cpu_idle(next_state, dev-cpu);
- entered_state = target_state-enter(dev, drv, next_state);
+ entered_state = cpuidle_enter_ops(dev, drv, next_state);
trace_power_end(dev-cpu);
trace_cpu_idle(PWR_EVENT_EXIT, dev-cpu);
@@ -110,7 +126,8 @@ int cpuidle_idle_call(void)
dev-states_usage[entered_state].time +=
(unsigned long long)dev-last_residency;
dev-states_usage[entered_state].usage++;
- }
+ } else
+ dev-last_residency = 0;
/* give the governor an opportunity to reflect on the outcome */
if (cpuidle_curr_governor-reflect)
@@ -164,20 +181,29 @@ void cpuidle_resume_and_unlock(void)
EXPORT_SYMBOL_GPL(cpuidle_resume_and_unlock);
-#ifdef CONFIG_ARCH_HAS_CPU_RELAX
-static int poll_idle(struct cpuidle_device *dev,
- struct cpuidle_driver *drv, int index)
+/**
+ * cpuidle_wrap_enter - performs timekeeping and irqen around enter function
+ * @dev: pointer to a valid cpuidle_device object
+ * @drv: pointer to a valid cpuidle_driver object
+ * @index: index of the target cpuidle state.
+ */
+int cpuidle_wrap_enter(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int index,
+ int (*enter)(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int index))
{
- ktime_t t1, t2;
+ ktime_t time_start, time_end;
s64 diff;
- t1 = ktime_get();
+ time_start = ktime_get();
+
+ index = enter(dev, drv, index);
+
+ time_end = ktime_get();
+
local_irq_enable();
- while (!need_resched())
- cpu_relax();
- t2 = ktime_get();
- diff = ktime_to_us(ktime_sub(t2, t1));
+ diff = ktime_to_us(ktime_sub(time_end, time_start));
if (diff INT_MAX)
diff = INT_MAX;
@@ -186,6 +212,31 @@ static int
Re: [PATCH v6 5/9] ARM: davinci: Consolidate time keeping and irq enable
On Wed, Feb 29, 2012 at 2:36 AM, Jean Pihet jean.pi...@newoldbits.com wrote:
Rob,
On Wed, Feb 29, 2012 at 4:11 AM, Robert Lee rob@linaro.org wrote:
Enable core cpuidle timekeeping and irq enabling and remove that
handling from this code.
Signed-off-by: Robert Lee rob@linaro.org
---
arch/arm/mach-davinci/cpuidle.c | 78
+++---
1 files changed, 31 insertions(+), 47 deletions(-)
diff --git a/arch/arm/mach-davinci/cpuidle.c
b/arch/arm/mach-davinci/cpuidle.c
index a30c7c5..6f457f1 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
...
@@ -30,12 +31,42 @@ struct davinci_ops {
u32 flags;
};
+/* Actual code that puts the SoC in different idle states */
+static int davinci_enter_idle(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv,
+ int index)
+{
+ struct cpuidle_state_usage *state_usage = dev-states_usage[index];
+ struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
+
+ if (ops ops-enter)
+ ops-enter(ops-flags);
+
+ return cpuidle_wrap_enter(dev, drv, index,
+ cpuidle_simple_enter);
This does not look right since ops-exit will never be called.
Yes, thanks. The 'return' should be 'index ='
+
+ if (ops ops-exit)
+ ops-exit(ops-flags);
+
+ return index;
+}
+
...
Regards,
Jean
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
On Wed, Feb 29, 2012 at 12:43 PM, Kevin Hilman khil...@ti.com wrote:
Robert Lee rob@linaro.org writes:
Make necessary changes to implement time keeping and irq enabling
in the core cpuidle code. This will allow the removal of these
functionalities from various platform cpuidle implementations whose
timekeeping and irq enabling follows the form in this common code.
Signed-off-by: Robert Lee rob@linaro.org
---
arch/arm/include/asm/cpuidle.h | 14 ++
drivers/cpuidle/cpuidle.c | 90
include/linux/cpuidle.h | 13 ++
3 files changed, 99 insertions(+), 18 deletions(-)
create mode 100644 arch/arm/include/asm/cpuidle.h
diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
new file mode 100644
index 000..1d2075b
--- /dev/null
+++ b/arch/arm/include/asm/cpuidle.h
@@ -0,0 +1,14 @@
+#ifndef __ASM_ARM_CPUIDLE_H
+#define __ASM_ARM_CPUIDLE_H
+
+/* Common ARM WFI state */
+#define CPUIDLE_ARM_WFI_STATE {\
+ .enter = cpuidle_simple_enter,\
+ .exit_latency = 1,\
+ .target_residency = 1,\
+ .flags = CPUIDLE_FLAG_TIME_VALID,\
+ .name = WFI,\
+ .desc = ARM core clock gating (WFI),\
+}
nit: just name this ARM WFI. Clock gating is platform specific, and
core has platform-specific meanings, so in order to keep this truly
generic, I think hat ARM WFI is the best name.
Agree. I'll make that change.
Kevin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
