[lm-sensors] [PATCH 17/17] drivers/hwmon/i5k_amb.c: fix checkpatch issues

Darrick J. Wong djwong at us.ibm.com
Mon Jan 9 17:32:19 CET 2012


On Sun, Jan 08, 2012 at 06:52:39PM +0100, Frans Meulenbroeks wrote:
> 2012/1/8 Guenter Roeck <guenter.roeck at ericsson.com>
> 
> > Hi Frans,
> >
> > On Sun, Jan 08, 2012 at 11:23:40AM -0500, Frans Meulenbroeks wrote:
> > > Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks at gmail.com>
> > > ---
> > >  drivers/hwmon/i5k_amb.c |   15 ++++++++++++---
> > >  1 files changed, 12 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/hwmon/i5k_amb.c b/drivers/hwmon/i5k_amb.c
> > > index d22f241..c516bdc 100644
> > > --- a/drivers/hwmon/i5k_amb.c
> > > +++ b/drivers/hwmon/i5k_amb.c
> > > @@ -159,7 +159,10 @@ static ssize_t store_amb_min(struct device *dev,
> > >  {
> > >       struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> > >       struct i5k_amb_data *data = dev_get_drvdata(dev);
> > > -     unsigned long temp = simple_strtoul(buf, NULL, 10) / 500;
> > > +     unsigned long temp;
> > > +     int ret = kstrtoul(buf, 10, &temp) / 500;
> >
> > This divides the error code by 500, not temp.
> >
> > > +     if (ret < 0)
> > > +             return ret;
> > >
> > >       if (temp > 255)
> > >               temp = 255;
> > > @@ -175,7 +178,10 @@ static ssize_t store_amb_mid(struct device *dev,
> > >  {
> > >       struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> > >       struct i5k_amb_data *data = dev_get_drvdata(dev);
> > > -     unsigned long temp = simple_strtoul(buf, NULL, 10) / 500;
> > > +     unsigned long temp;
> > > +     int ret = kstrtoul(buf, 10, &temp) / 500;
> >
> > Same here.
> >
> > > +     if (ret < 0)
> > > +             return ret;
> > >
> > >       if (temp > 255)
> > >               temp = 255;
> > > @@ -191,7 +197,10 @@ static ssize_t store_amb_max(struct device *dev,
> > >  {
> > >       struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> > >       struct i5k_amb_data *data = dev_get_drvdata(dev);
> > > -     unsigned long temp = simple_strtoul(buf, NULL, 10) / 500;
> > > +     unsigned long temp;
> > > +     int ret = kstrtoul(buf, 10, &temp) / 500;
> >
> > and here.
> >
> > Guenter
> >
> Yes of course!
> That is one of the risks of these edits. At some point too much autopilot
> creeps in.
> I'll send an update. There are not otehr places that hae this issue.
> 
> BTW, there is one other thing with this code that might be questionalble
> and that is the fact that temp is an unsigned long.
> Is it not possible with this sensor to have a temp < 0 (I'm assuming temp
> is degrees C or maybe F).
> This might also be an issue on other places. (actually I seem to recall
> having seen a place where the var was signed and the conversion function
> was unsigned (or the other way around))

According to section 14.5.3.6 of the Intel 6400/6402 AMB datasheet, the TEMP
register is an 8-bit value that measures AMB temperature from 0 to 127.5C in
0.5C increments.

--D





More information about the lm-sensors mailing list