Bug 94651

Summary: Macbook Air 6,1 doesn't detect/report battery
Product: ACPI Reporter: Benoit Gschwind (gschwind)
Component: Power-BatteryAssignee: Matthew Garrett (mjg59-kernel)
Status: CLOSED CODE_FIX    
Severity: normal CC: aaron.lu, chris.bainbridge, gilles.dartiguelongue, lenb, lists2009, oliver, rjw, rui.zhang
Priority: P1    
Hardware: Intel   
OS: Linux   
Kernel Version: 3.18.7 Subsystem:
Regression: Yes Bisected commit-id:
Attachments: kernel 3.18.7 dmesg
kernel 3.14.33 dmesg
DSDT
kernel 3.18.7 message when CONFIG_ACPI_SBS=y and freeze
macbook-sbs-fix.patch
0001-ACPI-SBS-Add-5-us-delay-to-fix-SBS-hangs-on-MacBook.patch
0001-ACPI-SBS-Debug-printk-when-SBS-read-fails.patch

Description Benoit Gschwind 2015-03-10 15:08:38 UTC
Created attachment 170271 [details]
kernel 3.18.7 dmesg

ACPI detect and report battery status properly in kernel 3.14.33 for Macbook Air 6,1 while in 3.18.7 and 4.0-rc3 the battery is not detected at all, no battery is reported.

ac status is reported properly.

Best regards
Comment 1 Benoit Gschwind 2015-03-10 15:09:33 UTC
Created attachment 170281 [details]
kernel 3.14.33 dmesg
Comment 2 Zhang Rui 2015-03-11 02:18:00 UTC
please attach the acpidump output.
Comment 3 Zhang Rui 2015-03-11 03:02:10 UTC
I think it is probably caused by this commit

Author: Matthew Garrett <matthew.garrett@nebula.com>  2014-09-20 19:19:46
Committer: Rafael J. Wysocki <rafael.j.wysocki@intel.com>  2014-09-25 05:31:11

    ACPI / SBS: Disable smart battery manager on Apple
    
    Touching the smart battery manager at all on Apple hardware appears to
    make it unhappy - unplugging the AC adapter triggers accesses that hang
    the controller for several minutes. Quirk it out via DMI in order to
    avoid this.  Compensate by changing battery presence if we fail to
    communicate with the battery.
    
    Signed-off-by: Matthew Garrett <matthew.garrett@nebula.com>
    Signed-off-by: Andreas Noever <andreas.noever@gmail.com>
    Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>


Matthew,
If I understand the patch correctly, it seems that you're disabling SBS battery on all Apple products in this patch, right?
BTW, about commit 3031cddea633ea5328162d3d712a582e4205dbed, the SBS charger is also disabled if ACPI AC charger exists, right?
Comment 4 Matthew Garrett 2015-03-11 05:49:44 UTC
No, that commit disables the smart battery manager (which handles SBS systems with multiple batteries), not SBS in general. Make sure CONFIG_ACPI_SBS is set - the change to report true for _OSI("Darwin") means that Apple hardware now reports SBS batteries, not Control Method ones.
Comment 5 Benoit Gschwind 2015-03-11 12:20:24 UTC
(In reply to Matthew Garrett from comment #4)
> No, that commit disables the smart battery manager (which handles SBS
> systems with multiple batteries), not SBS in general. Make sure
> CONFIG_ACPI_SBS is set - the change to report true for _OSI("Darwin") means
> that Apple hardware now reports SBS batteries, not Control Method ones.


ok I get battery the battery for 3.18.7 kernel when SBS is set as module, the kernel seems to freeze when I say 'yes' to CONFIG_ACPI_SBS

Best regards.
Comment 6 Benoit Gschwind 2015-03-11 12:25:22 UTC
Created attachment 170471 [details]
DSDT
Comment 7 Zhang Rui 2015-03-14 09:13:29 UTC
First of all,
commit 7bc5a2bad0b8d9d1ac9f7b8b33150e4ddf197334, which ships in 3.18-rc1, disables the ACPI control method battery for your system, that's why the battery is gone in 3.18.7.

(In reply to Benoit Gschwind from comment #5)
> ok I get battery the battery for 3.18.7 kernel when SBS is set as module,
> the kernel seems to freeze when I say 'yes' to CONFIG_ACPI_SBS
> 
do you mean the system works well if CONFIG_ACPI_SBS=m, and the system freezes if CONFIG_ACPI_SBS=y, in 3.18.7?
Hmmm, matthew, do you have any ideas about this?
Comment 8 Matthew Garrett 2015-03-14 09:31:07 UTC
Nothing that immediately springs to mind - Benoit, what's the last kernel output before it freezes?
Comment 9 Benoit Gschwind 2015-03-14 10:58:03 UTC
Created attachment 170601 [details]
kernel 3.18.7 message when CONFIG_ACPI_SBS=y and freeze


Yes, when CONFIG_ACPI_SBS=y the kernel freeze/lock, as attached jpeg show.

when CONFIG_ACPI_SBS=m linux boot normaly and battry is properly reported :)

Best regards
Comment 10 Chris Bainbridge 2015-04-17 17:13:31 UTC
Benoit's boot hang and subsequent failure to report battery state looks like the same problem that I reported in bug #96681 - the hang is inside acpi_battery_get_state and is presumably caused by the _OSI("Darwin") change 7bc5a2b causing the SBS to be exposed (as explained in Matthew's blog).

