Bug 210425

Summary: Plugging in or unplugging power cord while system is suspended does not trigger updates
Product: Drivers Reporter: Nate Graham (nate)
Component: USBAssignee: Default virtual assignee for Drivers/USB (drivers_usb)
Status: RESOLVED UNREPRODUCIBLE    
Severity: normal CC: andrew.co, avillar, bastian, batmansgehilfe, benoitg, bernie, bmarwell, bugzilla, cesarg9, danielmorgan, grzegorz.alibozek, heikki.krogerus, jacob+kernelbugs, jorge.cep.mart, maniette, meven29, morten242, neupsh, pchernik, postix, ramin, rubin, rui.zhang, saipavanchitta1998, speranskiy, tomi, wyattbiker
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 5.9.10 Subsystem:
Regression: Yes Bisected commit-id:
Attachments: UCSI resume
upower result unplugged

Description Nate Graham 2020-11-30 15:22:54 UTC
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0e6371fbfba3a4f76489e6e97c1c7f8386ad5fd2 fixed a bug that was causing the plugged-in/on battery status to not be updated properly when plugging in and unplugging the power cord. 
See downstream bug report https://gitlab.freedesktop.org/upower/upower/-/issues/126 for more details.

However the case where you unplug or plug in the cord while the computer is suspended is still broken. Upon waking up, the new plugged-in status is not signaled in any way, so software that relies on consuming these signals (e.g. upower) will not be notified of the change.

My hardware is a Lenovo ThinkPad X1 Yoga gen 4.
Comment 1 Bastien Nocera 2020-12-01 17:58:02 UTC
Why is it a regression? Has this functionality ever worked on this hardware? I'm guessing it hasn't ever worked, but if it did, the last kernel version where it worked would be useful.
Comment 2 Nate Graham 2020-12-01 19:50:47 UTC
Yes it has worked in the past. I *think* it broke with 5.8.something. Or at least that's when I started noticing https://gitlab.freedesktop.org/upower/upower/-/issues/126, which turned out to be a kernel regression.
Comment 3 Grzegorz Alibożek 2021-01-31 22:44:01 UTC
Still exists in 5.10.12
Comment 4 Maniette 2021-07-12 07:23:36 UTC
I have the issue on a Panasonic CF-MX4.Running XFCE on Debian. When unplugging, no prooblem. When plugging back the power cord, keyboard in not more responsive, nor the mouse. The only possiblr action is to try switching off. Then I have the shutdown pop-up, where I am allowed to click confirmation for shutdown or reboot. That is a serious issue. 

Release        = Debian GNU/Linux 10 (buster)
Kernel         = 4.19.0-17-amd64 #1 SMP Debian 4.19.194-2 (2021-06-21) x86_64
Comment 5 Méven Car 2021-10-24 12:09:59 UTC
It seems to me `ucsi_resume()` should take into account that connectors can be plugged-in/out while the system is suspended.

Upon wake-up it should someway scan its connectors and notify userspace then.

Instead resume only hook the notification after the fact and does not check current state. 

> int ucsi_resume(struct ucsi *ucsi)
> {
>       u64 command;
>
>       /* Restore UCSI notification enable mask after system resume */
>       command = UCSI_SET_NOTIFICATION_ENABLE | ucsi->ntfy;
>
>       return ucsi_send_command(ucsi, command, NULL, 0);
> }

Consequently after suspending, unplugging AC and resuming

> cat /sys/class/power_supply/ucsi-source-psy-USBC000\:001/online
> 1
Regardless of the connector state (re-plugging is not signaled either).
We don't get any `udevadm monitor` notification from the usci subsystem.

AFAICT We should have a call to "power_supply_changed(usci->connector[i]->psy);" somewhere for the usci connector concerned resulting from `ucsi_resume` after the connector state has been updated with `ucsi_handle_connector_change()` for instance. But I don't know about the work_struct API. 
Either this or a bug in `ucsi_handle_connector_change()` not taking into account the unplugged-during-suspend usecase. 

