[PATCH] ACPI: full battery charge cannot exceed current one

[PATCH] ACPI: full battery charge cannot exceed current one

[Jan Vcelak]

Some degraded batteries report maximal capacity to be smaller than
current charge of the battery. This can confuse some user space
applications (like upower).

This patch handles the situation by increasing maximal capacity
of the battery if it exceedes current charge.

Signed-off-by: Jan Vcelak <jan.vcelak@nic.cz>
---

upower: fully charged battery reported as 0% charged
https://bugzilla.redhat.com/show_bug.cgi?id=1025980

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

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index fbf1ace..71ec1f6 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -494,6 +494,11 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
 		battery->capacity_now = battery->capacity_now *
 		    10000 / battery->design_voltage;
 	}
+
+	/* Some degraded batteries report lower full charge than current one. */
+	if (battery->capacity_now > battery->full_charge_capacity)
+		battery->full_charge_capacity = battery->capacity_now;
+
 	return result;
 }
 
-- 
1.8.4.2

Re: [PATCH] ACPI: full battery charge cannot exceed current one

[Matthew Garrett]

On Sat, Dec 07, 2013 at 01:36:41PM +0100, Jan Vcelak wrote:
> Some degraded batteries report maximal capacity to be smaller than
> current charge of the battery. This can confuse some user space
> applications (like upower).

Can't we just fix upower?

> +
> +	/* Some degraded batteries report lower full charge than current one. */
> +	if (battery->capacity_now > battery->full_charge_capacity)
> +		battery->full_charge_capacity = battery->capacity_now;
> +

Does upower read *_full on every read of the value? How do we guarantee 
that it won't read energy_now without updating energy_full and still 
come up with an incorrect calculation?

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

Re: [PATCH] ACPI: full battery charge cannot exceed current one

[Jan Včelák]

>> Some degraded batteries report maximal capacity to be smaller than
>> current charge of the battery. This can confuse some user space
>> applications (like upower).
> 
> Can't we just fix upower?

It looks like upower already resolved that:

http://cgit.freedesktop.org/upower/tree/src/linux/up-device-supply.c#n652

If the purpose of sysfs is to provide raw values retrieved from battery,
then I agree that this should not be handled in kernel.
Otherwise I would expect semantically correct data there.

>> +	/* Some degraded batteries report lower full charge than current one. */
>> +	if (battery->capacity_now > battery->full_charge_capacity)
>> +		battery->full_charge_capacity = battery->capacity_now;
>> +
> 
> Does upower read *_full on every read of the value? How do we guarantee 
> that it won't read energy_now without updating energy_full and still 
> come up with an incorrect calculation?

That's a good point. I'm not sure about other applications, but upower
always refreshes all values. Function up_device_supply_refresh_battery
seems to be the only place where 'charge_now' is being read:

http://cgit.freedesktop.org/upower/tree/src/linux/up-device-supply.c#n485