lkml.org 
[lkml]   [2011]   [Jun]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH] hwmon: LM95245 driver
    Date
    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@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.

    Nice catch.

    > 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.

    Regards,
    Alexander


    \
     
     \ /
      Last update: 2011-06-23 11:17    [W:0.064 / U:128.072 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site