* [PATCH 1/1] thermal: fix cpu_cooling max_level behavior
@ 2013-11-13 18:11 Eduardo Valentin
2013-11-13 18:31 ` Eduardo Valentin
2013-12-06 13:52 ` Eduardo Valentin
0 siblings, 2 replies; 5+ messages in thread
From: Eduardo Valentin @ 2013-11-13 18:11 UTC (permalink / raw)
To: rui.zhang; +Cc: linux-pm, linux-kernel, Eduardo Valentin
As per Documentation/thermal/sysfs-api.txt, max_level
is an index, not a counter. Thus, in case a CPU has
3 valid frequencies, max_level is expected to be 2, for instance.
The current code makes max_level == number of valid frequencies,
which is bogus. This patch fix the cpu_cooling device by
ranging max_level properly.
Reported-by: Carlos Hernandez <ceh@ti.com>
Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
---
drivers/thermal/cpu_cooling.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index d179028..d0f8f8b5 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -173,6 +173,8 @@ static int get_property(unsigned int cpu, unsigned long input,
freq = table[i].frequency;
max_level++;
}
+ /* max_level is an index, not a counter */
+ max_level--;
/* get max level */
if (property == GET_MAXL) {
@@ -181,7 +183,7 @@ static int get_property(unsigned int cpu, unsigned long input,
}
if (property == GET_FREQ)
- level = descend ? input : (max_level - input - 1);
+ level = descend ? input : (max_level - input);
for (i = 0, j = 0; table[i].frequency != CPUFREQ_TABLE_END; i++) {
/* ignore invalid entry */
@@ -197,7 +199,7 @@ static int get_property(unsigned int cpu, unsigned long input,
if (property == GET_LEVEL && (unsigned int)input == freq) {
/* get level by frequency */
- *output = descend ? j : (max_level - j - 1);
+ *output = descend ? j : (max_level - j);
return 0;
}
if (property == GET_FREQ && level == j) {
--
1.8.2.1.342.gfa7285d
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] thermal: fix cpu_cooling max_level behavior
2013-11-13 18:11 [PATCH 1/1] thermal: fix cpu_cooling max_level behavior Eduardo Valentin
@ 2013-11-13 18:31 ` Eduardo Valentin
2013-12-06 13:52 ` Eduardo Valentin
1 sibling, 0 replies; 5+ messages in thread
From: Eduardo Valentin @ 2013-11-13 18:31 UTC (permalink / raw)
To: Eduardo Valentin; +Cc: rui.zhang, linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1940 bytes --]
On 13-11-2013 14:11, Eduardo Valentin wrote:
> As per Documentation/thermal/sysfs-api.txt, max_level
> is an index, not a counter. Thus, in case a CPU has
> 3 valid frequencies, max_level is expected to be 2, for instance.
>
> The current code makes max_level == number of valid frequencies,
> which is bogus. This patch fix the cpu_cooling device by
> ranging max_level properly.
>
> Reported-by: Carlos Hernandez <ceh@ti.com>
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
Rui,
This fix is applicable even on 3.12.
> ---
> drivers/thermal/cpu_cooling.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index d179028..d0f8f8b5 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -173,6 +173,8 @@ static int get_property(unsigned int cpu, unsigned long input,
> freq = table[i].frequency;
> max_level++;
> }
> + /* max_level is an index, not a counter */
> + max_level--;
>
> /* get max level */
> if (property == GET_MAXL) {
> @@ -181,7 +183,7 @@ static int get_property(unsigned int cpu, unsigned long input,
> }
>
> if (property == GET_FREQ)
> - level = descend ? input : (max_level - input - 1);
> + level = descend ? input : (max_level - input);
>
> for (i = 0, j = 0; table[i].frequency != CPUFREQ_TABLE_END; i++) {
> /* ignore invalid entry */
> @@ -197,7 +199,7 @@ static int get_property(unsigned int cpu, unsigned long input,
>
> if (property == GET_LEVEL && (unsigned int)input == freq) {
> /* get level by frequency */
> - *output = descend ? j : (max_level - j - 1);
> + *output = descend ? j : (max_level - j);
> return 0;
> }
> if (property == GET_FREQ && level == j) {
>
--
You have got to be excited about what you are doing. (L. Lamport)
Eduardo Valentin
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 295 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] thermal: fix cpu_cooling max_level behavior
2013-11-13 18:11 [PATCH 1/1] thermal: fix cpu_cooling max_level behavior Eduardo Valentin
2013-11-13 18:31 ` Eduardo Valentin
@ 2013-12-06 13:52 ` Eduardo Valentin
2014-01-02 4:02 ` Zhang Rui
1 sibling, 1 reply; 5+ messages in thread
From: Eduardo Valentin @ 2013-12-06 13:52 UTC (permalink / raw)
To: Eduardo Valentin; +Cc: rui.zhang, linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]
On 13-11-2013 14:11, Eduardo Valentin wrote:
> As per Documentation/thermal/sysfs-api.txt, max_level
> is an index, not a counter. Thus, in case a CPU has
> 3 valid frequencies, max_level is expected to be 2, for instance.
>
> The current code makes max_level == number of valid frequencies,
> which is bogus. This patch fix the cpu_cooling device by
> ranging max_level properly.
>
> Reported-by: Carlos Hernandez <ceh@ti.com>
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
Rui,
Can you please consider pushing this fix?
> ---
> drivers/thermal/cpu_cooling.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index d179028..d0f8f8b5 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -173,6 +173,8 @@ static int get_property(unsigned int cpu, unsigned long input,
> freq = table[i].frequency;
> max_level++;
> }
> + /* max_level is an index, not a counter */
> + max_level--;
>
> /* get max level */
> if (property == GET_MAXL) {
> @@ -181,7 +183,7 @@ static int get_property(unsigned int cpu, unsigned long input,
> }
>
> if (property == GET_FREQ)
> - level = descend ? input : (max_level - input - 1);
> + level = descend ? input : (max_level - input);
>
> for (i = 0, j = 0; table[i].frequency != CPUFREQ_TABLE_END; i++) {
> /* ignore invalid entry */
> @@ -197,7 +199,7 @@ static int get_property(unsigned int cpu, unsigned long input,
>
> if (property == GET_LEVEL && (unsigned int)input == freq) {
> /* get level by frequency */
> - *output = descend ? j : (max_level - j - 1);
> + *output = descend ? j : (max_level - j);
> return 0;
> }
> if (property == GET_FREQ && level == j) {
>
--
You have got to be excited about what you are doing. (L. Lamport)
Eduardo Valentin
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 295 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] thermal: fix cpu_cooling max_level behavior
2013-12-06 13:52 ` Eduardo Valentin
@ 2014-01-02 4:02 ` Zhang Rui
2014-01-04 13:53 ` Eduardo Valentin
0 siblings, 1 reply; 5+ messages in thread
From: Zhang Rui @ 2014-01-02 4:02 UTC (permalink / raw)
To: Eduardo Valentin; +Cc: linux-pm, linux-kernel
On Fri, 2013-12-06 at 09:52 -0400, Eduardo Valentin wrote:
> On 13-11-2013 14:11, Eduardo Valentin wrote:
> > As per Documentation/thermal/sysfs-api.txt, max_level
> > is an index, not a counter. Thus, in case a CPU has
> > 3 valid frequencies, max_level is expected to be 2, for instance.
> >
> > The current code makes max_level == number of valid frequencies,
> > which is bogus. This patch fix the cpu_cooling device by
> > ranging max_level properly.
> >
good catch.
> > Reported-by: Carlos Hernandez <ceh@ti.com>
> > Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
>
> Rui,
>
> Can you please consider pushing this fix?
>
applied.
> > ---
> > drivers/thermal/cpu_cooling.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> > index d179028..d0f8f8b5 100644
> > --- a/drivers/thermal/cpu_cooling.c
> > +++ b/drivers/thermal/cpu_cooling.c
> > @@ -173,6 +173,8 @@ static int get_property(unsigned int cpu, unsigned long input,
> > freq = table[i].frequency;
> > max_level++;
> > }
> > + /* max_level is an index, not a counter */
> > + max_level--;
> >
I think we should check the max_level first, like the patch I attached
below.
thanks,
rui
>From a116776f7b6052599df0c67db29c30ea9d69d7ee Mon Sep 17 00:00:00 2001
From: Zhang Rui <rui.zhang@intel.com>
Date: Thu, 2 Jan 2014 11:57:48 +0800
Subject: [PATCH] Thermal cpu cooling: return error if no valid cpu frequency
entry
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/thermal/cpu_cooling.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index cc556a8..bb486b4 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -173,6 +173,11 @@ static int get_property(unsigned int cpu, unsigned long input,
freq = table[i].frequency;
max_level++;
}
+
+ /* No valid cpu frequency entry */
+ if (max_level == 0)
+ return -EINVAL;
+
/* max_level is an index, not a counter */
max_level--;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] thermal: fix cpu_cooling max_level behavior
2014-01-02 4:02 ` Zhang Rui
@ 2014-01-04 13:53 ` Eduardo Valentin
0 siblings, 0 replies; 5+ messages in thread
From: Eduardo Valentin @ 2014-01-04 13:53 UTC (permalink / raw)
To: Zhang Rui; +Cc: Eduardo Valentin, linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2460 bytes --]
On 02-01-2014 00:02, Zhang Rui wrote:
> On Fri, 2013-12-06 at 09:52 -0400, Eduardo Valentin wrote:
>> On 13-11-2013 14:11, Eduardo Valentin wrote:
>>> As per Documentation/thermal/sysfs-api.txt, max_level
>>> is an index, not a counter. Thus, in case a CPU has
>>> 3 valid frequencies, max_level is expected to be 2, for instance.
>>>
>>> The current code makes max_level == number of valid frequencies,
>>> which is bogus. This patch fix the cpu_cooling device by
>>> ranging max_level properly.
>>>
> good catch.
>>> Reported-by: Carlos Hernandez <ceh@ti.com>
>>> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
>>
>> Rui,
>>
>> Can you please consider pushing this fix?
>>
> applied.
>
>>> ---
>>> drivers/thermal/cpu_cooling.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
>>> index d179028..d0f8f8b5 100644
>>> --- a/drivers/thermal/cpu_cooling.c
>>> +++ b/drivers/thermal/cpu_cooling.c
>>> @@ -173,6 +173,8 @@ static int get_property(unsigned int cpu, unsigned long input,
>>> freq = table[i].frequency;
>>> max_level++;
>>> }
>>> + /* max_level is an index, not a counter */
>>> + max_level--;
>>>
> I think we should check the max_level first, like the patch I attached
> below.
>
> thanks,
> rui
>
>>From a116776f7b6052599df0c67db29c30ea9d69d7ee Mon Sep 17 00:00:00 2001
> From: Zhang Rui <rui.zhang@intel.com>
> Date: Thu, 2 Jan 2014 11:57:48 +0800
> Subject: [PATCH] Thermal cpu cooling: return error if no valid cpu frequency
> entry
>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
> drivers/thermal/cpu_cooling.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
> index cc556a8..bb486b4 100644
> --- a/drivers/thermal/cpu_cooling.c
> +++ b/drivers/thermal/cpu_cooling.c
> @@ -173,6 +173,11 @@ static int get_property(unsigned int cpu, unsigned long input,
> freq = table[i].frequency;
> max_level++;
> }
> +
> + /* No valid cpu frequency entry */
> + if (max_level == 0)
> + return -EINVAL;
> +
Agreed. For the patch above:
Acked-by: Eduardo Valentin <eduardo.valentin@ti.com>
> /* max_level is an index, not a counter */
> max_level--;
>
>
--
You have got to be excited about what you are doing. (L. Lamport)
Eduardo Valentin
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 295 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-01-04 13:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-13 18:11 [PATCH 1/1] thermal: fix cpu_cooling max_level behavior Eduardo Valentin
2013-11-13 18:31 ` Eduardo Valentin
2013-12-06 13:52 ` Eduardo Valentin
2014-01-02 4:02 ` Zhang Rui
2014-01-04 13:53 ` Eduardo Valentin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).