I would welcome any code hint to come up with a proper patch or a patch that I would be happy to test.

My hardware is a DELL XPS 9370.
Comment 6 Méven Car 2021-10-29 18:00:28 UTC
Also the driver ucsi_acpi_platform_driver does not hook to platform_driver  resume() suspend()

This might be the right place to handle resume/suspend.

Found the usci intel documentation:
https://www.intel.com/content/dam/www/public/us/en/documents/technical-specifications/usb-type-c-ucsi-spec.pdf might be of use.
Comment 7 Méven Car 2021-10-29 19:27:39 UTC
I have this naive patch that fix the main issue:

diff --git drivers/usb/typec/ucsi/ucsi.c drivers/usb/typec/ucsi/ucsi.c
index d0c63afaf345..a679359c98be 100644
--- drivers/usb/typec/ucsi/ucsi.c
+++ drivers/usb/typec/ucsi/ucsi.c
@@ -187,11 +187,22 @@ EXPORT_SYMBOL_GPL(ucsi_send_command);
 int ucsi_resume(struct ucsi *ucsi)
 {
        u64 command;
+       int ret;
+       int i;
 
        /* Restore UCSI notification enable mask after system resume */
        command = UCSI_SET_NOTIFICATION_ENABLE | ucsi->ntfy;
 
-       return ucsi_send_command(ucsi, command, NULL, 0);
+       ret = ucsi_send_command(ucsi, command, NULL, 0);
+       if (ret)
+               return ret;
+
+       /* update all connectors */
+       for (i = 0; i < ucsi->cap.num_connectors; i++) {
+               ucsi_connector_change(ucsi, i);
+       }
+
+       return ret;
 }
 EXPORT_SYMBOL_GPL(ucsi_resume);
 /* -------------------------------------------------------------------------- */


Will probably post to LKML
Comment 8 Rubin Abdi 2022-02-05 01:16:34 UTC
Still an issue with 5.15.0. ThinkPad T480s, Debian Sid.

$ uname -a
Linux lines 5.15.0-3-amd64 #1 SMP Debian 5.15.15-1 (2022-01-18) x86_64 GNU/Linux
Comment 9 sai 2022-02-06 05:25:50 UTC
I have been facing this issue since the last two months or so. This problem is very troubling as the low battery indicators no longer showing up resulting in hard poweroffs and other issues. Could anyone also try to comment on the status of this bug report?

My hardware is Lenovo Ideapad Flex 5 - 14ARE05. For a more detailed description of my kernel and hardware this hardware probe can be helpful: https://linux-hardware.org/?probe=981deab8ee

$ uname -a
Linux archlinux 5.15.19-1-lts #1 SMP Tue, 01 Feb 2022 17:37:22 +0000 x86_64 GNU/Linux
Comment 10 Bernie Innocenti 2022-02-15 05:32:19 UTC
Still happening on 5.17.0 on a Thinkpad X1 Gen 7.

Steps to reproduce:

1. Unplug USB-C charger

2. Correctly detected as unplugged:
   % cat /sys/class/power_supply/ucsi-source-psy-USBC000:002/online
   0

3. plug USB-C charger

4. Correctly detected as plugged:
   % cat /sys/class/power_supply/ucsi-source-psy-USBC000:002/online
   1

5. Close the lid and wait for the laptop to sleep

6. Unplug USB-C charger while the laptop is suspended

7. Wake up the laptop

8. BUG: Still detected as plugged!
   % cat /sys/class/power_supply/ucsi-source-psy-USBC000:002/online
   1


Info:

  % uname -a
  Linux giskard 5.17.0-0.rc3.89.fc36.x86_64 #1 SMP PREEMPT Mon Feb 7 14:58:45 UTC 2022 
x86_64 x86_64 x86_64 GNU/Linux
Comment 11 Adrien 2022-04-01 14:10:52 UTC
I have this problem aswell on Thinkpad X1 nano

