A notification event 0x81 from an ACPI battery device requires us to
re-read the battery information structure. Do so, and if the battery's
reporting units have changed (as is the case on some Thinkpads) destroy
and recreate the battery in order to populate the fields correctly.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/acpi/battery.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index dc58402..69638c4 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -562,9 +562,10 @@ static void acpi_battery_quirks(struct acpi_battery *battery)
 	}
 }
 
-static int acpi_battery_update(struct acpi_battery *battery)
+static int acpi_battery_update(struct acpi_battery *battery, bool get_info)
 {
 	int result, old_present = acpi_battery_present(battery);
+	int old_power_unit = battery->power_unit;
 	result = acpi_battery_get_status(battery);
 	if (result)
 		return result;
@@ -587,6 +588,16 @@ static int acpi_battery_update(struct acpi_battery *battery)
 	if (!battery->bat.dev)
 		sysfs_add_battery(battery);
 #endif
+	if (get_info) {
+		acpi_battery_get_info(battery);
+#ifdef CONFIG_ACPI_SYSFS_POWER
+		if (old_power_unit != battery->power_unit) {
+			/* The battery has changed its reporting units */
+			sysfs_remove_battery(battery);
+			sysfs_add_battery(battery);
+		}
+#endif
+	}
 	return acpi_battery_get_state(battery);
 }
 
@@ -762,7 +773,7 @@ static print_func acpi_print_funcs[ACPI_BATTERY_NUMFILES] = {
 static int acpi_battery_read(int fid, struct seq_file *seq)
 {
 	struct acpi_battery *battery = seq->private;
-	int result = acpi_battery_update(battery);
+	int result = acpi_battery_update(battery, false);
 	return acpi_print_funcs[fid](seq, result);
 }
 
@@ -877,7 +888,8 @@ static void acpi_battery_notify(struct acpi_device *device, u32 event)
 #ifdef CONFIG_ACPI_SYSFS_POWER
 	old = battery->bat.dev;
 ...
[ message continues ]

I've had three reports in upower of this bug (the units changing) so
users are really hitting this in real world usage. As soon as this
kernel fix gets released in a new kernel version I'll remove the
upower workaround. I can't reproduce this myself, so I've not tested
the patch, although the logic is solid in my opinion.

Richard.
--

[ message continues ]

Len, any chance you could pick this up?

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

[ message continues ]

A notification event 0x81 from an ACPI battery device requires us to
re-read the battery information structure. Do so, and if the battery's
reporting units have changed (as is the case on some Thinkpads) destroy
and recreate the battery in order to populate the fields correctly.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---

Respun against Linus master

 drivers/acpi/battery.c |   24 +++++++++++++++++-------
 1 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 9fb9d5a..8da9c88 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -130,7 +130,7 @@ struct acpi_battery {
 	unsigned long flags;
 };
 
-static int acpi_battery_update(struct acpi_battery *battery);
+static int acpi_battery_update(struct acpi_battery *battery, bool get_info);
 
 #define to_acpi_battery(x) container_of(x, struct acpi_battery, bat);
 
@@ -186,7 +186,7 @@ static int acpi_battery_get_property(struct power_supply *psy,
 	int ret = 0;
 	struct acpi_battery *battery = to_acpi_battery(psy);
 
-	if (acpi_battery_update(battery))
+	if (acpi_battery_update(battery, false))
 		return -ENODEV;
 
 	if (acpi_battery_present(battery)) {
@@ -610,9 +610,11 @@ static void acpi_battery_quirks2(struct acpi_battery *battery)
 	}
 }
 
-static int acpi_battery_update(struct acpi_battery *battery)
+static int acpi_battery_update(struct acpi_battery *battery, bool get_info)
 {
 	int result, old_present = acpi_battery_present(battery);
+	int old_power_unit = battery->power_unit;
+
 	result = acpi_battery_get_status(battery);
 	if (result)
 		return result;
@@ -631,6 +633,14 @@ static int acpi_battery_update(struct acpi_battery *battery)
 	}
 	if (!battery->bat.dev)
 		sysfs_add_battery(battery);
+	if (get_info) {
+		acpi_battery_get_info(battery);
+		if (old_power_unit != battery->power_unit) {
+			/* The battery has changed its reporting units ...
[ message continues ]

As I've already said on IRC, I'd simply add a new function like:

void acpi_battery_refresh(struct acpi_battery *battery)
{
	if (!battery->bat.dev)
		return;
	acpi_battery_get_info(battery);
	/* The battery may have changed its reporting units */
	sysfs_remove_battery(battery);
	sysfs_add_battery(battery);
}

and call it before acpi_battery_update()
(a) in acpi_battery_resume()
(b) in acpi_battery_notify() if event == ACPI_BATTERY_NOTIFY_INFO

That would be more straightforward IMHO.

Thanks,

--

[ message continues ]

That might be more reasonable. Want to send that, or shall I?

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

[ message continues ]

I can do that.

Thanks,
Rafael
--

[ message continues ]