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 <c...@ti.com> >>> Signed-off-by: Eduardo Valentin <eduardo.valen...@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.zh...@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.zh...@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.valen...@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
signature.asc
Description: OpenPGP digital signature
