[lm-sensors] [PATCH] hwmon: (max1668) Add support for tempX_fault attributes

[Jean Delvare]

On Sat, 11 Jun 2011 08:35:31 -0700, Guenter Roeck wrote:
> On Sat, Jun 11, 2011 at 05:28:18AM -0400, Jean Delvare wrote:
> > On Thu, 9 Jun 2011 18:14:07 -0700, Guenter Roeck wrote:
> > > root at groeck-desktop# sensors
> > > max1668-i2c-5-18
> > > Adapter: i2c-devantech-iss at bus 001 device 007
> > > temp1:        +23.0°C  (low  = -55.0°C, high = +127.0°C)
> > > temp2:        +25.0°C  (low  = -55.0°C, high = +127.0°C)
> > > temp3:        +25.0°C  (low  = -55.0°C, high = +127.0°C)
> > > temp4:        +25.0°C  (low  = -55.0°C, high = +127.0°C)
> > > temp5:          FAULT  (low  = -55.0°C, high = +127.0°C)    ALARM(MAX)
> > > 
> > >  drivers/hwmon/max1668.c |   26 +++++++++++++++++++++++++-
> > >  1 files changed, 25 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/drivers/hwmon/max1668.c b/drivers/hwmon/max1668.c
> > > index ddefb64..af85d44 100644
> > > --- a/drivers/hwmon/max1668.c
> > > +++ b/drivers/hwmon/max1668.c
> > > @@ -123,7 +123,7 @@ static struct max1668_data *max1668_update_device(struct device *dev)
> > >  		goto abort;
> > >  	}
> > >  	data->alarms &= 0x00ff;
> > > -	data->alarms |= ((u8) (val & 0x60)) << 8;
> > > +	data->alarms |= ((u8) (val & 0x70)) << 8;
> > 
> > The (u8) type cast doesn't seem needed. Same a few lines below for
> > MAX1668_REG_STAT2.
> > 
> Actually, turns out that
> 	data->alarms = val << 8;
> followed by
> 	data->alarms |= val;
> for MAX1668_REG_STAT2 is good enough.

I think that the original idea was to preserve as much of data->alarms
as possible in case one of the two reads failed. But given that the rest
of the driver code doesn't leverage from that, you can indeed simplify
it.

-- 
Jean Delvare