Also it looks like there might have been a typo (?) in Matthew's patch 9faf6136 sbs.c:

-   result = acpi_manager_get_info(sbs);
-   if (!result) {
-       sbs->manager_present = 1;
-       for (id = 0; id < MAX_SBS_BAT; ++id)
-           if ((sbs->batteries_supported & (1 << id)))
-               acpi_battery_add(sbs, id);
-   } else
+   result = 0;
+
+   if (!sbs_manager_broken) {
+       result = acpi_manager_get_info(sbs);
+       if (!result) {
+           sbs->manager_present = 0;
+           for (id = 0; id < MAX_SBS_BAT; ++id)
+               if ((sbs->batteries_supported & (1 << id)))
+                   acpi_battery_add(sbs, id);
+       }
+   }
+
+   if (!sbs->manager_present)
        acpi_battery_add(sbs, 0);
+

Previously sbs->manager_present was set to 1 when the call to acpi_manager_get_info succeeds, which appears to be the correct thing to do?
Comment 11 Chris Bainbridge 2015-04-17 17:18:12 UTC
Another symptom of this bug (besides boot hang) is a missing line from the kernel log:

"ACPI: Smart Battery System [SBS0]: Battery Slot [BAT0] (battery present)"

This line only appears when the SBS is ok.
Comment 12 Aaron Lu 2015-04-21 06:16:14 UTC
*** Bug 95591 has been marked as a duplicate of this bug. ***
Comment 13 Aaron Lu 2015-04-21 06:16:47 UTC
*** Bug 96681 has been marked as a duplicate of this bug. ***
Comment 14 Chris Bainbridge 2015-04-22 17:21:18 UTC
Created attachment 174791 [details]
macbook-sbs-fix.patch

