[Bug 215799] New: apple-mfi-fastcharge causes automatic pm hibernation entry, when iPhone gets attached

[Bug 215799] New: apple-mfi-fastcharge causes automatic pm hibernation entry, when iPhone gets attached

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215799

            Bug ID: 215799
           Summary: apple-mfi-fastcharge causes automatic pm hibernation
                    entry, when iPhone gets attached
           Product: Drivers
           Version: 2.5
    Kernel Version: 5.17.1
          Hardware: All
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: USB
          Assignee: drivers_usb@kernel-bugs.kernel.org
          Reporter: labre@posteo.de
        Regression: No

Created attachment 300695
  --> https://bugzilla.kernel.org/attachment.cgi?id=300695&action=edit
dmesg log during a boot with an iPhone attached

=======================
Summary
=======================

If I attach an iPhone SE while the apple-mfi-fastcharge module is not
blacklisted, the kernel will immediately hibernate. Loading the module without
an attached iPhone does not have this effect. dmesg is not very helpful in this
case, probably I need a higher debug level. I’ll attach it anyway with some
context.

=======================
Steps to reproduce
=======================
1. Attach an iPhone SE (don’t have any others) via USB-A to Lightning cable
while apple-mfi-fastcharge is either builtin, loaded, or can be loaded
automatically, i.e. is not blacklisted.

=======================
Actual behaviour
=======================
The kernel automatically hibernates without user interaction.

=======================
Expected behaviour
=======================
The kernel continues normal operation.

=======================
Additional information
=======================
The cable has μUSB→Lightning and μUSB→USB C adapters attached. It works fine
with an Android phone and with the iPhone SE without the apple-mfi-fastcharge
module loaded, so it is not faulty.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 215799] apple-mfi-fastcharge causes automatic pm hibernation entry, when iPhone gets attached

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215799

--- Comment #1 from Manuel Ullmann (labre@posteo.de) ---
Created attachment 300696
  --> https://bugzilla.kernel.org/attachment.cgi?id=300696&action=edit
iPhone SE lsusb entry, while apple-mfi-fastcharge is unloaded

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 215799] apple-mfi-fastcharge causes automatic pm hibernation entry, when iPhone gets attached

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215799

Oliver Neukum (oliver@neukum.org) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |oliver@neukum.org

--- Comment #2 from Oliver Neukum (oliver@neukum.org) ---
Working theory:

You are misinterpreting the system's reaction. The hibernation is not done
automatically by the kernel. Instead the kernel reports a charger as a battery.
That battery, being a charger, reports a charge of zero. User space monitors
this and does as it should if its only known power source is a battery at zero
charge: it hibernates the system.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 215799] apple-mfi-fastcharge causes automatic pm hibernation entry, when iPhone gets attached

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215799

--- Comment #3 from Oliver Neukum (oliver@neukum.org) ---
Created attachment 300701
  --> https://bugzilla.kernel.org/attachment.cgi?id=300701&action=edit
correction of type of power source

Please test this patch

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 215799] apple-mfi-fastcharge causes automatic pm hibernation entry, when iPhone gets attached

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215799

Manuel Ullmann (labre@posteo.de) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |CODE_FIX

--- Comment #4 from Manuel Ullmann (labre@posteo.de) ---
(In reply to Oliver Neukum from comment #2)
> Working theory:
> 
> You are misinterpreting the system's reaction. The hibernation is not done
> automatically by the kernel. Instead the kernel reports a charger as a
> battery. That battery, being a charger, reports a charge of zero. User space
> monitors this and does as it should if its only known power source is a
> battery at zero charge: it hibernates the system.

Your theory is correct. The laptop-mode-tools auto-hibernation facility causes
the hibernation. This happens even with the patch, because laptop-mode-tools
fails to check, whether a battery has battery-like nodes or whether there are
just chargers in /sys/class/power_supply. [1]

> Please test this patch

Thank you for looking into this and for the patch. laptop-mode-tools is in
Bash, so I can send the PR myself. It would be nice, if it could be backported,
since it fixes a bug, but laptop-mode-tools could also use a better check for
battery nodes.

[1]:
https://github.com/rickysarraf/laptop-mode-tools/blob/25496b668d597f947a7d65e7c2d4712e8f77692e/usr/sbin/laptop_mode#L486

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 215799] apple-mfi-fastcharge causes automatic pm hibernation entry, when iPhone gets attached

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215799

--- Comment #5 from Oliver Neukum (oliver@neukum.org) ---
(In reply to Manuel Ullmann from comment #4)

> 
> Thank you for looking into this and for the patch. laptop-mode-tools is in
> Bash, so I can send the PR myself. It would be nice, if it could be
> backported, since it fixes a bug, but laptop-mode-tools could also use a
> better check for battery nodes.

Hi,

just to make this clear. We seem to have two bugs. Do both need to be fixed to
remedy this issue, or do we have two independent bugs?

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 215799] apple-mfi-fastcharge causes automatic pm hibernation entry, when iPhone gets attached

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215799

