Re: [PATCH] hwmon: Add support for max6695 and max6696 to lm90 driver

[Jean Delvare]

On Wed, 8 Sep 2010 03:28:16 -0700, Guenter Roeck wrote:
> Hi Jean,
> 
> On Mon, Sep 06, 2010 at 12:12:29PM -0400, Jean Delvare wrote:
> > Hi Guenter,
> > 
> > On Sat, 4 Sep 2010 15:34:35 -0700, Guenter Roeck wrote:
> > > Signed-off-by: Guenter Roeck <guenter.roeck@xxxxxxxxxxxx>
> > > ---
> > > To apply this patch, the previously submitted lm90 cleanup patch has to be
> > > applied first.
> > >
> > > My main concern with this patch is the chip detection code, specifically if it
> > > is able to safely distinguish between MAX6680/81 and MAX6695/96.
> > > Would be great to get some test coverage from a system with one of those chips.
> > 
> > Unfortunately I don't have any of these Maxim chips at hand. I have an
> > ADM1032 but it won't offer much coverage obviously. And I have dumps of
> > Maxim chips, but the real chips behave differently, so it's of little
> > help.
> > 
> Do you by any chance have register dumps of max6657/58/59 ?

Yes I do. I have dumps of max6658 and max6659 chips, I'm attaching
them. Please keep in mind that registers are read sequentially, in
order, and this matters because these chips return the last read value
whenever you access a nonexistent register. This even holds for
registers with fewer than 8 bits, e.g. config1.

> max6659 also supports a 3rd upper limit. Turns out it is convenient to add support
> for this limit first before adding support for max6696. To do that, I'll need
> to be able to distinguish between max6657/58 and max6659. My thought is to check
> for the address and if register 0x16 exists.
> 
> Also, do you happen to remember why address 0x4e for max6659 is not supported by the driver ?
> The code only says that it isn't supported, and it does not detect it, but there
> is not reason. sensors-detect does detect it at address 0x4e.

I think the reason is historical. As we never had a report of anyone
with a MAX6659 chip at 0x4e, and detection for this device is weak, and
the driver didn't support any other device at this address, it seemed
more reasonable to not scan the address at all. Now that the driver
supports more devices at this address and thus scans it anyway, I have
no objection supporting the MAX6659 at 0x4e.

-- 
Jean Delvare

Attachment: max6658.dump
Description: Binary data

Attachment: max6659.dump
Description: Binary data