battery: current a factor of 10 off?

battery: current a factor of 10 off?

[Ferenc Wagner]

Hi,

on my ThinkPad R50e the current_now reading seems something like 10
times the actual value.  Documentation/power/power_supply_class.txt
says it's in uA, and a tipical discharge value is around 15259000,
that is, 15 A on my system.  At the same time voltage is 11756000,
that is, 12 V, which sounds reasonable, but then the power is 15*12 VA,
ie 180 W, which is insane.  However, the numerical derivative of
energy_now gives 18 W, which is both reasonable and consistent with
the assumed factor of 10 error in current_now.  Is this possibly a bug
in 2.6.26.6?  During a quick glance over battery.c nothing obvious
jumped at me, but I'm no expert on this.
-- 
Thanks,
Feri.

Re: battery: current a factor of 10 off?

[Henrique de Moraes Holschuh]

On Fri, 21 Nov 2008, Ferenc Wagner wrote:
> on my ThinkPad R50e the current_now reading seems something like 10
> times the actual value.  Documentation/power/power_supply_class.txt
> says it's in uA, and a tipical discharge value is around 15259000,
> that is, 15 A on my system.  At the same time voltage is 11756000,
> that is, 12 V, which sounds reasonable, but then the power is 15*12 VA,
> ie 180 W, which is insane.  However, the numerical derivative of
> energy_now gives 18 W, which is both reasonable and consistent with
> the assumed factor of 10 error in current_now.  Is this possibly a bug
> in 2.6.26.6?  During a quick glance over battery.c nothing obvious
> jumped at me, but I'm no expert on this.

You could also patch in tp_smapi and compare with the data you get from both
drivers.  If they are both wrong, you need a BIOS/EC firmware update.  If
there is none, we will need DMI quirks on both drivers to work around the
issue.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: battery: current a factor of 10 off?

[Alexey Starikovskiy]

[-- Attachment #1: Type: text/plain, Size: 1336 bytes --]

Hi Ferenc,

Good catch.
ACPI battery interface report its state either in mW or in mA, and
discharge rate
in your case is reported in mW. power_supply interface does not have
such a parameter,
so current_now parameter is used for all cases. But in case of mW,
reported discharge should
be converted into mA.

Please test the attached patch.

Thanks,
Alex.

On Fri, Nov 21, 2008 at 3:33 AM, Ferenc Wagner <wferi@niif.hu> wrote:
> Hi,
>
> on my ThinkPad R50e the current_now reading seems something like 10
> times the actual value.  Documentation/power/power_supply_class.txt
> says it's in uA, and a tipical discharge value is around 15259000,
> that is, 15 A on my system.  At the same time voltage is 11756000,
> that is, 12 V, which sounds reasonable, but then the power is 15*12 VA,
> ie 180 W, which is insane.  However, the numerical derivative of
> energy_now gives 18 W, which is both reasonable and consistent with
> the assumed factor of 10 error in current_now.  Is this possibly a bug
> in 2.6.26.6?  During a quick glance over battery.c nothing obvious
> jumped at me, but I'm no expert on this.
> --
> Thanks,
> Feri.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: convert_discharge_energy_to_current.patch --]
[-- Type: text/x-patch; name=convert_discharge_energy_to_current.patch, Size: 1255 bytes --]

ACPI:battery: Convert discharge energy rate to current properly

From: Alexey Starikovskiy <astarikovskiy@suse.de>

ACPI battery interface reports its state either in mW or in mA, and 
discharge rate in your case is reported in mW. power_supply interface 
does not have such a parameter, so current_now parameter is used 
for all cases. But in case of mW, reported discharge should
be converted into mA.

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
---

 drivers/acpi/battery.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)


diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 1423b0c..44f9586 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -174,6 +174,15 @@ static int acpi_battery_get_property(struct power_supply *psy,
 		break;
 	case POWER_SUPPLY_PROP_CURRENT_NOW:
 		val->intval = battery->current_now * 1000;
+		/* if power units are mW, convert to mA by 
+		   dividing by current voltage (mV/1000) */
+		if (!battery->power_unit) { 
+			if (battery->voltage_now) {
+				val->intval *= 1000;
+				val->intval /= battery->voltage_now;
+			} else 
+				val->intval = -1;
+		}
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
 	case POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN:

Re: battery: current a factor of 10 off?

[Alexey Starikovskiy]

[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]

Hi Ferenc,
This patch should be better -- no integer overflow.

Regards,
Alex.

Alexey Starikovskiy wrote:
> Hi Ferenc,
>
> Good catch.
> ACPI battery interface report its state either in mW or in mA, and
> discharge rate
> in your case is reported in mW. power_supply interface does not have
> such a parameter,
> so current_now parameter is used for all cases. But in case of mW,
> reported discharge should
> be converted into mA.
>
> Please test the attached patch.
>
> Thanks,
> Alex.
>
> On Fri, Nov 21, 2008 at 3:33 AM, Ferenc Wagner <wferi@niif.hu> wrote:
>   
>> Hi,
>>
>> on my ThinkPad R50e the current_now reading seems something like 10
>> times the actual value.  Documentation/power/power_supply_class.txt
>> says it's in uA, and a tipical discharge value is around 15259000,
>> that is, 15 A on my system.  At the same time voltage is 11756000,
>> that is, 12 V, which sounds reasonable, but then the power is 15*12 VA,
>> ie 180 W, which is insane.  However, the numerical derivative of
>> energy_now gives 18 W, which is both reasonable and consistent with
>> the assumed factor of 10 error in current_now.  Is this possibly a bug
>> in 2.6.26.6?  During a quick glance over battery.c nothing obvious
>> jumped at me, but I'm no expert on this.
>> --
>> Thanks,
>> Feri.
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>     


[-- Attachment #2: convert_discharge_energy_to_current.patch --]
[-- Type: text/x-diff, Size: 1255 bytes --]

ACPI:battery: Convert discharge energy rate to current properly

From: Alexey Starikovskiy <astarikovskiy@suse.de>

ACPI battery interface reports its state either in mW or in mA, and 
discharge rate in your case is reported in mW. power_supply interface 
does not have such a parameter, so current_now parameter is used 
for all cases. But in case of mW, reported discharge should
be converted into mA.

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
---

 drivers/acpi/battery.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)


diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 1423b0c..0ebc04c 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -174,6 +174,15 @@ static int acpi_battery_get_property(struct power_supply *psy,
 		break;
 	case POWER_SUPPLY_PROP_CURRENT_NOW:
 		val->intval = battery->current_now * 1000;
+		/* if power units are mW, convert to mA by 
+		   dividing by current voltage (mV/1000) */
+		if (!battery->power_unit) { 
+			if (battery->voltage_now) {
+				val->intval /= battery->voltage_now;
+				val->intval *= 1000;
+			} else 
+				val->intval = -1;
+		}
 		break;
 	case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN:
 	case POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN:

Re: battery: current a factor of 10 off? [fix confirmed]

[Ferenc Wagner]

Alexey Starikovskiy <aystarik@gmail.com> writes:

> Alexey Starikovskiy wrote:
>
>> On Fri, Nov 21, 2008 at 3:33 AM, Ferenc Wagner <wferi@niif.hu> wrote:
>>
>>> on my ThinkPad R50e the current_now reading seems something like 10
>>> times the actual value.  [...]
>>
>> Good catch.  ACPI battery interface report its state either in mW
>> or in mA, and discharge rate in your case is reported in
>> mW. power_supply interface does not have such a parameter, so
>> current_now parameter is used for all cases. But in case of mW,
>> reported discharge should be converted into mA.
>>
>> Please test the attached patch.
>
> This patch should be better -- no integer overflow.

Hi Alex,

Thanks, this patch really fixes the issue (tried on 2.6.28-rc6).
Hope to see it soon in Linus' tree!

Btw. that kernel gives me two ACPI warnings on boot:

  ACPI: RSDP 000F6E40, 0024 (r2 IBM   )
  ACPI: XSDT 1F6EF33D, 004C (r1 IBM    TP-1W        2060  LTP        0)
  ACPI: FACP 1F6EF400, 00F4 (r3 IBM    TP-1W        2060 IBM         1)
  ACPI Warning (tbfadt-0460): Optional field "Gpe1Block" has zero address or length: 000000000000102C/0 [20080926]
  ACPI: DSDT 1F6EF5E7, 7865 (r1 IBM    TP-1W        2060 MSFT  100000E)
  ACPI: FACS 1F6F8000, 0040
  ACPI: SSDT 1F6EF5B4, 0033 (r1 IBM    TP-1W        2060 MSFT  100000E)
  ACPI: ECDT 1F6F6E4C, 0052 (r1 IBM    TP-1W        2060 IBM         1)
  ACPI: TCPA 1F6F6E9E, 0032 (r1 IBM    TP-1W        2060 PTL         1)
  ACPI: BOOT 1F6F6FD8, 0028 (r1 IBM    TP-1W        2060  LTP        1)
  [...]
  ACPI Warning (nspredef-0858): \_SB_.PCI0.LPC_.EC__.BAT0._BIF: Return Package type mismatch at index 9 - found Buffer, expected String [20080926]
  ACPI: Battery Slot [BAT0] (battery present)

I'm willing to provide further info if anybody here is interested.
-- 
Cheers,
Feri.

Re: battery: current a factor of 10 off? [fix confirmed]

[Alexey Starikovskiy]

Ferenc Wagner wrote:
> Alexey Starikovskiy <aystarik@gmail.com> writes:
>
>   
>> Alexey Starikovskiy wrote:
>>
>>     
>>> On Fri, Nov 21, 2008 at 3:33 AM, Ferenc Wagner <wferi@niif.hu> wrote:
>>>
>>>       
>>>> on my ThinkPad R50e the current_now reading seems something like 10
>>>> times the actual value.  [...]
>>>>         
>>> Good catch.  ACPI battery interface report its state either in mW
>>> or in mA, and discharge rate in your case is reported in
>>> mW. power_supply interface does not have such a parameter, so
>>> current_now parameter is used for all cases. But in case of mW,
>>> reported discharge should be converted into mA.
>>>
>>> Please test the attached patch.
>>>       
>> This patch should be better -- no integer overflow.
>>     
>
> Hi Alex,
>
> Thanks, this patch really fixes the issue (tried on 2.6.28-rc6).
> Hope to see it soon in Linus' tree!
>   
Great! Do you want to be memorized by "Tested-by:" line?
> Btw. that kernel gives me two ACPI warnings on boot:
>
>   ACPI: RSDP 000F6E40, 0024 (r2 IBM   )
>   ACPI: XSDT 1F6EF33D, 004C (r1 IBM    TP-1W        2060  LTP        0)
>   ACPI: FACP 1F6EF400, 00F4 (r3 IBM    TP-1W        2060 IBM         1)
>   ACPI Warning (tbfadt-0460): Optional field "Gpe1Block" has zero address or length: 000000000000102C/0 [20080926]
>   
Almost all machines produce this... Spec defines 2 GPE blocks as 
default, so we complain if only one is used, but that happens quite 
often...
>   ACPI: DSDT 1F6EF5E7, 7865 (r1 IBM    TP-1W        2060 MSFT  100000E)
>   ACPI: FACS 1F6F8000, 0040
>   ACPI: SSDT 1F6EF5B4, 0033 (r1 IBM    TP-1W        2060 MSFT  100000E)
>   ACPI: ECDT 1F6F6E4C, 0052 (r1 IBM    TP-1W        2060 IBM         1)
>   ACPI: TCPA 1F6F6E9E, 0032 (r1 IBM    TP-1W        2060 PTL         1)
>   ACPI: BOOT 1F6F6FD8, 0028 (r1 IBM    TP-1W        2060  LTP        1)
>   [...]
>   ACPI Warning (nspredef-0858): \_SB_.PCI0.LPC_.EC__.BAT0._BIF: Return Package type mismatch at index 9 - found Buffer, expected String [20080926]
>   
This one also could be treated as false positive -- spec says it should 
be string, but almost all BIOS implementations return buffer here.
>   ACPI: Battery Slot [BAT0] (battery present)
>
> I'm willing to provide further info if anybody here is interested.
>   

These issues are already known, and second even has a patch lurking 
somewhere in this list... So no action required.

Thanks again,
Alex.

Re: battery: current a factor of 10 off? [fix confirmed]

[Ferenc Wagner]

Alexey Starikovskiy <aystarik@gmail.com> writes:

> Ferenc Wagner wrote:
>
>> Alexey Starikovskiy <aystarik@gmail.com> writes:
>>
>>> Alexey Starikovskiy wrote:
>>>
>>>> On Fri, Nov 21, 2008 at 3:33 AM, Ferenc Wagner <wferi@niif.hu> wrote:
>>>>
>>>>> on my ThinkPad R50e the current_now reading seems something like 10
>>>>> times the actual value.  [...]
>>>>
>>>> Good catch.  ACPI battery interface report its state either in mW
>>>> or in mA, and discharge rate in your case is reported in
>>>> mW. power_supply interface does not have such a parameter, so
>>>> current_now parameter is used for all cases. But in case of mW,
>>>> reported discharge should be converted into mA.
>>>>
>>>> Please test the attached patch.
>>>
>>> This patch should be better -- no integer overflow.
>>
>> Thanks, this patch really fixes the issue (tried on 2.6.28-rc6).
>> Hope to see it soon in Linus' tree!
>
> Great! Do you want to be memorized by "Tested-by:" line?

Feel free to quote me if you see fit.

>> Btw. that kernel gives me two ACPI warnings on boot:
>
> These issues are already known, and second even has a patch lurking
> somewhere in this list... So no action required.

Glad to hear.  Thanks for the good work!
-- 
Cheers,
Feri.

Re: battery: current a factor of 10 off?

[Len Brown]

trailing whitespace repaired.
applied for 2.6.28.

thanks,
-Len


On Fri, 21 Nov 2008, Alexey Starikovskiy wrote:

> Hi Ferenc,
> This patch should be better -- no integer overflow.
> 
> Regards,
> Alex.
> 
> Alexey Starikovskiy wrote:
> > Hi Ferenc,
> > 
> > Good catch.
> > ACPI battery interface report its state either in mW or in mA, and
> > discharge rate
> > in your case is reported in mW. power_supply interface does not have
> > such a parameter,
> > so current_now parameter is used for all cases. But in case of mW,
> > reported discharge should
> > be converted into mA.
> > 
> > Please test the attached patch.
> > 
> > Thanks,
> > Alex.
> > 
> > On Fri, Nov 21, 2008 at 3:33 AM, Ferenc Wagner <wferi@niif.hu> wrote:
> >   
> > > Hi,
> > > 
> > > on my ThinkPad R50e the current_now reading seems something like 10
> > > times the actual value.  Documentation/power/power_supply_class.txt
> > > says it's in uA, and a tipical discharge value is around 15259000,
> > > that is, 15 A on my system.  At the same time voltage is 11756000,
> > > that is, 12 V, which sounds reasonable, but then the power is 15*12 VA,
> > > ie 180 W, which is insane.  However, the numerical derivative of
> > > energy_now gives 18 W, which is both reasonable and consistent with
> > > the assumed factor of 10 error in current_now.  Is this possibly a bug
> > > in 2.6.26.6?  During a quick glance over battery.c nothing obvious
> > > jumped at me, but I'm no expert on this.
> > > --
> > > Thanks,
> > > Feri.
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > 
> > >     
> 
>