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

From: Guenter Roeck <guenter.roeck@ericsson.com>
To: lm-sensors@vger.kernel.org
Subject: Re: [lm-sensors] [PATCH] hwmon: (max1668) Add support for
Date: Sun, 12 Jun 2011 00:52:52 +0000	[thread overview]
Message-ID: <20110612005252.GA28008@ericsson.com> (raw)
In-Reply-To: <20110611112818.670e5db8@endymion.delvare>

Hi Jean,
[ ... ]
> > > > +	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.
> 
Yes, that is what I figured. I also don't really believe in error path code optimization.

Anyway, in case you are interested, here are some chip register dumps for you:

MAX1668:

No size specified (using byte-data access)
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
00: 1c 1c 1c 00 00 80 00 00 7f c9 7f c9 7f c9 7f c9    ???..?..????????
10: 7f c9 05 05 05 05 05 05 05 05 05 05 05 05 05 05    ????????????????
20: 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05    ????????????????
30: 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05    ????????????????
40: 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05    ????????????????
50: 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05    ????????????????
60: 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05    ????????????????
70: 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05    ????????????????
80: 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05    ????????????????
90: 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05    ????????????????
a0: 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05    ????????????????
b0: 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05    ????????????????
c0: 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05    ????????????????
d0: 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05    ????????????????
e0: 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05 05    ????????????????
f0: 05 05 05 05 05 05 05 05 05 05 05 05 05 05 4d 05    ??????????????M?

MAX1805:

No size specified (using byte-data access)
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
00: 1a 1b 1b 1b 1b 80 00 00 7f c9 7f c9 7f c9 7f c9    ??????..????????
10: 7f c9 03 03 03 03 03 03 03 03 03 03 03 03 03 03    ????????????????
20: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03    ????????????????
30: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03    ????????????????
40: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03    ????????????????
50: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03    ????????????????
60: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03    ????????????????
70: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03    ????????????????
80: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03    ????????????????
90: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03    ????????????????
a0: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03    ????????????????
b0: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03    ????????????????
c0: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03    ????????????????
d0: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03    ????????????????
e0: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03    ????????????????
f0: 03 03 03 03 03 03 03 03 03 03 03 03 03 03 4d 03    ??????????????M?

Interesting is that the chip ID is returned in all unused registers,
even if I read an existing register first.

Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors