Re: [PATCH] thermal: Add missing cpumask_clear
On Fri, Jul 04, 2014 at 11:22:40AM +0100, Jonghwan Choi wrote: > Cpumasks should be cleared before using. > > Signed-off-by: Jonghwan Choi > --- > drivers/thermal/db8500_cpufreq_cooling.c|1 + > drivers/thermal/imx_thermal.c |1 + > drivers/thermal/samsung/exynos_thermal_common.c |1 + > 3 files changed, 3 insertions(+) Reviewed-by: Javi Merino You should send this patch to linux-pm, I think it should go via the thermal tree. Cheers, Javi -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] thermal: Add missing cpumask_clear
On Sat, Jul 5, 2014 at 6:04 AM, Tomasz Figa wrote:
> Hi Jonghwan,
>
> On 05.07.2014 01:37, Jonghwan Choi wrote:
>> On Fri, Jul 4, 2014 at 8:23 PM, Sachin Kamat wrote:
>>
Cpumasks should be cleared before using.
>>>
>>> Please explain why and what is issue observed without this.
>>>
>>
>> -> When I checked the mask value, I knew that unwanted bit is set.
>>
>> Test code without cpumask_clear.
>>
>> + cpumask_set_cpu(0, &mask_val);
>> + cpulist_scnprintf(buf, 64, &mask_val);
>> + printk("--ID [ %d] = %s \n", id, buf);
>> + th_zone->cool_dev[id] = cpufreq_cooling_register(&mask_val);
>>
>>
>> Console message-> 4.861157] [c6] --ID [ 1] = 0,4-5,7 (4,5,7 cpu bit was set.)
>>
>> And when I tried to register two cooling devices with cpumask_set_cpu(0,
>> &mask_val) and cpumask_set_cpu(4, &mask_val).
>>
>> I found that cpu 0 bit is also set in latter cpumask. (I hope latter cpumask
>> has a cpu 4 bit.)
>>
>> So I think that cpumask_clear should be inserted.
>
> I believe Sachin's concern was related to your patch description. A good
> description should say what the patch changes and what is the rationale
> behind this change. Also for fixes it is a good practice to specify
> observed issues in patch description as well.
Yes, that is correct. Sorry if it wasn't clear in my previous mail.
--
Regards,
Sachin.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] thermal: Add missing cpumask_clear
On Fri, Jul 5, 2014 at 9:34 AM, Tomasz Figa wrote: > > Console message-> 4.861157] [c6] --ID [ 1] = 0,4-5,7 (4,5,7 cpu bit > > was set.) > > > > And when I tried to register two cooling devices with cpumask_set_cpu(0, > > &mask_val) and > cpumask_set_cpu(4, &mask_val). > > > > I found that cpu 0 bit is also set in latter cpumask. (I hope latter > > cpumask has a cpu 4 bit.) > > > > So I think that cpumask_clear should be inserted. > > I believe Sachin's concern was related to your patch description. A good > description should say what > the patch changes and what is the rationale behind this change. Also for > fixes it is a good practice > to specify observed issues in patch description as well. > > Best regards, -> You are right. Thanks for your kind advice. Best regards. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] thermal: Add missing cpumask_clear
Hi Jonghwan,
On 05.07.2014 01:37, Jonghwan Choi wrote:
> On Fri, Jul 4, 2014 at 8:23 PM, Sachin Kamat wrote:
>
>>> Cpumasks should be cleared before using.
>>
>> Please explain why and what is issue observed without this.
>>
>
> -> When I checked the mask value, I knew that unwanted bit is set.
>
> Test code without cpumask_clear.
>
> + cpumask_set_cpu(0, &mask_val);
> + cpulist_scnprintf(buf, 64, &mask_val);
> + printk("--ID [ %d] = %s \n", id, buf);
> + th_zone->cool_dev[id] = cpufreq_cooling_register(&mask_val);
>
>
> Console message-> 4.861157] [c6] --ID [ 1] = 0,4-5,7 (4,5,7 cpu bit was set.)
>
> And when I tried to register two cooling devices with cpumask_set_cpu(0,
> &mask_val) and cpumask_set_cpu(4, &mask_val).
>
> I found that cpu 0 bit is also set in latter cpumask. (I hope latter cpumask
> has a cpu 4 bit.)
>
> So I think that cpumask_clear should be inserted.
I believe Sachin's concern was related to your patch description. A good
description should say what the patch changes and what is the rationale
behind this change. Also for fixes it is a good practice to specify
observed issues in patch description as well.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] thermal: Add missing cpumask_clear
On Fri, Jul 4, 2014 at 8:23 PM, Sachin Kamat wrote:
> > Cpumasks should be cleared before using.
>
> Please explain why and what is issue observed without this.
>
-> When I checked the mask value, I knew that unwanted bit is set.
Test code without cpumask_clear.
+ cpumask_set_cpu(0, &mask_val);
+ cpulist_scnprintf(buf, 64, &mask_val);
+ printk("--ID [ %d] = %s \n", id, buf);
+ th_zone->cool_dev[id] = cpufreq_cooling_register(&mask_val);
Console message-> 4.861157] [c6] --ID [ 1] = 0,4-5,7 (4,5,7 cpu bit was set.)
And when I tried to register two cooling devices with cpumask_set_cpu(0,
&mask_val) and cpumask_set_cpu(4, &mask_val).
I found that cpu 0 bit is also set in latter cpumask. (I hope latter cpumask
has a cpu 4 bit.)
So I think that cpumask_clear should be inserted.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] thermal: Add missing cpumask_clear
Hi Jonghwan, On Fri, Jul 4, 2014 at 3:52 PM, Jonghwan Choi wrote: > Cpumasks should be cleared before using. Please explain why and what is issue observed without this. > > Signed-off-by: Jonghwan Choi > --- -- Regards, Sachin. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] thermal: Add missing cpumask_clear
Cpumasks should be cleared before using.
Signed-off-by: Jonghwan Choi
---
drivers/thermal/db8500_cpufreq_cooling.c|1 +
drivers/thermal/imx_thermal.c |1 +
drivers/thermal/samsung/exynos_thermal_common.c |1 +
3 files changed, 3 insertions(+)
diff --git a/drivers/thermal/db8500_cpufreq_cooling.c
b/drivers/thermal/db8500_cpufreq_cooling.c
index 786d192..1fa46ff 100644
--- a/drivers/thermal/db8500_cpufreq_cooling.c
+++ b/drivers/thermal/db8500_cpufreq_cooling.c
@@ -34,6 +34,7 @@ static int db8500_cpufreq_cooling_probe(struct
platform_device *pdev)
if (!cpufreq_frequency_get_table(0))
return -EPROBE_DEFER;
+ cpumask_clear(&mask_val);
cpumask_set_cpu(0, &mask_val);
cdev = cpufreq_cooling_register(&mask_val);
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index a99c631..a21acf8 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -449,6 +449,7 @@ static int imx_thermal_probe(struct platform_device *pdev)
regmap_write(map, MISC0 + REG_SET, MISC0_REFTOP_SELBIASOFF);
regmap_write(map, TEMPSENSE0 + REG_SET, TEMPSENSE0_POWER_DOWN);
+ cpumask_clear(&clip_cpus);
cpumask_set_cpu(0, &clip_cpus);
data->cdev = cpufreq_cooling_register(&clip_cpus);
if (IS_ERR(data->cdev)) {
diff --git a/drivers/thermal/samsung/exynos_thermal_common.c
b/drivers/thermal/samsung/exynos_thermal_common.c
index 3f5ad25..47efa4c 100644
--- a/drivers/thermal/samsung/exynos_thermal_common.c
+++ b/drivers/thermal/samsung/exynos_thermal_common.c
@@ -361,6 +361,7 @@ int exynos_register_thermal(struct thermal_sensor_conf
*sensor_conf)
return -ENOMEM;
th_zone->sensor_conf = sensor_conf;
+ cpumask_clear(&mask_val);
/*
* TODO: 1) Handle multiple cooling devices in a thermal zone
* 2) Add a flag/name in cooling info to map to specific
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