Linux XXX 5.15.27 #1 SMP PREEMPT Tue Mar 8 18:18:11 CST 2022 x86_64 11th Gen Intel(R) Core(TM) i5-1140G7 @ 1.10GHz GenuineIntel GNU/Linux

I can't find any workaround that work on my system. Is there any command that can be ran to force check if a power supply is present ? For now, I have to reboot the laptop, or plug/unplug the charger once after resuming from sleep.
Comment 12 Grzegorz Alibożek 2022-04-11 20:51:06 UTC
(In reply to Méven Car from comment #7)
> I have this naive patch that fix the main issue:
> 
> diff --git drivers/usb/typec/ucsi/ucsi.c drivers/usb/typec/ucsi/ucsi.c
> index d0c63afaf345..a679359c98be 100644
> --- drivers/usb/typec/ucsi/ucsi.c
> +++ drivers/usb/typec/ucsi/ucsi.c
> @@ -187,11 +187,22 @@ EXPORT_SYMBOL_GPL(ucsi_send_command);
>  int ucsi_resume(struct ucsi *ucsi)
>  {
>         u64 command;
> +       int ret;
> +       int i;
>  
>         /* Restore UCSI notification enable mask after system resume */
>         command = UCSI_SET_NOTIFICATION_ENABLE | ucsi->ntfy;
>  
> -       return ucsi_send_command(ucsi, command, NULL, 0);
> +       ret = ucsi_send_command(ucsi, command, NULL, 0);
> +       if (ret)
> +               return ret;
> +
> +       /* update all connectors */
> +       for (i = 0; i < ucsi->cap.num_connectors; i++) {
> +               ucsi_connector_change(ucsi, i);
> +       }
> +
> +       return ret;
>  }
>  EXPORT_SYMBOL_GPL(ucsi_resume);
>  /*
> -------------------------------------------------------------------------- */
> 
> 
> Will probably post to LKML

any update about your fix?
Comment 13 Adrien 2022-05-16 10:02:19 UTC
same problem on 5.15.38
Comment 14 Bastian Rieck 2022-06-08 04:52:52 UTC
Does not appear with Arch Linux 5.18.1-arch1-1 any more.
Comment 15 Daniel Morgan 2022-06-11 10:16:39 UTC
(In reply to Bastian Rieck from comment #14)
> Does not appear with Arch Linux 5.18.1-arch1-1 any more.

Odd, I still experience the bug with Manjaro kernel 5.18.3-1. Plug in the charger, boot, sleep, unplug, wake from sleep, and the system reports it's still plugged in. I'm sticking with the 5.4 kernel series on my laptop until this gets fixed.

Are there any other Arch users out there (or users of other distributions) who have the problem solved on the 5.18 kernels?
Comment 16 Bastian Rieck 2022-06-11 16:07:23 UTC
(In reply to Daniel Morgan from comment #15)
> (In reply to Bastian Rieck from comment #14)
> > Does not appear with Arch Linux 5.18.1-arch1-1 any more.
> 
> Odd, I still experience the bug with Manjaro kernel 5.18.3-1. Plug in the
> charger, boot, sleep, unplug, wake from sleep, and the system reports it's
> still plugged in. I'm sticking with the 5.4 kernel series on my laptop until
> this gets fixed.
> 
> Are there any other Arch users out there (or users of other distributions)
> who have the problem solved on the 5.18 kernels?

I celebrated too early---the problem just re-appeared; I am now wondering whether this might be indicative of a timing issue, because I could change power status ~1–2 times after waking up.
Comment 17 Jorge Cepeda 2022-06-17 23:56:34 UTC
Can confirm it happens using Feren OS and kernel 5.13.0-51-generic
Comment 18 Pavel Chernikov 2022-06-18 16:41:28 UTC
Confirming still happens on Fedora 36, 5.18.5-200.fc36.x86_64
ThinkPad T13 Gen 3
Comment 19 Méven Car 2022-06-29 19:50:23 UTC
(In reply to grzegorz.alibozek from comment #12)
> (In reply to Méven Car from comment #7)
> > I have this naive patch that fix the main issue:
> > 
> > diff --git drivers/usb/typec/ucsi/ucsi.c drivers/usb/typec/ucsi/ucsi.c
> > index d0c63afaf345..a679359c98be 100644
> > --- drivers/usb/typec/ucsi/ucsi.c
> > +++ drivers/usb/typec/ucsi/ucsi.c
> > @@ -187,11 +187,22 @@ EXPORT_SYMBOL_GPL(ucsi_send_command);
> >  int ucsi_resume(struct ucsi *ucsi)
> >  {
> >         u64 command;
> > +       int ret;
> > +       int i;
> >  
> >         /* Restore UCSI notification enable mask after system resume */
> >         command = UCSI_SET_NOTIFICATION_ENABLE | ucsi->ntfy;
> >  
> > -       return ucsi_send_command(ucsi, command, NULL, 0);
> > +       ret = ucsi_send_command(ucsi, command, NULL, 0);
> > +       if (ret)
> > +               return ret;
> > +
> > +       /* update all connectors */
> > +       for (i = 0; i < ucsi->cap.num_connectors; i++) {
> > +               ucsi_connector_change(ucsi, i);
> > +       }
> > +
> > +       return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(ucsi_resume);
> >  /*
> > --------------------------------------------------------------------------
> */
> > 
> > 
> > Will probably post to LKML
> 
> any update about your fix?

I was wrong, I did not test properly, or at least I am unsure.
I am not familiar with the code base and it would take a long to learn about the API to properly find a proper solution. 

I hope a usci maintainer will have a look.
Comment 20 Adrien 2022-07-27 13:45:27 UTC
I recently switched to slackware current with kernel 5.18.14 and the problem is still present
Comment 21 Grzegorz Alibożek 2022-08-17 21:34:42 UTC
I don't know if it's related:

sie 17 18:06:54 kernel: ucsi_acpi USBC000:00: ucsi_handle_connector_change: GET_CONNECTOR_STATUS failed (-110)
sie 17 18:06:54 kernel: ucsi_acpi USBC000:00: PPM init failed (-110)

on 5.19.2-arch1-1
Comment 22 Grzegorz Alibożek 2022-08-31 20:07:05 UTC
currently the problem occurs even when I plug and unplug the charger on a working laptop.

arch linux, 5.19.5-arch1-1
Comment 23 Heikki Krogerus 2022-09-02 11:10:26 UTC
Created attachment 301729 [details]
UCSI resume

Grzegorz, thanks for letting me know about this issue.

Can someone if this helps?
Comment 24 Bastian Rieck 2022-09-02 11:31:09 UTC
Happy to check---can I just apply this to a current HEAD checkout of the kernel? (It's been a while since I last compiled my own kernel)
Comment 25 Grzegorz Alibożek 2022-09-03 06:24:59 UTC
Heikki, it looks like it is solving the problem after waking up,
Thanks :) 

I have one more case:

1. reboot
2. plug in the power adapter
3. plug out the power adapter

 . after going to sleep and waking up, the state is now set correctly

it is possible?
Comment 26 Grzegorz Alibożek 2022-09-03 06:38:54 UTC
correcting the problem occurs on the dock: Lenovo Thinkpad Ultra Dock 40AJ,
in journalctl i see:
sep 03 08:33:25 kernel: ucsi_acpi USBC000:00: ucsi_handle_connector_change: GET_CONNECTOR_STATUS failed (-110)

but the main problem seems to be resolved
Comment 27 Heikki Krogerus 2022-09-05 08:02:29 UTC
(In reply to Grzegorz Alibożek from comment #26)
> correcting the problem occurs on the dock: Lenovo Thinkpad Ultra Dock 40AJ,
> in journalctl i see:
> sep 03 08:33:25 kernel: ucsi_acpi USBC000:00: ucsi_handle_connector_change:
> GET_CONNECTOR_STATUS failed (-110)
> 
> but the main problem seems to be resolved

Okay. In your case this bug may be just a symptom of that other issue. So my patch may just be a workaround in your case, but how about the others? Does everybody with this issue see that warning when they resume?

There is another report for that issue. I think it's best that we try to solve that first: https://bugzilla.kernel.org/show_bug.cgi?id=216426
Comment 28 Grzegorz Alibożek 2022-09-05 08:22:35 UTC
for the sake of clarity, I had this warning also before your patch.
Comment 29 Grzegorz Alibożek 2022-09-05 14:33:56 UTC
Will you post the change request to the Linux kernel?
Comment 30 Heikki Krogerus 2022-09-06 10:29:12 UTC
I'll clean up the patch and send it out tomorrow.
Comment 31 Grzegorz Alibożek 2022-09-07 11:26:35 UTC
Ok, thanks :)
Comment 32 Heikki Krogerus 2022-09-23 08:51:45 UTC
Quick update. Bastien saw a WARNING that my patch causes. The warning is caused by unbalanced DisplayPort alt mode driver's module reference count. I'm still working on that.
Comment 33 Bastien Nocera 2022-09-27 11:33:34 UTC
(In reply to Heikki Krogerus from comment #32)
> Quick update. Bastien saw a WARNING that my patch causes.

I don't remember seeing anything, I don't even have the hardware to trigger this.
Comment 34 Heikki Krogerus 2022-09-27 13:09:45 UTC
(In reply to Bastien Nocera from comment #33)
> I don't remember seeing anything, I don't even have the hardware to trigger
> this.

Sorry, Bastian not Bastien:
https://lore.kernel.org/linux-usb/27257661.hdJqBvvX10@nimue/#t

The DisplayPort alt mode is activated (in the code) before the DP alt mode driver is bound to it in that case. I don't know exactly why is that happening - I have a theory, but I can reproduce the problem, so I can't test anything. Bear with me.
Comment 35 Bastian Rieck 2022-09-27 13:18:43 UTC
Dear Heikki,

Thanks for your great work. I was unable to reproduce the DP bug so far. Will try some more.

Best,
  Bastian
Comment 36 Grzegorz Alibożek 2022-09-30 17:17:11 UTC
I'm still testing, currently on 5.19.12-arch1 and everything works fine
Comment 37 Grzegorz Alibożek 2022-09-30 17:32:59 UTC
@Heikki, maybe problem which @Bastian had was caused by:

usb: typec: ucsi: add a common function ucsi_unregister_connectors()

and is now resolved, because this commit was reverted in 5.19.8

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v5.19.8&id=3d4044c9e6d2e3f11f1f8b5e0ee8647d3eb1afad
Comment 38 Heikki Krogerus 2022-10-03 07:56:06 UTC
Maybe... Okay, I'll resend the fix later this week.
I'll test it one more time first.
Comment 39 Adrien 2022-10-26 08:04:30 UTC
@heikki Today i have installed 5.19.17 kernel (btw it appears to be the last 5.19.X kernel) I have saw that you made a fix regarding usci but the issue is still present on my machine. So I don't know if this fix was referring to this specific issue, please let me know :)
thanks
Comment 40 Heikki Krogerus 2022-10-26 09:58:50 UTC
(In reply to Adrien from comment #39)
> @heikki Today i have installed 5.19.17 kernel (btw it appears to be the last
> 5.19.X kernel) I have saw that you made a fix regarding usci but the issue
> is still present on my machine. So I don't know if this fix was referring to
> this specific issue, please let me know :)
> thanks

The fix for this bug is not in v5.19.17. It's not even part of mainline yet.

It's now waiting in Greg's tree (there are actually two patches, but you can just keep an eye on this one):
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-linus&id=99f6d43611135bd6f211dec9e88bb41e4167e304

Once it get's applied - presumable withing the next couple of weeks - I'm expecting there to be a bot that reacts by changing the status of this bug. We shall see.
Comment 41 Adrien 2022-10-26 10:13:25 UTC
(In reply to Heikki Krogerus from comment #40)
> (In reply to Adrien from comment #39)
> > @heikki Today i have installed 5.19.17 kernel (btw it appears to be the
> last
> > 5.19.X kernel) I have saw that you made a fix regarding usci but the issue
> > is still present on my machine. So I don't know if this fix was referring
> to
> > this specific issue, please let me know :)
> > thanks
> 
> The fix for this bug is not in v5.19.17. It's not even part of mainline yet.
> 
> It's now waiting in Greg's tree (there are actually two patches, but you can
> just keep an eye on this one):
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/commit/?h=usb-
> linus&id=99f6d43611135bd6f211dec9e88bb41e4167e304
> 
> Once it get's applied - presumable withing the next couple of weeks - I'm
> expecting there to be a bot that reacts by changing the status of this bug.
> We shall see.

Thanks a lot for these information. I will keep an eye on status updates.
Comment 42 Ben M 2022-10-31 19:06:46 UTC
(In reply to Heikki Krogerus from comment #40)

> Once it get's applied - presumable withing the next couple of weeks - I'm
> expecting there to be a bot that reacts by changing the status of this bug.
> We shall see.

I can confirm it is not in 6.0.2-2 on Manjaro.
Using a Lenovo L15, 1st Gen.
Comment 43 Denis 2022-11-17 07:31:18 UTC
I'm not sure if it's okay to report here, but it seems like changes made here breake resume from suspend on my laptop. 
Archlinux kernel 6.0.6- resumes fine
Archlinux kernel 6.0.7- hangs on resume, laptop appears to stuck, nothing works except GUI displayed but not updated and I get errors saying something like
RIP: 0010:ucsi_resume+0x2a/0x70

Lenovo ThinkBook 13s-IML

If it's inappropriate to report such issue here, could you please guide me how to report, I'm totally new to kernel issue reporting.
Comment 44 Denis 2022-11-17 07:56:32 UTC
Just checked with 6.1.0-rc5-1-mainline package wich is a "patchless" kernel from Archlinux AUR and the problem still occurs.
Comment 45 Denis 2022-11-17 08:04:50 UTC
Submitted a bug https://bugzilla.kernel.org/show_bug.cgi?id=216697
Comment 46 Nate Graham 2022-12-06 23:43:43 UTC
This is working for me as of Kernel 6.0.10-300.fc37.x86_64 (64-bit) on Fedora 37.
Comment 47 Adrien 2023-01-16 09:52:41 UTC
Works for me too with kernel 6.1.6 on Slackware (current)
Comment 48 wyattb 2023-03-04 00:10:56 UTC
Exact same issue. Not sure how to reproduce it. Just want to add my versions

Operating System: Kubuntu 22.10
KDE Plasma Version: 5.25.5
KDE Frameworks Version: 5.98.0
Qt Version: 5.15.6
Kernel Version: 5.19.0-35-generic (64-bit)

Graphics Platform: X11
Processors: 16 × 12th Gen Intel® Core™ i7-1260P
Memory: 15.3 GiB of RAM
Graphics Processor: Mesa Intel® Graphics
Manufacturer: LG Electronics
Product Name: 14ZB90Q-G.AAC6U1
System Version: 0.1
Comment 49 wyattb 2023-03-06 01:56:39 UTC
Created attachment 303880 [details]
upower result unplugged

Any chance the reason the icon shows plugged in when it's not, is because power supply always show as YES whether plugged in or not.
Comment 50 Heikki Krogerus 2023-03-06 11:00:46 UTC
(In reply to wyattb from comment #49)
> Any chance the reason the icon shows plugged in when it's not, is because
> power supply always show as YES whether plugged in or not.

I'm not sure that problem is related to this one.

I don't think your kernel has the fix for this one. I don't know to which 5.19.x release the fix was included, but it definitely is not available in v5.19.0.