Bug 94651

Created attachment 170281 [details]
kernel 3.14.33 dmesg

please attach the acpidump output.

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?

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.

(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.

Created attachment 170471 [details]
DSDT

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?

Nothing that immediately springs to mind - Benoit, what's the last kernel output before it freezes?

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

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?

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.

*** Bug 95591 has been marked as a duplicate of this bug. ***

*** Bug 96681 has been marked as a duplicate of this bug. ***

Created attachment 174791 [details]
macbook-sbs-fix.patch

Benoit would it be possible for you to test this patch?

(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.

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.

Oliver,
Can you please also test Chris' patch in comment #14? Thanks.

(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)

(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.

(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!!

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

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).

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?

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.

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.

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.

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.

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