Bug 25232 - battery sysfs I/F returns cached value - Belinea o.book 1301
Summary: battery sysfs I/F returns cached value - Belinea o.book 1301
Status: CLOSED WILL_NOT_FIX
Alias: None
Product: ACPI
Classification: Unclassified
Component: Power-Battery (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Lan Tianyu
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-19 12:31 UTC by Andreas Fackler
Modified: 2013-08-05 08:39 UTC (History)
4 users (show)

See Also:
Kernel Version: 2.6.32 and later
Subsystem:
Regression: No
Bisected commit-id:


Attachments
"acpi -V" before "rmmod battery && modprobe battery" (409 bytes, text/plain)
2010-12-19 12:32 UTC, Andreas Fackler
Details
"acpi -V" after "rmmod battery && modprobe battery" (406 bytes, text/plain)
2010-12-19 12:33 UTC, Andreas Fackler
Details
"acpidump" before "rmmod battery && modprobe battery" (178.33 KB, text/plain)
2010-12-19 12:34 UTC, Andreas Fackler
Details
"acpidump" after "rmmod battery && modprobe battery" (178.33 KB, text/plain)
2010-12-19 12:36 UTC, Andreas Fackler
Details
"dmidecode" before "rmmod battery && modprobe battery" (13.72 KB, text/plain)
2010-12-19 12:37 UTC, Andreas Fackler
Details
"dmidecode" after "rmmod battery && modprobe battery" (13.72 KB, text/plain)
2010-12-19 12:37 UTC, Andreas Fackler
Details
dmesg (46.20 KB, text/plain)
2010-12-19 12:39 UTC, Andreas Fackler
Details
Output of "grep . /sys/class/power_supply/*/*" with wrong charge. (1.90 KB, text/plain)
2013-04-25 19:28 UTC, Andreas Fackler
Details
Output of "grep . /sys/class/power_supply/*/*" with right charge. (1.90 KB, text/plain)
2013-04-25 19:29 UTC, Andreas Fackler
Details
debug.patch (722 bytes, patch)
2013-05-18 14:50 UTC, Lan Tianyu
Details | Diff
debug.patch (2.09 KB, patch)
2013-06-05 05:54 UTC, Lan Tianyu
Details | Diff
dmesg: booted without, then inserted battery (47.10 KB, text/plain)
2013-06-09 16:54 UTC, Andreas Fackler
Details
dmesg: booted without, then inserted battery (47.17 KB, text/plain)
2013-06-09 17:00 UTC, Andreas Fackler
Details
dmesg: booted with battery, got wrong value (47.03 KB, text/plain)
2013-06-15 11:23 UTC, Andreas Fackler
Details
dmesg: booted with battery, got right value (46.96 KB, text/plain)
2013-06-15 11:25 UTC, Andreas Fackler
Details

Description Andreas Fackler 2010-12-19 12:31:37 UTC
The battery state of my Notebook (Belinea o.book 1301) is not determined correctly. Usually after booting it shows a bit less than a quarter of the actual battery charge, e.g. 23% instead of 99% or 10% instead of 43%. Reloading the battery module with "rmmod battery && modprobe battery" solves the problem. Sometimes (randomly?) it does not occur at all.

This happens at least since Linux 2.6.32 on Debian. I have also tried Linux 2.6.37-rc5 and several versions in between, all with the same result.

The issue may be related to bug 14867 concerning the same hardware: My ac adapter also is not detected.
Comment 1 Andreas Fackler 2010-12-19 12:32:59 UTC
Created attachment 40842 [details]
"acpi -V" before "rmmod battery && modprobe battery"
Comment 2 Andreas Fackler 2010-12-19 12:33:53 UTC
Created attachment 40852 [details]
"acpi -V" after "rmmod battery && modprobe battery"
Comment 3 Andreas Fackler 2010-12-19 12:34:45 UTC
Created attachment 40862 [details]
"acpidump" before "rmmod battery && modprobe battery"
Comment 4 Andreas Fackler 2010-12-19 12:36:19 UTC
Created attachment 40872 [details]
"acpidump" after "rmmod battery && modprobe battery"

Both invocations of acpidump generated the error message "Wrong checksum for generic table!"
Comment 5 Andreas Fackler 2010-12-19 12:37:19 UTC
Created attachment 40882 [details]
"dmidecode" before "rmmod battery && modprobe battery"
Comment 6 Andreas Fackler 2010-12-19 12:37:59 UTC
Created attachment 40892 [details]
"dmidecode" after "rmmod battery && modprobe battery"
Comment 7 Andreas Fackler 2010-12-19 12:39:18 UTC
Created attachment 40902 [details]
dmesg

The last line occurred when I reloaded the battery module.
Comment 8 Zhang Rui 2010-12-27 02:32:53 UTC
so the battery status is always correct after you unloading/loading the battery driver again, right?
what if you unplug/plug the AC adapter, is the battery status updated?
Comment 9 Andreas Fackler 2010-12-27 09:48:53 UTC
Unplugging or plugging the AC adapter correctly updates the AC adapter status (which also is often incorrect after booting) but leaves the battery state untouched. Only unloading and loading the battery module seems to always restore the battery state to its true value.
Comment 10 Zhang Rui 2010-12-28 00:35:34 UTC
(In reply to comment #9)
> Unplugging or plugging the AC adapter correctly updates the AC adapter status
> (which also is often incorrect after booting)

please try the patch attached below and see if the AC adapter status is shown correctly.
Comment 11 Zhang Rui 2010-12-28 00:36:03 UTC
Oh, I mean the patch here:
http://marc.info/?l=linux-acpi&m=129177604615355&w=2
Comment 12 Andreas Fackler 2010-12-29 13:38:15 UTC
Unfortunately I was unable to apply the patch to 2.6.32. But I did a few reboots with Linux 2.6.37-rc7, which already contains those four lines, and could not reproduce the AC adapter bug, so the patch seems to fix it.

The battery state however still is not always determined correctly.
Comment 13 Zhang Rui 2010-12-30 00:46:19 UTC
so you may want to try this patch
http://marc.info/?l=linux-acpi&m=129177607115387&w=2
to see if the battery problem is gone.

Note that this patch has already been reverted as it causes an oops.
If this patch help, I think this is a known problem, which we will fix after rewriting the battery driver.
Comment 14 Andreas Fackler 2010-12-30 18:09:08 UTC
I booted about ten times with the patch and the battery status was always detected correctly, so I suppose it was the same problem.

Thank you very much for your quick responses and for pointing out the patch for me!
Comment 15 Zhang Rui 2010-12-31 00:23:59 UTC
good to know.
let's leave this bug open for a few days because the current fix is dropped from upstream kernel.
I'll close it after rewriting the battery driver, sometime in Q1 2011. :)
Comment 16 Zhang Rui 2012-01-18 02:29:32 UTC
Hi, Tianyu,

any update on this?
Comment 17 Lan Tianyu 2013-04-02 14:14:26 UTC
Hi:
    Does this problem exist in the latest kernel?
Comment 18 Andreas Fackler 2013-04-07 12:05:34 UTC
I just tried out kernel 3.9 on the old laptop and the problem still exists: It sometimes shows only a quarter of the real battery status and going into and returning from suspend quadruples it.
Comment 19 Lan Tianyu 2013-04-09 08:24:02 UTC
Please try following patch. Thanks in advance.

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index c5cd5b5..347ed04 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -146,6 +146,9 @@ struct acpi_battery {

 #define to_acpi_battery(x) container_of(x, struct acpi_battery, bat)

+static int acpi_battery_get_info(struct acpi_battery *battery);
+static int acpi_battery_get_state(struct acpi_battery *battery);
+
 inline int acpi_battery_present(struct acpi_battery *battery)
 {
        return battery->device->status.battery_present;
@@ -200,6 +203,7 @@ static int acpi_battery_get_property(struct power_supply *psy,

        if (acpi_battery_present(battery)) {
                /* run battery update only if it is present */
+               acpi_battery_get_info(battery);
                acpi_battery_get_state(battery);
        } else if (psp != POWER_SUPPLY_PROP_PRESENT)
                return -ENODEV;
Comment 20 Lan Tianyu 2013-04-12 02:27:03 UTC
ping ...
Comment 21 Andreas Fackler 2013-04-14 08:06:15 UTC
Thank you, the patch seems to work!
I applied it and rebooted ten times: I could not reproduce the problem.
Comment 22 Lan Tianyu 2013-04-14 12:32:36 UTC
Great, I will upstream the patch as soon as possible.
Comment 23 Lan Tianyu 2013-04-24 03:00:45 UTC
Hi, can you provide "grep .  /sys/class/power_supply/*/*" twice without my previous patch? First time is at error battery charge. Second time is after uploading and loading battery driver when the battery charge is correct.
Comment 24 Andreas Fackler 2013-04-25 19:28:54 UTC
Created attachment 99991 [details]
Output of "grep .  /sys/class/power_supply/*/*" with wrong charge.
Comment 25 Andreas Fackler 2013-04-25 19:29:56 UTC
Created attachment 100001 [details]
Output of "grep .  /sys/class/power_supply/*/*" with right charge.
Comment 26 Lan Tianyu 2013-05-09 13:08:35 UTC
Sorry for late reply. From the output,
The first log show AML BIF/BIX return error charge_full and charge_full_design
/sys/class/power_supply/BAT0/charge_full:20499000 <==
/sys/class/power_supply/BAT0/charge_full_design:276000 <==
/sys/class/power_supply/BAT0/charge_now:4369000

Please try following patch
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index e710045..087c683 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -35,6 +35,7 @@
 #include <linux/slab.h>
 #include <linux/suspend.h>
 #include <asm/unaligned.h>
+#include <linux/delay.h>

 #ifdef CONFIG_ACPI_PROCFS_POWER
 #include <linux/proc_fs.h>
@@ -429,9 +430,13 @@ static int acpi_battery_get_info(struct acpi_battery *battery)
                        "_BIX" : "_BIF";

        struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+       int i = 3;

        if (!acpi_battery_present(battery))
                return 0;
+
+       while (i-- > 0) {
+       msleep(3000);
        mutex_lock(&battery->lock);
        status = acpi_evaluate_object(battery->device->handle, name,
                                                NULL, &buffer);
@@ -441,6 +446,7 @@ static int acpi_battery_get_info(struct acpi_battery *battery)
                ACPI_EXCEPTION((AE_INFO, status, "Evaluating %s", name));
                return -ENODEV;
        }
+       }
        if (test_bit(ACPI_BATTERY_XINFO_PRESENT, &battery->flags))
                result = extract_package(battery, buffer.pointer,
                                extended_info_offsets,
Comment 27 Andreas Fackler 2013-05-12 08:23:04 UTC
I booted ten times again: That patch seems to work, too.
Comment 28 Lan Tianyu 2013-05-18 14:50:58 UTC
Created attachment 101921 [details]
debug.patch

Sorry for late reply. Please try this patch which just remvoe the sleep.
Comment 29 Andreas Fackler 2013-05-20 19:54:42 UTC
Without the sleep, the bug occurs again.
Comment 30 Lan Tianyu 2013-06-05 05:53:35 UTC
Ok. How about bootup without battery and then insert it? The battery volume will be correct?
I produce a patch which will try to check the design_capacity and last_full_capacity for 30 times and 100ms among them. When design_capacity is great than last_full_capacity, it will jump out of loop and produce a log. Please try this patch and attach the output of dmesg. Thanks.
Comment 31 Lan Tianyu 2013-06-05 05:54:00 UTC
Created attachment 103531 [details]
debug.patch
Comment 32 Andreas Fackler 2013-06-09 16:54:42 UTC
Created attachment 104031 [details]
dmesg: booted without, then inserted battery
Comment 33 Andreas Fackler 2013-06-09 17:00:17 UTC
Created attachment 104041 [details]
dmesg: booted without, then inserted battery

I just noticed that after inserting the battery, the status was in fact wrong. I removed and inserted it again, and now it is correct.
I updated the dmesg output.
Comment 34 Lan Tianyu 2013-06-10 02:02:20 UTC
Ok. Thanks for test. How about patch in the comment #30? Could you test it?
Comment 35 Lan Tianyu 2013-06-10 02:23:13 UTC
Sorry. I see the log. Original, I want to test the patch with battery inserted and check whether battery driver can get correct battery info. 
From the log, the battery state is present when battery is not inserted, right?
This looks strange. I only can say this is a Bios issue.
Anyway, please try the patch again with battery inserted and check the battery info. I think it can work unless we need more delay.
Comment 36 Andreas Fackler 2013-06-15 11:23:58 UTC
Created attachment 104761 [details]
dmesg: booted with battery, got wrong value

Sorry for the long delay! This is the dmesg output when booting with inserted battery and with the patch: After a few attempts, the charge was displayed wrong again.

(BTW, I would completely understand should you decide not to pursue this issue further: After all, I'm just one user with old and possibly broken hardware. But if you decide to continue, I'll happily do more experiments, of course.)
Comment 37 Andreas Fackler 2013-06-15 11:25:01 UTC
Created attachment 104771 [details]
dmesg: booted with battery, got right value

This is the dmesg output of an attempt with patch and inserted battery where the bug didn't occur.
Comment 38 Lan Tianyu 2013-08-05 08:39:15 UTC
So sorry for later response. These logs show the EC part of battery info works abnormally about the 10s of bootup. It's hard to add such delay in the battery driver since this totally depends on the kernel configure. 

I have pushed the patch in the comment 19 which can fix this issue but get some feedback that it breaks battery info cache function.

This bug is apparently caused by EC related hardware bug and not sure whether this happens on the another such machine. So marked this bug as WILL_NOT_FIX.

Note You need to log in before you can comment on or make changes to this bug.