On Fri, Feb 10, 2012 at 11:32 AM, Rob Lee <rob.lee@xxxxxxxxxx> wrote: > Maintainers for drivers/cpuidle, do you have any comments/opinions > about this patch? Venki has changed employers (probably needs a patch to MAINTAINERS?). Cc'ing his new email address. > Intel cpuidle and acpi cpuidle maintainers, do you have any > comments/opinions about this patch and the changes to your code? > > Any other review and comments welcome. > > Summary of positive and negatives as I understand them so far: > > version 1, 2, and 3 (Original "wrapper" method of consolidating > timekeeping and interrupt enabling) > + opportunistically provides consolidation for simple platform cpuidle > implementations without disturbing the more complex implementations. > By simple, I mean those at can be wrapped in the time keeping calls > and interrupt enabling calls without significantly affecting idle time > keeping accuracy or interrupt latency > - Does not provide consolidation for the more complex platform cpuidle > implementations > - Adds an additional interface, perhaps unnecessarily if this > consolidation could be done in cpuidle > > version 4 (modifications to drivers/cpuidle/cpuidle.c cpuidle_idle_call) > + Adds consolidation work to cpuidle_idle_call which allows all > platform timekeeping / interrupt handling to be consolidated. > - Requires splitting up of more complex platform cpuidle > implementations, adding further complexity and risk of breaking > something. > ? Allows both pre_enter or enter to change the idle state. Is there > an objection to this? > ? Splitting up the enter functions can require additional function > calls. Is there any concern that this is significant additional > overhead? > > Thanks, > Rob > > > On Tue, Jan 31, 2012 at 7:00 PM, Robert Lee <rob.lee@xxxxxxxxxx> wrote: >> This patch series moves the timekeeping and irq enabling from the platform >> code 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(). >> >> To save reviewers time, only a few platforms which required the most changes >> are included in this version. If these changes are approved, the next version >> will include the remaining platform code which should require minimal changes. >> >> For those who have followed the previous patch versions, as you know, the >> previous version of this patch series added some helper functionality which >> used a wrapper function to remove the time keeping and irq enabling/disabling >> from the platform code. There was also initialization helper functionality >> which has now been removed from this version. If the basic implementation >> in this version is approved, then a separate patch submission effort can be >> made to focus on consolidation of initialziation functionality. >> >> Based on 3.3-rc1 >> >> v3 submission can be found here: >> http://www.spinics.net/lists/arm-kernel/msg156751.html >> Changes since v3: >> * Removed drivers/cpuidle/common.c >> ** Removed the initialization helper functions >> ** Removed the wrapper used to consolidate time keeping and irq enable/disable >> * Add time keeping and local_irq_disable handling in cpuidle_call_idle(). >> * Made necessary modifications to a few platforms that required the most changes >> ** Note on omap3: changed structure of omap3_idle_drvdata and added >> per_next_state and per_saved_state vars to accomodate new framework. >> >> v2 submission can be found here: >> http://comments.gmane.org/gmane.linux.ports.arm.kernel/144199 >> >> Changes since v2: >> * Made various code organization and style changes as suggested in v1 review. >> * Removed at91 use of common code. A separate effort is underway to clean >> at91 code and the author has offered to convert to common interface as part >> of those changes (if this common interface is accepted in time). >> * Made platform cpuidle_driver objects __initdata and dynamically added one >> persistent instance of this object in common code. >> * Removed imx5 pm usage of gpc_dvfs clock as it is no longer needed after >> being enabled during clock initialization. >> * Re-organized patches. >> >> v1 submission can be found here: >> http://comments.gmane.org/gmane.linux.ports.arm.kernel/142791 >> >> Changes since v1: >> * Common interface moved to drivers/cpuidle and made non arch-specific. >> * Made various fixes and suggested additions to the common cpuidle >> code from v1 review. >> * Added callback for filling in driver_data field as needed. >> * Modified the various platforms with these changes. >> >> Robert Lee (4): >> cpuidle: Add time keeping and irq enabling >> ARM: omap: Remove cpuidle timekeeping and irq enable/disable >> acpi: Remove cpuidle timekeeping and irq enable/disable >> x86: Remove cpuidle timekeeping and irq enable/disable >> >> arch/arm/mach-omap2/cpuidle34xx.c | 96 ++++++++---------- >> drivers/acpi/processor_idle.c | 203 ++++++++++++++++++++++--------------- >> drivers/cpuidle/cpuidle.c | 75 +++++++++++--- >> drivers/idle/intel_idle.c | 110 ++++++++++++++------ >> include/linux/cpuidle.h | 26 +++-- >> 5 files changed, 317 insertions(+), 193 deletions(-) >> -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
