Bug 12632

Summary:	don't assume we are fully charged when not charging or discharging
Product:	ACPI	Reporter:	Richard Hughes (richard)
Component:	Power-Battery	Assignee:	Zhang Rui (rui.zhang)
Status:	CLOSED CODE_FIX
Severity:	normal	CC:	acpi-bugzilla, rui.zhang
Priority:	P1
Hardware:	All
OS:	Linux
Kernel Version:	2.6.29-rc3	Subsystem:
Regression:	---	Bisected commit-id:
Attachments:	patch to fix the problem

Description Richard Hughes 2009-02-04 05:53:58 UTC

Latest working kernel version: None
Earliest failing kernel version: Everything since ACPI moved to using the power_supply class
Distribution: Fedora rawhide
Hardware Environment: Lenovo T61
Software Environment: Fedora rawhide
Problem Description:

On hardware like the T61 it can take a couple of seconds for the battery
to start charging after the power is connected, and we incorrectly tell
userspace that we are fully charged, and then go back to charging.

Only mark a battery as fully charged when the preset charge matches either
the last full charge, or the design charge.

Steps to reproduce:

Charge battery to 50%. Watch /sys/class/power_supply/BAT*/status, and observe that when the power is disconnected, the status goes from charging->fully_charged->discharging. This also happens when going from battery to AC. This greatly confuses userspace.

Comment 1 Richard Hughes 2009-02-04 05:56:19 UTC

Created attachment 20107 [details]
patch to fix the problem

This fixes things up for me. I've sent this patch twice to linux-acpi but it's so far been ignored: http://www.spinics.net/lists/linux-acpi/msg19946.html. Thanks for considering.

Comment 2 Zhang Rui 2009-02-04 18:00:23 UTC

why?
there are three battery status in all, charging/discharging/full changed, right?
The BIOS should reflect the correct state all the time.
I did your test on another laptop, and the status goes from charging to discharging immediately. So this may be a BIOS problem?

please
1. kill acpid, run "cat /proc/acpi/event", unplug the AC, and attach the output.
2. attach the output of "grep . /sys/firmware/acpi/interrupts/*"
3. attach the full dmesg output after boot
4. attach the acpidump output

Comment 3 Alexey Starikovskiy 2009-02-04 18:14:42 UTC

Rui, this one is already discussed and solved.
There is a short time frame then EC decides what to do with the battery, during this time, battery will not have charging or discharging state, even if it is almost empty. There is a special state in power class for that -- NOT_CHARGING.
So, there are 4 states to the battery.

Comment 4 Richard Hughes 2009-02-05 04:20:08 UTC

Real users are hitting this in Fedora 11 RC1 -- For instance see the debug output in http://bugzilla.gnome.org/show_bug.cgi?id=570133

Do I need to resubmit anything, or is the patch in the queue to send to Linus? Thanks.

Comment 5 Len Brown 2009-02-07 20:48:27 UTC

I was able to reproduce this failure on my FC10 T61.

However, I was also able to reproduce this failure after
the patch in comment #1 was applied.

This situation seems to correct itself in about 1 sec, sometimes much less.
As this is about the amount of time it takes me to get my attention
back to the screen after plugging in the power, I'm not alarmed
about a battery status icon being incorrect for 1 sec.  Are there
more severe implications of this bug that I'm unaware of?

I found that the "mouse hover" scheme for getting more information from the
battery monitor is broken.  If the mouse is not moved, the hover
text is not updated and can quickly become inconsistent with the
battery icon (and reality).

Richard, I discharged the T61 down to about 85%, plugged in AC,
and saw the battery icon switch from DC, to AC w/o an electron bolt
and hovered and was told I'm on AC with the battery is fully charged at 85%,
then a moment later the electron bolt appeared on the icon and
hover tells me that I'm charging at 85%.

Comment 6 Len Brown 2009-02-07 21:05:09 UTC

I should have done this in the first place...

watch -d -n 1 grep . /sys/class/power_supply/BAT0/*

during AC/DC changes shows that the patch works --
status in the DC->AC transient state becomes "Unknown"
instead of "Full".

However, this doesn't seem to solve the problems in the GUI,
which continues to often show full on DC->AC transition,
followed by 'charging' a few seconds later.

From what I can see, the kernel part of this puzzle
is now working as well as it can, giving what the hardware gives us.

Comment 7 Richard Hughes 2009-02-08 02:04:06 UTC

(In reply to comment #5)
> This situation seems to correct itself in about 1 sec, sometimes much less.
> As this is about the amount of time it takes me to get my attention
> back to the screen after plugging in the power, I'm not alarmed
> about a battery status icon being incorrect for 1 sec.  Are there
> more severe implications of this bug that I'm unaware of?

When we emit fully charged from the kernel to userspace it triggers a notification to the user and also stops the battery statistics profiler.

> during AC/DC changes shows that the patch works --
> status in the DC->AC transient state becomes "Unknown"
> instead of "Full".

Correct, this is the desired state as we can't guess from the EC, as the EC doesn't itself know the state.

> However, this doesn't seem to solve the problems in the GUI,
> which continues to often show full on DC->AC transition,
> followed by 'charging' a few seconds later.

Right, I just need to hide the icon when the status is unknown. That's a patch I'm running with locally here. I don't think we should show a "confused" battery icon, just hide it until the EC knows the state.

> From what I can see, the kernel part of this puzzle
> is now working as well as it can, giving what the hardware gives us.

Yes, the patch makes the interaction go from state->unknown->!state rather than state->fully-charged->!state which is much better for userspace.

Thanks for sorting this, cheers.

Comment 8 Len Brown 2009-02-21 09:38:04 UTC

okay, patch in comment #1, which was in acpi-test,
is now staged for upstream.

Comment 9 Len Brown 2009-02-22 17:48:20 UTC

56f382a08722186623400180adbb9d1be1721cee
battery: don't assume we are fully charged when not charging or discharging

shipped in 2.6.29-rc5-git7

closed.