On Sunday 30 November 2014 10:00:00 Guenter Roeck wrote: > On 11/18/2014 06:56 AM, Pali Rohár wrote: > > Static array prev[] was incorrectly initialized. It should > > be initialized to some "invalid" temperature value (above > > I8K_MAX_TEMP). > > > > Next, function should store "invalid" value to prev[] (above > > I8K_MAX_TEMP), not valid (= I8K_MAX_TEMP) because whole > > temperature bug handling will not work. > > > > And last part, to not break existing detection of > > temperature sensors, register them also if i8k report too > > high temperature (above I8K_MAX_TEMP). This is needed > > because some sensors are sometimes turned off (e.g sensor > > on GPU which can be turned off/on) and in this case SMM > > report too high value. > > > > To prevent reporting "invalid" values to userspace, return > > -EINVAL. In this case sensors which are currently turned > > off (e.g optimus/powerexpress/enduro gpu) are reported as > > "N/A" by lm-sensors package. > > > > Signed-off-by: Pali Rohár <[email protected]> > > --- > > > > drivers/char/i8k.c | 16 ++++++++++------ > > 1 file changed, 10 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/char/i8k.c b/drivers/char/i8k.c > > index 7272b08..e34a019 100644 > > --- a/drivers/char/i8k.c > > +++ b/drivers/char/i8k.c > > @@ -298,7 +298,7 @@ static int i8k_get_temp(int sensor) > > > > int temp; > > > > #ifdef I8K_TEMPERATURE_BUG > > > > - static int prev[4]; > > + static int prev[4] = { I8K_MAX_TEMP+1, I8K_MAX_TEMP+1, > > I8K_MAX_TEMP+1, I8K_MAX_TEMP+1 }; > > > > #endif > > > > regs.ebx = sensor & 0xff; > > rc = i8k_smm(®s); > > > > @@ -317,10 +317,12 @@ static int i8k_get_temp(int sensor) > > > > */ > > > > if (temp > I8K_MAX_TEMP) { > > > > temp = prev[sensor]; > > > > - prev[sensor] = I8K_MAX_TEMP; > > + prev[sensor] = I8K_MAX_TEMP+1; > > > > } else { > > > > prev[sensor] = temp; > > > > } > > > > + if (temp > I8K_MAX_TEMP) > > + return -ERANGE; > > Can we return -ENODATA in this case ? I think that would be > more appropriate. >
This is internal kernel function, no problem. If you prefer NODATA instead RANGE I will change it. > > #endif > > > > return temp; > > > > @@ -499,6 +501,8 @@ static ssize_t > > i8k_hwmon_show_temp(struct device *dev, > > > > int temp; > > > > temp = i8k_get_temp(index); > > > > + if (temp == -ERANGE) > > + return -EINVAL; > > and can we also return -ENODATA to user space ? > This would make the code a bit cleaner. > > Thanks, > Guenter There was some problems when I tested similar patch for radeon.ko (do not report temperature to userspace when card is turned off). I can test lm-sensors which is in Ubuntu 12.04 LTS (there is probably some older version) what happens with -ENODATA from i8k. -- Pali Rohár [email protected]
signature.asc
Description: This is a digitally signed message part.
