Comment 2 for bug 421985

Revision history for this message
Pete Graner (pgraner) wrote : [Fwd: [Fwd: [patch 1/9] acpi battery: work around negative s16 battery current on Acer]]

Pasting in for completeness since this started as a mail thread.

-------- Original Message --------
Subject: [Fwd: [patch 1/9] acpi battery: work around negative s16
battery current on Acer]
Date: Mon, 31 Aug 2009 14:09:22 +0200
From: Stefan Bader <email address hidden>
To: Pete Graner <email address hidden>

Hi Pete,

while walking over the linux-acpi list, this has been just accepted and it
looks pretty much like fitting. From your acpi dump I see it is using mA
and
does a calculation which takes a 16bit value from the EC and then subtracts
65536 from that. Though that should not directly cause a battery critical
warning (this is stored in another bit of the EC), but maybe gnome power
manager tries to be smart about the discharge rate. It might be an
interesting
experiment to have the battery info logged in a tight loop and then
unplug ac.
At least a bit of it should make it to disk before suspending.

-Stefan

-------- Original Message --------
Subject: [patch 1/9] acpi battery: work around negative s16 battery
current on Acer
Date: Thu, 06 Aug 2009 15:57:48 -0700
From: <email address hidden>
To: <email address hidden>
CC: <email address hidden>, <email address hidden>,
<email address hidden>, <email address hidden>

From: Hector Martin <email address hidden>

Acer Aspire 8930G laptops (and possibly others) report the battery current
as a 16-bit signed negative when it is charging. It also reports it as
0x10000 when the current is 0. This patch adds a quirk for this which
takes the absolute value of the reported current cast to an s16. This is
a DSDT bug present in the latest BIOS revision (the EC register is 16 bits
signed and the DSDT attempts to take the 16-bit two's complement of this,
which works for discharge but not charge. It also breaks zero values
because a 32-bit register is used and the high bits aren't thrown away).

I've enabled this for all Acer systems which report in mA units. This
should be safe since it won't break compliant systems unless they report a
current above 32A, which is insane. The patch also detects the valid
32-bit value -1, which indicates unknown status, and does not attempt the
fix in that case (note that this does not conflict with 16-bit -1, which
is 65535 as read normally and gets translated to 1mA).

Signed-off-by: Hector Martin <email address hidden>
Cc: Alexey Starikovskiy <email address hidden>
Cc: Len Brown <email address hidden>
Signed-off-by: Andrew Morton <email address hidden>
---

  drivers/acpi/battery.c | 19 +++++++++++++++++++
  1 file changed, 19 insertions(+)

diff -puN
drivers/acpi/battery.c~acpi-battery-work-around-negative-s16-battery-current-on-acer

drivers/acpi/battery.c
---
a/drivers/acpi/battery.c~acpi-battery-work-around-negative-s16-battery-current-on-acer
+++ a/drivers/acpi/battery.c
@@ -85,6 +85,10 @@ static const struct acpi_device_id batte

  MODULE_DEVICE_TABLE(acpi, battery_device_ids);

+/* For buggy DSDTs that report negative 16-bit values for either charging
+ * or discharging current and/or report 0 as 65536 due to bad math.
+ */
+#define QUIRK_SIGNED16_CURRENT 0x0001

  struct acpi_battery {
   struct mutex lock;
@@ -112,6 +116,7 @@ struct acpi_battery {
   int state;
   int power_unit;
   u8 alarm_present;
+ long quirks;
  };

  #define to_acpi_battery(x) container_of(x, struct acpi_battery, bat);
@@ -390,6 +395,11 @@ static int acpi_battery_get_state(struct
       state_offsets, ARRAY_SIZE(state_offsets));
   battery->update_time = jiffies;
   kfree(buffer.pointer);
+
+ if ((battery->quirks & QUIRK_SIGNED16_CURRENT) &&
+ battery->rate_now != -1)
+ battery->rate_now = abs((s16)battery->rate_now);
+
   return result;
  }

@@ -495,6 +505,14 @@ static void sysfs_remove_battery(struct
  }
  #endif

+static void acpi_battery_quirks(struct acpi_battery *battery)
+{
+ battery->quirks = 0;
+ if (dmi_name_in_vendors("Acer") && battery->power_unit) {
+ battery->quirks |= QUIRK_SIGNED16_CURRENT;
+ }
+}
+
  static int acpi_battery_update(struct acpi_battery *battery)
  {
   int result, old_present = acpi_battery_present(battery);
@@ -513,6 +531,7 @@ static int acpi_battery_update(struct ac
    result = acpi_battery_get_info(battery);
    if (result)
     return result;
+ acpi_battery_quirks(battery);
    acpi_battery_init_alarm(battery);
   }
  #ifdef CONFIG_ACPI_SYSFS_POWER
_
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to <email address hidden>
More majordomo info at http://vger.kernel.org/majordomo-info.html

--

When all other means of communication fail, try words!

--
Pete Graner <email address hidden>
Manager +1.828.358.0068 Office
Ubuntu Kernel Team +1.828.582.9466 Mobile
Canonical Ltd. http://www.canonical.com/