Re: LM90 driver and ti tmp461 detection

[Guenter Roeck]

On 12/10/2015 09:12 AM, Andreas Werner wrote:
> On Thu, Dec 10, 2015 at 08:41:57AM -0800, Guenter Roeck wrote:
> > On 12/10/2015 04:08 AM, Andreas Werner wrote:
> > > Hi,
> > > i have a temperature sensor device named "TI TMP461" which is quite the
> > > same than the tmp451 which is already included in the lm90 driver.
> > >
> > > I just want to add the tmp461 to the driver but currently i have no way
> > > to differ between the tmp461 and tmp451.
> > >
> > > The main different is that the tmp461 address can be configured by to Pins
> > > whereas the tmp451 has a fixed address.
> > >
> > > The register layout is quite the same except that the tmp461 has one
> > > register more (0x16 Channel Enable Register).
> > >
> > > Every zero bit is the same for both bits, so we cannot use these bits for
> > > identification.
> > >
> > > Does anybody of you have a tmp451 to read out the register 0x16 (which is
> > > not implement but i expect a value). May be we can use this to
> > > identify the tmp461.
> >
> > Here is a register dump.
> >
> > 00: 5d 69 00 04 08 bf 00 ff 53 00 00 00 00 00 00 00    ]i.???..S.......
> > 10: a0 00 00 00 00 f0 00 00 00 a9 00 00 00 00 00 00    ?....?...?......
> > 20: b8 0a 01 00 00 XX 00 00 00 00 0e 00 00 00 00 00    ???..X....?.....
> > 30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> > 40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> > 50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> > 60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> > 70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> > 80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> > 90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> > a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> > b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> > c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> > d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> > e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
> > f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 55 00    ..............U.
> >
> > there is nothing to distinguish the two chips as far as I can see.
> > Is your system a devicetree system ? If so, you can just
> > provide a compatible statement such as
> > 	compatible = "ti,tmp461", "ti,tmp451";
> > or simply instantiate the chip as tmp451.
> >
> > Feel free to provide a patch adding '{ "tmp461", tmp451 },'
> > to lm90_id as well as the necessary documentation, but I don't
> > think there is anything we can do to detect the chip.
> >
> > On the other side, I am a bit concerned that the chip detection doesn't
> > work well, since the chip ID register is really unsupported and just happens
> > to return 0 for both chips. I wonder if there are other TI chips which may
> > be mis-detected as TMP451.
> >
> > Thanks,
> > Guenter
>
> Hi,
> thanks for the register dump :-)
>
> There is one difference. The temperature ranges differ:
>
> TMP451:
> Standard Mode: 0 to +127
> Extended Mode: -64 to +191
>
> TMP461:
> Standard Mode: -40 to +127
> Extended Mode: -64 to +191
>
> Therefore the calcualtion is different then the tmp451 one.
Not really. The tmp451 will return 0 for negative temperatures in standard mode,
the tmp461 will return negative (signed) values. Temperatures are stored
as signed values, so the code should be ok as is. Am I missing something ?

> I also aggree with you with the magical ID register. I hate such
> kind of register with are not documented and may have a value.
>
> I have a DT based board yes, but i have also planed to submit my changes
> and also wanted to implement the autodetection of the chip.

I don't think auto-detection (other than handling the chip as tmp451)
will be possible, but I am looking forward to seeing your patch(es).

> I have also planned (or need) to implement the "n-Factor Correction" for the
> CPU diodes. There is a register to set this value in the tmp451 and tmp461.
> Would that make sense to add a DT property for the driver to be able
> to set this value? What do you think?
Yes, since this is a board (hardware) property. Other chips have similar
registers. We'll just have to find a good set of property names and descriptions.

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/