Hi Jean, On Sat, Jul 09, 2011 at 04:36:18PM -0400, Jean Delvare wrote: > Hi Guenter, > > On Thu, 7 Jul 2011 13:42:44 -0700, Guenter Roeck wrote: > > Negative temperatures were returned in degrees C instead of milli-Degrees C. > > Also, negative temperatures were reported for remote temperature sensors even > > if the chip was configured for positive-only results. > > > > Fix by detecting temperature modes, and by treating negative temperatures > > similar to positive temperatures, with appropriate sign extension. > > > > Signed-off-by: Guenter Roeck <guenter.roeck@xxxxxxxxxxxx> > > --- > > Candidate for -stable. > > > > drivers/hwmon/lm95241.c | 20 ++++++++++++++------ > > 1 files changed, 14 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/hwmon/lm95241.c b/drivers/hwmon/lm95241.c > > index 01c638e..ec02e30 100644 > > --- a/drivers/hwmon/lm95241.c > > +++ b/drivers/hwmon/lm95241.c > > @@ -98,11 +98,16 @@ struct lm95241_data { > > }; > > > > /* Conversions */ > > -static int TempFromReg(u8 val_h, u8 val_l) > > +static int temp_from_reg_signed(u8 val_h, u8 val_l) > > { > > - if (val_h & 0x80) > > - return val_h - 0x100; > > - return val_h * 1000 + val_l * 1000 / 256; > > + s16 val_hl = (val_h << 8) | val_l; > > + return val_hl * 1000 / 256; > > +} > > + > > +static int temp_from_reg_unsigned(u8 val_h, u8 val_l) > > +{ > > + u16 val_hl = (val_h << 8) | val_l; > > + return val_hl * 1000 / 256; > > } > > > > static struct lm95241_data *lm95241_update_device(struct device *dev) > > @@ -135,10 +140,13 @@ static ssize_t show_input(struct device *dev, struct device_attribute *attr, > > char *buf) > > { > > struct lm95241_data *data = lm95241_update_device(dev); > > + int index = to_sensor_dev_attr(attr)->index; > > > > return snprintf(buf, PAGE_SIZE - 1, "%d\n", > > - TempFromReg(data->temp[to_sensor_dev_attr(attr)->index], > > - data->temp[to_sensor_dev_attr(attr)->index + 1])); > > + !index || (data->config & index) ? > > Hmm. This really should be: > > + !index || (data->config & (1 << (index / 2))) ? > > It just _happens_ that (1 << (index / 2)) == index for index values 2 > and 4, so your code works. But if we ever add support for a compatible > device with an extra temperature channel, it'll break. Not sure if the > rest of the driver code would properly cope with such a change anyway, > though. > > Furthermore, I think that readability could be slightly improved by > changing "!index" to "index == 0", as this is neither an error > condition nor a boolean). > Makes sense. I changed it to index == 0 || (data->config & (1 << (index / 2))) > Readability could be improved even more with driver-wide changes but > this is obviously out-of-scope for this patch. > Yes, I know. Another time, maybe. Thanks, Guenter _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors