Alexey Starikovskiy wrote:
> Andrew Morton пишет:
>> On Wed, 01 Jul 2009 20:20:41 +0200
>> acpi_battery has no field `current_now' in 2.6.30 or 2.6.31-rc1. Which
>> kernel version are you patching here?
2.6.29. Guess I got unlucky.
>> Also, I wonder if we need a quirk. Is a "negative" value _ever_
>> correct? If not, could we do the negation unconditionally?
> The problem is that the variable is s64 and not "negative" value is
> related to s16 portion of it.
> It's possible to do this only for "current" mode, in "power" mode values
> of "negative" s16 range may be valid positive values.
Exactly. This is why I further qualified the quirk so it operates for
Acer machines in current (mA) mode only. Currents >32767mA are insane,
but powers >32767mW are not.
The value is nominally a signed 32-bit int as follows:
>=0: charge current
-1: unknown
(The spec defines this in unsigned terms but the result is the same).
The problem here is that my laptop erroneously reports charging currents
as (65536 - current). Interpreting this as a signed 16-bit int yields
the correct signed value, which can then be abs()ed.
The only addition that I'd imagine might make sense would be to check
for current_now != -1 before applying the 16-bit trick, since 32-bit
signed -1 is a valid value that means "unknown current". This does not
conflict with 16-bit -1 because the 16-bits are not sign-extended so
-1mA reports as 65535, not -1. Attached new patch with this change.
For reference, the DSDT does something like this:
s32 current_now;
current_now = (EC_BAT_CURRENT_HIGH<<8) | EC_BAT_CURRENT_LOW;
current_now = (0xFFFF - current_now) + 1;
The "homebrew" two's complement negation attempts to make discharge
currents positive but also makes charge currents negative. It's also
completely wrong for 32-bit quantities, which is what we're dealing with
here.
--
Hector Martin (hector@xxxxxxxxxxxxxx)
Public Key: http://www.marcansoft.com/marcan.asc
Signed-off-by: Hector Martin <hector@xxxxxxxxxxxxxx>
--- linux/drivers/acpi/battery.c.old 2009-07-01 19:17:33.000000000 +0200
+++ linux/drivers/acpi/battery.c 2009-07-01 19:52:43.000000000 +0200
@@ -84,6 +84,10 @@
MODULE_DEVICE_TABLE(acpi, battery_device_ids);
+/* For buggy DSDTs that report negative 16-bit values for either charging
+ * or discharging and/or report 0 as 65536 due to bad math.
+ */
+#define QUIRK_SIGNED16_CURRENT 0x0001
struct acpi_battery {
struct mutex lock;
@@ -111,6 +115,7 @@
int state;
int power_unit;
u8 alarm_present;
+ long quirks;
};
#define to_acpi_battery(x) container_of(x, struct acpi_battery, bat);
@@ -387,6 +392,10 @@
state_offsets, ARRAY_SIZE(state_offsets));
battery->update_time = jiffies;
kfree(buffer.pointer);
+
+ if ((battery->quirks & QUIRK_SIGNED16_CURRENT) && battery->current_now != -1)
+ battery->current_now = abs((s16)battery->current_now);
+
return result;
}
@@ -492,6 +501,14 @@
}
#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);
@@ -510,6 +527,7 @@
result = acpi_battery_get_info(battery);
if (result)
return result;
+ acpi_battery_quirks(battery);
acpi_battery_init_alarm(battery);
}
#ifdef CONFIG_ACPI_SYSFS_POWER
