Re: [bug?] Battery notifications produce flashing battery icon, syslog spam (Re: [PATCH 11/11] ACPI / Battery: Update information on info notification and resume)

[Andy Whitcroft <apw@canonical.com>]

On Tue, May 01, 2012 at 02:14:08PM -0500, Jonathan Nieder wrote:
> (cc-ing Andy)
> Adrian Fita wrote:
> 
> > Also, searching on Google after "upowerd device
> > removed:/org/freedesktop/UPower/devices/battery_BAT0", reveals much
> > more bug reports with the exact issue.
> 
> Thanks.  That confirms the high CPU consumption in upowerd ---
> see [1], for example.
> 
> [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/987807

It does seem somewhat heavyweight to be removing and reinstalling all of
the sysfs files every time we get one of these events.  I am assuming
here that some BIOSs are using this interface to tell us the batery
capacity has changed and triggering the constant add/remove of the
devices and associated flickering.

>From the description of the change this is necessary because the
capacity units may change over time?  Can we not use those to avoid this
update?  I presume it is these two we are referring to?

        int capacity_granularity_1;
        int capacity_granularity_2;

If those are unchanged perhaps we can just skip the update?  Something
like the below (completly untested, for discussion).

Thoughts?

-apw

commit d558d0c38e26e2c7eae68d19f4d2fa3ecd8e31f2
Author: Andy Whitcroft <apw@canonical.com>
Date:   Thu May 3 09:52:28 2012 +0100

    battery: only refresh the sysfs files when pertinant information changes
    
    We only need to regenerate the sysfs files when the capacity units
    change, avoid the update otherwise.
    
    Signed-off-by: Andy Whitcroft <apw@canonical.com>

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index eb18c44..f8d37b4 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -643,10 +643,20 @@ static int acpi_battery_update(struct acpi_battery *battery)
 
 static void acpi_battery_refresh(struct acpi_battery *battery)
 {
+	int cg1, cg2;
+
 	if (!battery->bat.dev)
 		return;
 
+	cg1 = battery->capacity_granularity_1;
+	cg2 = battery->capacity_granularity_2;
+
 	acpi_battery_get_info(battery);
+
+	if (cg1 == battery->capacity_granularity_1 &&
+					cg2 == capacity_granularity_2)
+		return;
+
 	/* The battery may have changed its reporting units. */
 	sysfs_remove_battery(battery);
 	sysfs_add_battery(battery);