On 06/08/2012 07:33 PM, Deepthi Dharwar wrote:
Hi Daniel,
Hi Deepthi,
On 06/08/2012 09:32 PM, Daniel Lezcano wrote:
We have the state index passed as parameter to the 'enter' function. Most of the drivers assign their 'enter' functions several times in the cpuidle_state structure, as we have the index, we can delegate to the driver to handle their own callback array.
That will have the benefit of removing multiple lines of code in the different drivers.
In order to smoothly modify the driver, the 'enter' function are in the driver structure and in the cpuidle state structure. That will let the time to modify the different drivers one by one. So the 'cpuidle_enter' function checks if the 'enter' callback is assigned in the driver structure and use it, otherwise it invokes the 'enter' assigned to the cpuidle_state.
Currently, the backend driver initializes all the cpuidle states supported on the platform, and each state can have its own enter routine which can be unique This is a clean approach.
Yes, I perfectly understood the purpose of this field but as clean it is it does not make sense as it is not used in this way. If it is supposed to be done in the way you are describing here, we should have the same number of states and enter functions. Here it is how it is used:
-------------------------------------------------- | Arch | nr states | nr enter function | -------------------------------------------------- | x86 (nehalem) | 3 | 1 | -------------------------------------------------- | x86 (snb) | 4 | 1 | -------------------------------------------------- | x86 (atom) | 4 | 1 | -------------------------------------------------- | ARM tegra | 1 | 1 | -------------------------------------------------- | ARM omap3 | 7 | 2 | -------------------------------------------------- | ARM omap4 | 3 | 1 | -------------------------------------------------- | ARM ux500 | 2 | 1 | -------------------------------------------------- | ARM shmobile | 1 | 1 | -------------------------------------------------- | ARM davinci | 2 | 1 | -------------------------------------------------- | ARM at91 | 2 | 1 | -------------------------------------------------- | ARM s3c64xx | 1 | 1 | -------------------------------------------------- | ARM exynos | 2 | 1 | -------------------------------------------------- | ARM kirkwood | 2 | 1 | -------------------------------------------------- | SH | 3 | 1 | -------------------------------------------------- | PPC | 2 | 2 | -------------------------------------------------- | | | | | TOTAL | 39 | 17 | | | | | --------------------------------------------------
As you can see most of the enter functions are only used as one. The Omap3 cpuidle driver enter function for C2 calls the enter function of C1. Other arch, already use a table of callbacks or the index.
By moving the enter routine into the driver, we are enforcing in having only one enter state. There is unnecessary overhead involved in calling a wrapper routine just to index into the right idle state routine for many platforms at runtime.
I don't agree. For the sake of encapsulated code, we duplicate n-times a field and that is not used in this way. It is quite easy to have in the driver specific code a common enter function to ventilate to the right routine without adding extra overhead and let the common code use a single enter routine (which is already the case today).