Messages in this thread |  | | Date | Thu, 18 Oct 2012 10:21:52 +0200 | | From | Daniel Lezcano <> | | Subject | Re: [PATCH] cpuidle: reinitialize power_usage values when adding/removing C-states |
| |
On 10/17/2012 08:43 PM, Julius Werner wrote: >> This is specific to the acpi and should be handled in the >> processor_idle.c file instead of the cpuidle core code. >> >> Could be the function 'acpi_processor_cst_has_changed' the right place >> to set a dummy power value for the power in the new C-state ? > > Thanks for your feedback. I think it wouldn't be wise to split the > dummy power value logic over two places, but I could submit a patch > that makes set_power_states globally accessible and calls it from > acpi_processor_cst_has_changed instead.
No please, do not export this function. That will add more confusion on the acpi code and more generally in the cpuidle core code.
IIUC, a new state is inserted/deleted and we will set the entire array of states to setup the power.
You have the acpi_processor_cst_has_changed function calling the acpi_processor_setup_cpuidle_states function. It seems to be a good candidate to setup the power of the new state. All the states are filled again AFAICS under the lock.
> However, I do not think this should really be ACPI specific. It > applies to any cpuidle driver that wants to change its idle states at > runtime. Currently only the ACPI one does, but the future might bring > others that would run into the same problem. I also think that > set_power_states fits much better into cpuidle_enable_device > conceptually anyway (right next to poll_idle_init which also does > state initialization).
The states are now part of the cpuidle driver and the set_power_states should remain to this file. The dynamic C-states brought some complexity in the acpi code and honestly this code is very confusing.
Maybe one day, that would make sense but until then I am in favor of keeping the arch specific bits in the drivers, especially when they are *hum* so "complex".
poll_idle_init looks hackish for me and probably move it to the arch would also make sense.
Thanks -- Daniel
-- <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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |