Re: [RFC PATCH] cpuidle: fix device->state_count handling

[Daniel Lezcano <daniel.lezcano@linaro.org>]

On 08/09/2013 01:59 PM, Bartlomiej Zolnierkiewicz wrote:
> Use device->state_count instead of driver->state_count in
> cpuidle_play_dead(), ladder_select_state() and menu_select().
> 
> This allows cpuidle drivers to override maximum allowable
> state number for a given device, which is what some cpuidle
> drivers would like to do (i.e. EXYNOS cpuidle driver).
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Amit Daniel Kachhap <amit.daniel@samsung.com>
> ---
> Alternatively, we can make EXYNOS cpuidle driver work without
> device->state_count (at a some tiny performance cost?). This
> would allow us to remove device->state_count completely as it
> is not needed by any other cpuidle driver currently (AFAICS).

I am inclined for this solution.

The state_count was initially in the device structure, then it has been
moved to the driver structure and the code has been changed accordingly.
That was part of the cpuidle code consolidation made by Deepthi Dharwar
(46bcfad7).

This patch is one step back.

There is an alternate solution. May be you can use the multiple driver
support to register two cpuidle driver (one for each cpus) ? It was
initially made to support cpus with different latencies but that should
work.

>  drivers/cpuidle/cpuidle.c          |    2 +-
>  drivers/cpuidle/governors/ladder.c |    2 +-
>  drivers/cpuidle/governors/menu.c   |    2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> Index: b/drivers/cpuidle/cpuidle.c
> ===================================================================
> --- a/drivers/cpuidle/cpuidle.c	2013-08-08 14:36:09.611488750 +0200
> +++ b/drivers/cpuidle/cpuidle.c	2013-08-08 14:39:02.291486116 +0200
> @@ -57,7 +57,7 @@ int cpuidle_play_dead(void)
>  		return -ENODEV;
>  
>  	/* Find lowest-power state that supports long-term idle */
> -	for (i = drv->state_count - 1; i >= CPUIDLE_DRIVER_STATE_START; i--)
> +	for (i = dev->state_count - 1; i >= CPUIDLE_DRIVER_STATE_START; i--)
>  		if (drv->states[i].enter_dead)
>  			return drv->states[i].enter_dead(dev, i);
>  
> Index: b/drivers/cpuidle/governors/ladder.c
> ===================================================================
> --- a/drivers/cpuidle/governors/ladder.c	2013-08-08 14:36:09.611488750 +0200
> +++ b/drivers/cpuidle/governors/ladder.c	2013-08-08 14:39:02.299486116 +0200
> @@ -87,7 +87,7 @@ static int ladder_select_state(struct cp
>  		last_residency = last_state->threshold.promotion_time + 1;
>  
>  	/* consider promotion */
> -	if (last_idx < drv->state_count - 1 &&
> +	if (last_idx < dev->state_count - 1 &&
>  	    !drv->states[last_idx + 1].disabled &&
>  	    !dev->states_usage[last_idx + 1].disable &&
>  	    last_residency > last_state->threshold.promotion_time &&
> Index: b/drivers/cpuidle/governors/menu.c
> ===================================================================
> --- a/drivers/cpuidle/governors/menu.c	2013-08-08 14:36:09.611488750 +0200
> +++ b/drivers/cpuidle/governors/menu.c	2013-08-08 14:39:02.299486116 +0200
> @@ -312,7 +312,7 @@ static int menu_select(struct cpuidle_dr
>  	 * Find the idle state with the lowest power while satisfying
>  	 * our constraints.
>  	 */
> -	for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
> +	for (i = CPUIDLE_DRIVER_STATE_START; i < dev->state_count; i++) {
>  		struct cpuidle_state *s = &drv->states[i];
>  		struct cpuidle_state_usage *su = &dev->states_usage[i];
>  
> 


-- 
 <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

  * English - detected
  * English
  * French

  * English
  * French

 <javascript:void(0);>