Re: [PATCH v6 1/6] thermal: add generic cpufreq cooling implementation

From: jonghwa3 . lee
Date: Thu Sep 20 2012 - 22:23:58 EST


Hi, Amit,
On 2012년 08월 16일 20:41, Amit Daniel Kachhap wrote:

> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> new file mode 100644
> index 0000000..66cbd52
> --- /dev/null
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -0,0 +1,586 @@

> +/**
> + * cpufreq_cooling_register - function to create cpufreq cooling device.
> + * @clip_cpus: cpumask of cpus where the frequency constraints will happen.
> + */
> +struct thermal_cooling_device *cpufreq_cooling_register(
> + struct cpumask *clip_cpus)
> +{
> + struct thermal_cooling_device *cool_dev;
> + struct cpufreq_cooling_device *cpufreq_dev = NULL;
> + unsigned int cpufreq_dev_count = 0, min = 0, max = 0;
> + char dev_name[THERMAL_NAME_LENGTH];
> + int ret = 0, id = 0, i;
> + struct cpufreq_policy policy;
> + ret = get_idr(&cpufreq_idr,&cpufreq_dev->id);
> + if (ret) {
> + kfree(cpufreq_dev);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + sprintf(dev_name, "thermal-cpufreq-%d", cpufreq_dev->id);
> +
> + cool_dev = thermal_cooling_device_register(dev_name, cpufreq_dev,
> +&cpufreq_cooling_ops);
> + if (!cool_dev) {
> + release_idr(&cpufreq_idr, cpufreq_dev->id);
> + kfree(cpufreq_dev);
> + return ERR_PTR(-EINVAL);
> + }
> + cpufreq_dev->id = id;


Why is this needed? Should every cpufreq_dev instance's id be same for
zero? It looks odd. And it also has problem which can be occurred when
it release id during unregistering cpufreq_dev. I think it should keep
the idr value taken before.


> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/