Re: [lm-sensors] [PATCH] hwmon: (ina2xx) Change register cache to signed

From: Fabio Baltieri
Date: Sun Jun 08 2014 - 16:54:41 EST


On Sun, Jun 8, 2014 at 9:30 PM, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> On Sun, Jun 08, 2014 at 01:16:00PM -0700, Guenter Roeck wrote:
>> On Sat, Jun 07, 2014 at 09:47:01PM +0100, Fabio Baltieri wrote:
>> > All devices supported by the ina2xx driver are bidirectional and reports
>> > the measured value as a signed 16 bit, but the current driver
>> > implementation caches the number as an u16, leading to an incorrect sign
>> > extension when reporting to the userspace in ina2xx_get_value().
>> >
>> > This patch fixes the problem by using a s16 instead, and has been tested
>> > on an INA219.
>> >
>> > Signed-off-by: Fabio Baltieri <fabio.baltieri@xxxxxxxxx>
>>
>> Applied.
>>
> Actually, no, this won't work. The statement above is only correct for current
> and shunt voltage measurements, but not for power measurements and not for bus
> voltage measurements. Changing the register to s16 won't help; conversion needs
> to be done in ina2xx_get_value() for shunt voltage and current measurement only.
> Otherwise we just move the bug from current/shunt voltage measurements to
> power / bus voltage measurements.
>
> Even more interesting, the power is supposed to be the product of Bus voltage
> and current, and the latter can be negative. However, the power register
> description does not suggest that the upper bit would be a sign bit. So there
> is some discrepancy in the datasheet, and we'll need some real-world data to
> understand if the upper power bit is signed or not.

Hi Guenter,

looks like you're right here, I wasn't paying too attention to the
power register and it actually always reads positive, even when the
current is flowing in the reverse direction, real data from the
ina219:

[55694.263502] shunt_voltage=fb46
[55694.263691] bus_voltage=20c2
[55694.263847] power=00fe
[55694.263954] current=fb46

I'll send a v2 to only cast the two signed register then.

Many thanks for spotting this!

Fabio

--
Fabio Baltieri
--
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/