Benoit would it be possible for you to test this patch?
Comment 15 Benoit Gschwind 2015-04-22 20:47:02 UTC
(In reply to Chris Bainbridge from comment #14)
> Created attachment 174791 [details]
> macbook-sbs-fix.patch
> 
> Benoit would it be possible for you to test this patch?

Hello,

I tryed the patch by applying the following procedure:

1. install and configure the bugy kernel 4.0
2. reboot with this kernel to get the bug
3. patch and install the bugy kernel 4.0 without changing the configuration
4. boot on patched kernel

As expected the unpatched kernel hang. After the patch the kernel booted twice properly.

Best regards.
Comment 16 Chris Bainbridge 2015-04-24 01:36:22 UTC
Thanks for testing the patch. Since it works on Benoit's 2009 Macbook and my 2012 I would suggest merging it, hopefully it will work on others. I've sent the patch on to the linux-acpi mailing list.
Comment 17 Aaron Lu 2015-04-24 02:15:17 UTC
Oliver,
Can you please also test Chris' patch in comment #14? Thanks.
Comment 18 Benoit Gschwind 2015-04-24 09:08:01 UTC
(In reply to Chris Bainbridge from comment #16)
> Thanks for testing the patch. Since it works on Benoit's 2009 Macbook and my
> 2012 I would suggest merging it, hopefully it will work on others. I've sent
> the patch on to the linux-acpi mailing list.

mine is MacBookAir6,1 (i.e. MacBook Air 2014)
Comment 19 Chris Bainbridge 2015-04-24 09:28:10 UTC
(In reply to Benoit Gschwind from comment #18)
> 
> mine is MacBookAir6,1 (i.e. MacBook Air 2014)

Ok, that is Haswell. At least we know that the fix works for IvyBridge and Haswell.
Comment 20 Oliver 2015-04-28 06:18:50 UTC
(In reply to Aaron Lu from comment #17)
> Oliver,
> Can you please also test Chris' patch in comment #14? Thanks.

Yes, After patching 4.1-rc1 I rebooted a few times, without issue, running 2 days now (with shutdowns/reboots inbetween) no issue as of yet.

So for me this fixes it. Thanks!!
Comment 21 Brad Campbell 2015-05-15 03:18:47 UTC
I have a Macbook Pro 11,1. While this patch resolves the hang on boot on 4.0.2, on my machine it causes the kernel to be unable to properly poll the battery controller resulting in power issues as the battery state gets stuck and does not update.

It responds and updates when the charger is plugged/unplugged and it updates correctly on resume, but after that
Comment 22 Brad Campbell 2015-05-15 03:20:51 UTC
Previous comment truncated by accident..

I have a Macbook Pro 11,1. While this patch resolves the hang on boot on 4.0.2, on my machine it causes the kernel to be unable to properly poll the battery controller resulting in power issues as the battery state gets stuck and does not update.

It responds and updates when the charger is plugged/unplugged and it updates correctly on resume, but after that the battery state of charge and power consumption remains static and no further updates occur until I plug the charger in or the machine forces power off with a flat battery (and of course no warning).
Comment 23 Chris Bainbridge 2015-05-15 12:30:36 UTC
Created attachment 176991 [details]
0001-ACPI-SBS-Add-5-us-delay-to-fix-SBS-hangs-on-MacBook.patch

Brad, this is the patch that was merged upstream, perhaps you could test it?

It would also be useful if you could confirm that the problem you are seeing has the same cause as the boot hang - if you revert 7bc5a2bad0b8 does everything work ok?
Comment 24 Chris Bainbridge 2015-05-15 12:32:22 UTC
Created attachment 177001 [details]
0001-ACPI-SBS-Debug-printk-when-SBS-read-fails.patch

Do you see any errors in the kernel log? This patch logs a debug message every time an SBS read fails, you may find it useful. You will need to set kernel parameter 'debug' or 'ignore_loglevel' to see debug messages.
Comment 25 Brad Campbell 2015-05-16 02:31:48 UTC
G'day Chris,

I'm currently testing git head (4.1-rc3+) which appears to integrate all your patches. It *looks* like the addition of the 5us delay fixes the problem. If I have issues I'll take it to linux-acpi as it's not directly related to this bug.
Comment 26 Gilles Dartiguelongue 2015-05-21 22:17:55 UTC
Hello, macbook 3.1 here (late 2007). Reverting 7bc5a2bad0b8d9d1ac9f7b8b33150e4ddf197334 fixed battery detection/reading on kernels > 3.17. I'll test attached patches to confirm they work on older hardware too.
Comment 27 Gilles Dartiguelongue 2015-05-21 23:06:09 UTC
With patch b3b2e3fbbd404ed76a916838047e08766efc548f applied, I have battery status again and kernel reports:

[    3.684024] ACPI: Smart Battery System [SBS0]: Battery Slot [BAT0] (battery present)

Thanks.
Comment 28 Len Brown 2015-06-23 14:35:56 UTC
commit 3349fb64b2927407017d970dd5c4daf3c5ad69f8
Author: Chris Bainbridge <chris.bainbridge@gmail.com>
Date:   Wed Apr 29 21:21:40 2015 +0100

    ACPI / SBS: Add 5 us delay to fix SBS hangs on MacBook

is upstream in Linux-4.1