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 _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
