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? > > 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. I think the question of what the timekeeping means needs to be considered. If the timekeeping is supposed to be a very accurate measurement of the time spent in the low power idle state, only the cpuidle driver can guarantee that - there may be significant time spent in the hardware transition or the very low level power code that cannot be split into pre_enter, but should not be counted in the timekeeping. Or there may be a long boot time out of the low power state that should not be counted. If it is just a rough estimate of how often the cpu is getting to idle, there is no need to split out the pre_enter time - just measure the time around the entire driver enter call. Either way, pre_enter doesn't seem useful. > - 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? pre_enter (if it is kept) would probably have to support state demotion, because its actions may depend on the final state. For coupled SMP cpuidle, enter also has to support state demotion, because the final state will depend on actions of the other cpu after idle has been entered. > ? Splitting up the enter functions can require additional function > calls. Is there any concern that this is significant additional > overhead? I don't think so, especially if you support NULL pre_enter and post_enter functions to allow drivers that care to skip them. -- 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
