Re: [PATCH 05/18] cpuidle: make a single register function for all

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

On 04/10/2013 08:23 PM, Andrew Lunn wrote:
> On Wed, Apr 10, 2013 at 08:02:31PM +0200, Daniel Lezcano wrote:
>> On 04/10/2013 07:04 PM, Andrew Lunn wrote:
>>>> +int cpuidle_register(struct cpuidle_driver *drv,
>>>> +		     const struct cpumask *const coupled_cpus)
>>>> +{
>>>> +	int ret, cpu;
>>>> +	struct cpuidle_device *device;
>>>> +
>>>> +	ret = cpuidle_register_driver(drv);
>>>> +	if (ret) {
>>>> +		printk(KERN_ERR "failed to register cpuidle driver\n");
>>>> +		return ret;
>>>> +	}
>>>> +
>>>> +	for_each_possible_cpu(cpu) {
>>>> +		device = &per_cpu(cpuidle_dev, cpu);
>>>> +		device->cpu = cpu;
>>>> +#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
>>>> +		device->coupled_cpus = *coupled_cpus;
>>>> +#endif
>>>
>>> At least the kirkwood and the calxeda driver set 
>>>
>>>    device->state_count
>>>
>>> which you don't appear to do. cpuidle_add_state_sysfs() and
>>> cpuidle_remove_state_sysfs() use this. Is it now being set somewhere
>>> else?
>>
>> Yes, in cpuidle_enable_device called from cpuidle_register_device:
> 
> O.K. It would be nice to add a comment in the change log message about
> this.
> 
> Ah, also, it would be good to update Documentation/cpuidle/drivers.txt
> with these new functions and update the text.

Ok, I will take care of that.

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