Patch to correct battery capacity values on Thinkpads (repost)

Patch to correct battery capacity values on Thinkpads (repost)

[Kamil Iskra]

[ Note: this is a follow-up on
https://bugzilla.kernel.org/show_bug.cgi?id=41062; in particular, check
comments #4, #19, and #21 ]

Please find below a patch to drivers/acpi/battery.c that adds a quirk to
correctly report battery capacity on 2010 and 2011 Lenovo Thinkpad models.
I think the patch is ready to be applied (Len: please apply).  However,
since it specifically affects Thinkpads, I've copied ibm-acpi-devel ML on
this message to give them a chance to provide input, if any.

The affected models that I tested (x201, t410, t410s, and x220) exhibit a
problem where, when battery capacity reporting unit is mAh, the values
being reported are wrong.  Pre-2010 and 2012 models appear to always report
in mWh and are thus unaffected.  Also, in mid-2012 Lenovo issued a BIOS
update for the 2011 models that fixes the issue (tested on x220 with a
post-1.29 BIOS).  No such update is available for the 2010 models, so those
still need this patch.

Problem description: for some reason, the affected Thinkpads switch the
reporting unit between mAh and mWh; generally, mAh is used when a laptop is
plugged in and mWh when it's unplugged, although a suspend/resume or
rmmod/modprobe is needed for the switch to take effect.  The values
reported in mAh are *always* wrong.  This does not appear to be a kernel
regression; I believe that the values were never reported correctly.  I
tested back to kernel 2.6.34, with multiple machines and BIOS versions.

Simply plugging a laptop into mains before turning it on is enough to
reproduce the problem.  Here's a sample /proc/acpi/battery/BAT0/info from
Thinkpad x220 (before a BIOS update) with a 4-cell battery:

present:                 yes
design capacity:         2886 mAh
last full capacity:      2909 mAh
battery technology:      rechargeable
design voltage:          14800 mV
design capacity warning: 145 mAh
design capacity low:     13 mAh
cycle count:              0
capacity granularity 1:  1 mAh
capacity granularity 2:  1 mAh
model number:            42T4899
serial number:           21064
battery type:            LION
OEM info:                SANYO

Once the laptop switches the unit to mWh (unplug from mains, suspend,
resume), the output changes to:

present:                 yes
design capacity:         28860 mWh
last full capacity:      29090 mWh
battery technology:      rechargeable
design voltage:          14800 mV
design capacity warning: 1454 mWh
design capacity low:     200 mWh
cycle count:              0
capacity granularity 1:  1 mWh
capacity granularity 2:  1 mWh
model number:            42T4899
serial number:           21064
battery type:            LION
OEM info:                SANYO

Can you see how the values for "design capacity", etc., differ by a factor
of 10 instead of 14.8 (the design voltage of this battery)?  On the battery
itself it says: 14.8V, 1.95Ah, 29Wh, so clearly the values reported in mWh
are correct and the ones in mAh are not.

My guess is that this problem has been around ever since those machines
have been released, but because the most common Thinkpad batteries are
rated at 10.8V, the error (8%) is small enough that it simply hasn't been
noticed or at least nobody could be bothered to look into it.

My patch works around the problem by adjusting the incorrectly reported mAh
values by "10000 / design_voltage".  The patch also has code to figure out
if it should be activated or not.  It only activates on Lenovo Thinkpads,
only when the unit is mAh, and, as an extra precaution, only when the
battery capacity reported through ACPI does not match what is reported
through DMI (I've never encountered a machine where the first two
conditions would be true but the last would not, but better safe than
sorry).

I've been using this patch for close to a year on several systems without
any problems.  I believe it is ready to be applied, although I lack kernel
hacking experience to make that determination.  The patch was generated
agains 3.7-rc5.

Note that I'm not on either list so I would appreciate a Cc on any replies.
I will also try to monitor the list archives.

Kamil Iskra


--- battery.c.orig	2012-11-11 06:44:33.000000000 -0600
+++ battery.c	2012-11-11 20:03:01.589851544 -0600
@@ -34,6 +34,7 @@
 #include <linux/dmi.h>
 #include <linux/slab.h>
 #include <linux/suspend.h>
+#include <asm/unaligned.h>
 
 #ifdef CONFIG_ACPI_PROCFS_POWER
 #include <linux/proc_fs.h>
@@ -95,6 +96,18 @@ enum {
 	ACPI_BATTERY_ALARM_PRESENT,
 	ACPI_BATTERY_XINFO_PRESENT,
 	ACPI_BATTERY_QUIRK_PERCENTAGE_CAPACITY,
+	/* On Lenovo Thinkpad models from 2010 and 2011, the power unit
+	   switches between mWh and mAh depending on whether the system
+	   is running on battery or not.  When mAh is the unit, most
+	   reported values are incorrect and need to be adjusted by
+	   10000/design_voltage.  Verified on x201, t410, t410s, and x220.
+	   Pre-2010 and 2012 models appear to always report in mWh and
+	   are thus unaffected (tested with t42, t61, t500, x200, x300,
+	   and x230).  Also, in mid-2012 Lenovo issued a BIOS update for
+	   the 2011 models that fixes the issue (tested on x220 with a
+	   post-1.29 BIOS), but as of Nov. 2012, no such update is
+	   available for the 2010 models.  */
+	ACPI_BATTERY_QUIRK_THINKPAD_MAH,
 };
 
 struct acpi_battery {
@@ -438,6 +451,21 @@ static int acpi_battery_get_info(struct
 	kfree(buffer.pointer);
 	if (test_bit(ACPI_BATTERY_QUIRK_PERCENTAGE_CAPACITY, &battery->flags))
 		battery->full_charge_capacity = battery->design_capacity;
+	if (test_bit(ACPI_BATTERY_QUIRK_THINKPAD_MAH, &battery->flags) &&
+	    battery->power_unit && battery->design_voltage) {
+		battery->design_capacity = battery->design_capacity *
+		    10000 / battery->design_voltage;
+		battery->full_charge_capacity = battery->full_charge_capacity *
+		    10000 / battery->design_voltage;
+		battery->design_capacity_warning =
+		    battery->design_capacity_warning *
+		    10000 / battery->design_voltage;
+		/* Curiously, design_capacity_low, unlike the rest of them,
+		   is correct.  */
+		/* capacity_granularity_* equal 1 on the systems tested, so
+		   it's impossible to tell if they would need an adjustment
+		   or not if their values were higher.  */
+	}
 	return result;
 }
 
@@ -486,6 +514,11 @@ static int acpi_battery_get_state(struct
 	    && battery->capacity_now >= 0 && battery->capacity_now <= 100)
 		battery->capacity_now = (battery->capacity_now *
 				battery->full_charge_capacity) / 100;
+	if (test_bit(ACPI_BATTERY_QUIRK_THINKPAD_MAH, &battery->flags) &&
+	    battery->power_unit && battery->design_voltage) {
+		battery->capacity_now = battery->capacity_now *
+		    10000 / battery->design_voltage;
+	}
 	return result;
 }
 
@@ -595,6 +628,24 @@ static void sysfs_remove_battery(struct
 	mutex_unlock(&battery->sysfs_lock);
 }
 
+static void find_battery(const struct dmi_header *dm, void *private)
+{
+	struct acpi_battery *battery = (struct acpi_battery *)private;
+	/* Note: the hardcoded offsets below have been extracted from
+	   the source code of dmidecode.  */
+	if (dm->type == DMI_ENTRY_PORTABLE_BATTERY && dm->length >= 8) {
+		const u8 *dmi_data = (const u8 *)(dm + 1);
+		int dmi_capacity = get_unaligned((const u16 *)(dmi_data + 6));
+		if (dm->length >= 18)
+			dmi_capacity *= dmi_data[17];
+		if (battery->design_capacity * battery->design_voltage / 1000
+		    != dmi_capacity &&
+		    battery->design_capacity * 10 == dmi_capacity)
+			set_bit(ACPI_BATTERY_QUIRK_THINKPAD_MAH,
+				&battery->flags);
+	}
+}
+
 /*
  * According to the ACPI spec, some kinds of primary batteries can
  * report percentage battery remaining capacity directly to OS.
@@ -620,6 +671,32 @@ static void acpi_battery_quirks(struct a
 		battery->capacity_now = (battery->capacity_now *
 				battery->full_charge_capacity) / 100;
 	}
+
+	if (test_bit(ACPI_BATTERY_QUIRK_THINKPAD_MAH, &battery->flags))
+		return ;
+
+	if (battery->power_unit && dmi_name_in_vendors("LENOVO")) {
+		const char *s;
+		s = dmi_get_system_info(DMI_PRODUCT_VERSION);
+		if (s && !strnicmp(s, "ThinkPad", 8)) {
+			dmi_walk(find_battery, battery);
+			if (test_bit(ACPI_BATTERY_QUIRK_THINKPAD_MAH,
+				     &battery->flags) &&
+			    battery->design_voltage) {
+				battery->design_capacity =
+				    battery->design_capacity *
+				    10000 / battery->design_voltage;
+				battery->full_charge_capacity =
+				    battery->full_charge_capacity *
+				    10000 / battery->design_voltage;
+				battery->design_capacity_warning =
+				    battery->design_capacity_warning *
+				    10000 / battery->design_voltage;
+				battery->capacity_now = battery->capacity_now *
+				    10000 / battery->design_voltage;
+			}
+		}
+	}
 }
 
 static int acpi_battery_update(struct acpi_battery *battery)

-- 
Kamil Iskra  kamil@iskra.name

Re: Patch to correct battery capacity values on Thinkpads (repost)

[Henrique de Moraes Holschuh]

On Sun, 11 Nov 2012, Kamil Iskra wrote:
> Please find below a patch to drivers/acpi/battery.c that adds a quirk to
> correctly report battery capacity on 2010 and 2011 Lenovo Thinkpad models.
> I think the patch is ready to be applied (Len: please apply).  However,
> since it specifically affects Thinkpads, I've copied ibm-acpi-devel ML on
> this message to give them a chance to provide input, if any.

FWIW,

Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>


-- 
  "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: Patch to correct battery capacity values on Thinkpads (repost)

[Matthew Garrett]

On Sun, Nov 11, 2012 at 11:09:35PM -0600, Kamil Iskra wrote:

> My guess is that this problem has been around ever since those machines
> have been released, but because the most common Thinkpad batteries are
> rated at 10.8V, the error (8%) is small enough that it simply hasn't been
> noticed or at least nobody could be bothered to look into it.

Yeah, the DSDT simply multiples the values by 10 when it's in this 
state. But the issue is purely cosmetic, right? I can't see any reason 
anything would be using the design voltage in calculations.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: Patch to correct battery capacity values on Thinkpads (repost)

[Peter Stuge]

Matthew Garrett wrote:
> > My guess is that this problem has been around ever since those machines
> > have been released, but because the most common Thinkpad batteries are
> > rated at 10.8V, the error (8%) is small enough that it simply hasn't been
> > noticed or at least nobody could be bothered to look into it.
> 
> Yeah, the DSDT simply multiples the values by 10 when it's in this 
> state. But the issue is purely cosmetic, right? I can't see any reason 
> anything would be using the design voltage in calculations.

current energy percent = 100 * energy_now / energy_full_design


//Peter

Re: Patch to correct battery capacity values on Thinkpads (repost)

[Matthew Garrett]

On Wed, Nov 14, 2012 at 10:58:57AM +0100, Peter Stuge wrote:
> Matthew Garrett wrote:
> > > My guess is that this problem has been around ever since those machines
> > > have been released, but because the most common Thinkpad batteries are
> > > rated at 10.8V, the error (8%) is small enough that it simply hasn't been
> > > noticed or at least nobody could be bothered to look into it.
> > 
> > Yeah, the DSDT simply multiples the values by 10 when it's in this 
> > state. But the issue is purely cosmetic, right? I can't see any reason 
> > anything would be using the design voltage in calculations.
> 
> current energy percent = 100 * energy_now / energy_full_design

Right. That's not the voltage.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: Patch to correct battery capacity values on Thinkpads (repost)

[Kamil Iskra]

On Wed, Nov 14, 2012 at 07:41:13 +0000, Matthew Garrett wrote:

> On Sun, Nov 11, 2012 at 11:09:35PM -0600, Kamil Iskra wrote:
> > My guess is that this problem has been around ever since those machines
> > have been released, but because the most common Thinkpad batteries are
> > rated at 10.8V, the error (8%) is small enough that it simply hasn't been
> > noticed or at least nobody could be bothered to look into it.
> Yeah, the DSDT simply multiples the values by 10 when it's in this 
> state. But the issue is purely cosmetic, right? I can't see any reason 
> anything would be using the design voltage in calculations.

Well, when the capacity unit is mAh, the correct way of translating it to
mWh is to multiply by the design voltage, AFAIK, and not the current
voltage (which seems to fluctuate a lot and is often higher than the design
voltage for whatever weird reason).  That's what upower does, for example.

I validated that with the Thinkpad-specific tp_smapi out-of-tree driver,
e.g., right now on my x201 (with my patch; values without the patch are in
parentheses):

/proc/acpi/battery/BAT0/info:
design capacity:         5200 mAh (5616 mAh)
design voltage:          10800 mV
/proc/acpi/battery/BAT0/state:
remaining capacity:      3418 mAh (3691 mAh)
present voltage:         12249 mV

/sys/devices/platform/smapi/BAT0 (capacities are always in mWh here):
design_capacity:         56160
design_voltage:          10800
remaining_capacity:      36920
voltage:                 12249

Few tools need to do the translation from mAh to mWh, though -- remaining
battery capacity monitors on the desktop just display percentages, so they
don't need to care about the units so long as capacity_now/design_capacity
is sane (which it is, with or without the patch).  One tool that certainly
does care though is PowerTOP, although, curiously, it seems to use current
voltage for calculations, resulting in incorrect values (with or without
the patch -- worse without).

Kamil

-- 
Kamil Iskra  kamil@iskra.name

Re: Patch to correct battery capacity values on Thinkpads (repost)

[Rafael J. Wysocki]

On Monday, November 12, 2012 06:22:03 PM Henrique de Moraes Holschuh wrote:
> On Sun, 11 Nov 2012, Kamil Iskra wrote:
> > Please find below a patch to drivers/acpi/battery.c that adds a quirk to
> > correctly report battery capacity on 2010 and 2011 Lenovo Thinkpad models.
> > I think the patch is ready to be applied (Len: please apply).  However,
> > since it specifically affects Thinkpads, I've copied ibm-acpi-devel ML on
> > this message to give them a chance to provide input, if any.
> 
> FWIW,
> 
> Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

Applied to the linux-next branch of the linux-pm.git tree as v3.8 material.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.