On 2 April 2015 at 09:44, Saravana Kannan skannan@codeaurora.org wrote:
When you say unmanaged CPUs, do you mean offline CPUs? If so, could you use that term instead? The term unmanaged is kinda ambiguous. This comment applies to the entire series.
Hmm, will take a note of that.
write_lock_irqsave(&cpufreq_driver_lock, flags);for_each_cpu(j, policy->related_cpus)per_cpu(cpufreq_cpu_data, j) = policy;write_unlock_irqrestore(&cpufreq_driver_lock, flags);}Why not do this is cpufreq_policy_alloc() to be consistent/symmetric with cpufreq_policy_free()? You had to set cpufreq_cpu_data/add to the list this late in cpufreq_add_dev before because you shouldn't be adding a unfinished policy to the cpufreq_cpu_data/list. But now that you check for policy->cpus before returning it, you may be do all of this in cpufreq_policy_alloc()?
We don't know related_cpus until the time ->init() is called and that happens after alloc()..
write_lock_irqsave(&cpufreq_driver_lock, flags);list_add(&policy->policy_list, &cpufreq_policy_list);write_unlock_irqrestore(&cpufreq_driver_lock, flags);}You could probably do this as part of policy alloc too?
I wouldn't like to add a unfinished policy (which may or maynot be simultaneously referred by other parts of code) to the list.