--- Comment #6 from Manuel Ullmann (labre@posteo.de) ---
(In reply to Oliver Neukum from comment #5)
> just to make this clear. We seem to have two bugs. Do both need to be fixed
> to remedy this issue, or do we have two independent bugs?

They are partially dependent on each other, however the kernel can’t fix the
issue alone, while userspace can by adding a safe guard/workaround.

Is it technically more correct to report the charger as USB or should it rather
stay like it is? If the answer is yes, then the patch should be incorporated.
Whether it is required depends on how the laptop-mode-tools developing
community considers workarounds for buggy drivers.

Consider the following commit [1]:

diff --git a/usr/sbin/laptop_mode b/usr/sbin/laptop_mode
index 5e2ef72..c4387ab 100755
--- a/usr/sbin/laptop_mode
+++ b/usr/sbin/laptop_mode
@@ -488,7 +488,7 @@ lmt_load_config ()
        BATTERY_NOT_DISCHARGING=0
        for POWER_SUPPLY in /sys/class/power_supply/* ; do
                if [ -f $POWER_SUPPLY/type ] ; then
-                       SYSFS_POWER_SUPPLY=1
+                       SYSFS_POWER_SUPPLY=$(( SYSFS_POWER_SUPPLY + 1 ))
                        if [ "$(cat $POWER_SUPPLY/type)" = "Mains" ]; then
                                log "VERBOSE" "Determining power state from
$POWER_SUPPLY/online."
                                if [ "$(cat $POWER_SUPPLY/online)" = 1 ]; then
@@ -497,6 +497,14 @@ lmt_load_config ()
                                fi
                         elif [ "$(cat $POWER_SUPPLY/type)" = "Battery" ]; then
                                log "VERBOSE" "Determining power state from
status of battery $POWER_SUPPLY."
+                               #INFO: Because there are and will always be
br0ken drivers
+                               if [ ! -f $POWER_SUPPLY/status ]; then
+                                       log "ERR" "Your power_supply is lacking
a status node"
+                                       log "ERR" "Its driver might be
reporting a wrong type"
+                                       log "ERR" "Ignoring this battery"
+                                       SYSFS_POWER_SUPPLY=$((
SYSFS_POWER_SUPPLY - 1 ))
+                                       continue
+                               fi
                                 #INFO: Because there are and will always be
br0ken batteries
                                if [ "$(cat $POWER_SUPPLY/status)" !=
"Discharging" ]; then
                                        if [ "$(cat $POWER_SUPPLY/status)" =
"Unknown" ]; then
@@ -512,11 +520,15 @@ lmt_load_config ()
                                        ON_AC=0
                                        break
                                fi
+                       else
+                           log "VERBOSE" "Power_supply has unexpected type
$(cat $POWER_SUPPLY/type)"
+                           log "VERBOSE" "Ignoring it"
+                           SYSFS_POWER_SUPPLY=$(( SYSFS_POWER_SUPPLY - 1 ))
                        fi
                fi
        done

-       if [ $SYSFS_POWER_SUPPLY = 1 ] ; then
+       if [ $SYSFS_POWER_SUPPLY -gt 0 ] ; then
                # Already found it!
                log "VERBOSE" "Not trying other options, already found a power
supply."
        elif [ $BATTERY_NOT_DISCHARGING = 1 ]; then
-- 
2.35.1

With the kernel patch, just the else case and increment/decrement would be
required. Without it, the handling in the battery case is required. As I said,
I would consider this a reasonable addition for robustness, but I don’t know,
whether the laptop-mode-tools community shares that opinion.

Thank you again for pointing me in the right direction.

[1]:
https://github.com/rickysarraf/laptop-mode-tools/pull/186/commits/9ddcb2e9f4665fa0b0620fc7eb397917d029fdfd

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 215799] apple-mfi-fastcharge causes automatic pm hibernation entry, when iPhone gets attached

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215799

Bastien Nocera (bugzilla@hadess.net) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla@hadess.net

--- Comment #7 from Bastien Nocera (bugzilla@hadess.net) ---
I replied about the problem on the linux-usb list and in
https://github.com/rickysarraf/laptop-mode-tools/issues/183

This is most certainly a bug in the script, not a problem with the driver.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 215799] apple-mfi-fastcharge causes automatic pm hibernation entry, when iPhone gets attached

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215799

--- Comment #8 from Manuel Ullmann (labre@posteo.de) ---
(In reply to Bastien Nocera from comment #7)
> I replied about the problem on the linux-usb list and in
> https://github.com/rickysarraf/laptop-mode-tools/issues/183
> 
> This is most certainly a bug in the script, not a problem with the driver.

I tend to agree. But the remaining question is still, whether the driver should
report the power_supply as battery, since it is not reporting any battery
stats. It just allows to change the charging type (Fast|Trickle). This might be
unexpected in other areas of userspace, so it might be reasonable to change it.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 215799] apple-mfi-fastcharge causes automatic pm hibernation entry, when iPhone gets attached

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215799

--- Comment #9 from Bastien Nocera (bugzilla@hadess.net) ---
"battery" is the "least incorrect" power supply type we could use. From my
reply to the list:
> POWER_SUPPLY_TYPE_USB also seems to only be used by USB ports that
> are used to charge the machine itself (so a "system" scope), like the
> USB port on a phone, not for devices connected through USB (of which
> there are a lot).

(In reply to Manuel Ullmann from comment #8)
> It just allows to change the charging type (Fast|Trickle).

This is one of the things that power_supply class devices can do. They don't
have to report battery information if it's not available.

If you want to read the battery information of iOS devices, it's available in
upower after at least one user has paired with the device.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 215799] apple-mfi-fastcharge causes automatic pm hibernation entry, when iPhone gets attached

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=215799

Manuel Ullmann (labre@posteo.de) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|CODE_FIX                    |INVALID

--- Comment #10 from Manuel Ullmann (labre@posteo.de) ---
> (In reply to Manuel Ullmann from comment #8)
> > It just allows to change the charging type (Fast|Trickle).
> 
> This is one of the things that power_supply class devices can do. They don't
> have to report battery information if it's not available. Guess I have to try
> this out with my laptop for comparison.

And userspace has to handle that. I get your point. I can implement the scope
handling, if I find any documentation on this.

> If you want to read the battery information of iOS devices, it's available
> in upower after at least one user has paired with the device.
Okay, thanks for the hint.

Changing this to invalid, since userspace has to handle that.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.