[lm-sensors] [PATCH] hwmon: LM95245 driver
alexander.stein at systec-electronic.com
Thu Jun 23 11:14:52 CEST 2011
On Wednesday 22 June 2011 21:15:49 Guenter Roeck wrote:
> On Tue, 2011-06-21 at 05:52 -0400, Alexander Stein wrote:
> > An hwmon driver for the National Semiconductor LM95245 dual temperature
> > sensors chip.
> > Signed-off-by: Alexander Stein <alexander.stein at systec-electronic.com>
> Overall pretty well written, only I am a bit at loss why you try to deal
> with the unsigned temperature registers. That adds a bit of complexity
> to the driver. Given that the signed temperature value range is up to
> 127 degrees C, and that the chip is only rated up to 125 degrees C, the
> added complexity doesn't seem to be worth it.
> Can you elaborate ?
Well, AFAIK the 125 degrees C limit is for the chip itself. I didn't found
anything about a remote diode limit. Also the remote TCRIT and OS limit range
up to 255 degrees C.
Another point is the optional offset register (not implemented, see below). I
could not found much about it, but I guess this is immediately added to the
remote temperature register.
> Other comments:
> For the interval attribute, idea would be to write the value into the
> conversion rate register. Your code does not match the interval with the
> rate programmed into the chip (which is the idea), nor does it update
> the rate if the interval is changed.
Well, I noticed that. But I went the way lm95241 does. I'm also unsure which
interval to choose, if user specify a unsupported interval. Choose the next
small or the next greater one? Maybe you can give me a hint here.
> Chip address 0x29 is missing.
> It would be nice if the driver would support limit, hysteresis, alarm,
> and fault attributes.
Let's see if I find time for that.
Thanks for the review though.
More information about the lm-sensors