On 09/22/2013 03:21 AM, Viresh Kumar wrote:
> Signed-off-by: Viresh Kumar <viresh.ku...@linaro.org>

The optimization sounds good but IMHO if we can move this state out of
the cpuidle common framework that would be nicer.

The poll_idle is only applicable for x86 (acpi_driver and intel_idle),
hence I suggest we move this state to these drivers, that will cleanup
the framework code and will remove index shift macro
CPUIDLE_DRIVER_STATE_START which IMHO is weid and prone-to-error.

> ---
>  drivers/cpuidle/cpuidle.c | 41 -----------------------------------------
>  drivers/cpuidle/driver.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 43d5836..bf80236 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -226,45 +226,6 @@ void cpuidle_resume(void)
>       mutex_unlock(&cpuidle_lock);
>  }
>  
> -#ifdef CONFIG_ARCH_HAS_CPU_RELAX
> -static int poll_idle(struct cpuidle_device *dev,
> -             struct cpuidle_driver *drv, int index)
> -{
> -     ktime_t t1, t2;
> -     s64 diff;
> -
> -     t1 = ktime_get();
> -     local_irq_enable();
> -     while (!need_resched())
> -             cpu_relax();
> -
> -     t2 = ktime_get();
> -     diff = ktime_to_us(ktime_sub(t2, t1));
> -     if (diff > INT_MAX)
> -             diff = INT_MAX;
> -
> -     dev->last_residency = (int) diff;
> -
> -     return index;
> -}
> -
> -static void poll_idle_init(struct cpuidle_driver *drv)
> -{
> -     struct cpuidle_state *state = &drv->states[0];
> -
> -     snprintf(state->name, CPUIDLE_NAME_LEN, "POLL");
> -     snprintf(state->desc, CPUIDLE_DESC_LEN, "CPUIDLE CORE POLL IDLE");
> -     state->exit_latency = 0;
> -     state->target_residency = 0;
> -     state->power_usage = -1;
> -     state->flags = 0;
> -     state->enter = poll_idle;
> -     state->disabled = false;
> -}
> -#else
> -static void poll_idle_init(struct cpuidle_driver *drv) {}
> -#endif /* CONFIG_ARCH_HAS_CPU_RELAX */
> -
>  /**
>   * cpuidle_enable_device - enables idle PM for a CPU
>   * @dev: the CPU
> @@ -294,8 +255,6 @@ int cpuidle_enable_device(struct cpuidle_device *dev)
>       if (!dev->state_count)
>               dev->state_count = drv->state_count;
>  
> -     poll_idle_init(drv);
> -
>       ret = cpuidle_add_device_sysfs(dev);
>       if (ret)
>               return ret;
> diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
> index 7b2510a..a4a93b4 100644
> --- a/drivers/cpuidle/driver.c
> +++ b/drivers/cpuidle/driver.c
> @@ -10,6 +10,7 @@
>  
>  #include <linux/mutex.h>
>  #include <linux/module.h>
> +#include <linux/sched.h>
>  #include <linux/cpuidle.h>
>  #include <linux/cpumask.h>
>  #include <linux/clockchips.h>
> @@ -179,6 +180,45 @@ static void __cpuidle_driver_init(struct cpuidle_driver 
> *drv)
>       }
>  }
>  
> +#ifdef CONFIG_ARCH_HAS_CPU_RELAX
> +static int poll_idle(struct cpuidle_device *dev,
> +             struct cpuidle_driver *drv, int index)
> +{
> +     ktime_t t1, t2;
> +     s64 diff;
> +
> +     t1 = ktime_get();
> +     local_irq_enable();
> +     while (!need_resched())
> +             cpu_relax();
> +
> +     t2 = ktime_get();
> +     diff = ktime_to_us(ktime_sub(t2, t1));
> +     if (diff > INT_MAX)
> +             diff = INT_MAX;
> +
> +     dev->last_residency = (int) diff;
> +
> +     return index;
> +}
> +
> +static void poll_idle_init(struct cpuidle_driver *drv)
> +{
> +     struct cpuidle_state *state = &drv->states[0];
> +
> +     snprintf(state->name, CPUIDLE_NAME_LEN, "POLL");
> +     snprintf(state->desc, CPUIDLE_DESC_LEN, "CPUIDLE CORE POLL IDLE");
> +     state->exit_latency = 0;
> +     state->target_residency = 0;
> +     state->power_usage = -1;
> +     state->flags = 0;
> +     state->enter = poll_idle;
> +     state->disabled = false;
> +}
> +#else
> +static void poll_idle_init(struct cpuidle_driver *drv) {}
> +#endif /* !CONFIG_ARCH_HAS_CPU_RELAX */
> +
>  /**
>   * __cpuidle_register_driver: register the driver
>   * @drv: a valid pointer to a struct cpuidle_driver
> @@ -212,6 +252,8 @@ static int __cpuidle_register_driver(struct 
> cpuidle_driver *drv)
>               on_each_cpu_mask(drv->cpumask, cpuidle_setup_broadcast_timer,
>                                (void *)CLOCK_EVT_NOTIFY_BROADCAST_ON, 1);
>  
> +     poll_idle_init(drv);
> +
>       return 0;
>  }
>  
> 


-- 
 <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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to