lkml.org 
[lkml]   [2020]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [REGRESSION][BISECTED] 5.5-rc suspend/resume failure caused by patch a4f55d8b8c14 ("usb: hub: Check device descriptor before resusciation")
On Wed, 22 Jan 2020 14:31:29 -0500 (EST)
Alan Stern <stern@rowland.harvard.edu> wrote:

> On Tue, 21 Jan 2020, Paul Zimmerman wrote:
>
> > On Mon, 20 Jan 2020 13:52:15 -0700 Paul Zimmerman <pauldzim@gmail.com> wrote:
> >
> > > On Mon, 20 Jan 2020 10:23:11 -0500 (EST) Alan Stern <stern@rowland.harvard.edu> wrote:
> > >
> > > > On Sun, 19 Jan 2020, Paul Zimmerman wrote:
> > > >
> > > > > I reported this regression last week (see
> > > > > https://lore.kernel.org/linux-usb/20200115153714.03d5b3aa@EliteBook/T/#u)
> > > > > but I got no response to my email. Today I have retested with
> > > > > 5.5-rc7 and verified that the problem still exists. So I am
> > > > > resending with a different subject line to see if anyone responds.
> > > > >
> > > > > The $subject patch causes a regression on my HP EliteBook laptop
> > > > > with a built-in USB bluetooth adapter. About 50% of the time, a
> > > > > suspend/resume cycle will cause the bluetooth adapter to stop
> > > > > working.
> > > > >
> > > > > The dmesg log below shows two suspend/resume cycles. At time
> > > > > 63.928 you can see the bluetooth adapter being successfully
> > > > > resumed, and at time 140.969 you can see it fail. After reverting
> > > > > the patch, the bluetooth adapter resumes 100% of the time.
> > > > >
> > > > > I also included below a lsusb -v of the bluetooth adapter. Is
> > > > > there any other debugging info you'd like me to send?
> > > >
> > > > It looks like your dmesg log was made without enabling debugging
> > > > messages in usbcore. Can you collect another log with debugging
> > > > messages turned on?
> > > >
> > > > echo 'module usbcore =p'
> > > > >/sys/kernel/debug/dynamic_debug/control
> > > >
> > > > Also, it might not hurt to collect and post a usbmon trace for a bad
> > > > suspend-resume cycle.
> > >
> > > Hi Alan,
> > >
> > > Thanks for responding. The new dmesg log and the usbmon trace are
> > > below. The dmesg shows a good suspend/resume followed by a bad one.
> > > The bluetooth device is usb 2-3.2 I believe. The usbmon trace is only
> > > for the failed suspend/resume case.
>
> It might be interesting to have a usbmon trace of a successful resume
> as well, for comparison. However I suspect it would just show that
> the new Get-Device-Descriptor request worked and everything else
> continued on normally.

< snip >

> > So if I'm understanding this correctly, there are two threads that are
> > trying to access the USB bluetooth device at the same time. I have no
> > idea if that is how it's supposed to work.
>
> In fact it isn't, although in principle this shouldn't cause any
> trouble. It looks like your bluetooth device is deficient: It
> sometimes crashes if it receives a Get-Device-Descriptor request while
> it is busy with something else.
>
> Still, since there was no real connection change at the port, there's
> no reason to call hub_port_connect_change() here. Let's see if the
> patch below fixes your problem.
>
> Alan Stern
>
>
>
> Index: usb-devel/drivers/usb/core/hub.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/core/hub.c
> +++ usb-devel/drivers/usb/core/hub.c
> @@ -1216,11 +1216,6 @@ static void hub_activate(struct usb_hub
> #ifdef CONFIG_PM
> udev->reset_resume = 1;
> #endif
> - /* Don't set the change_bits when the device
> - * was powered off.
> - */
> - if (test_bit(port1, hub->power_bits))
> - set_bit(port1, hub->change_bits);
>
> } else {
> /* The power session is gone; tell hub_wq */
>

I can confirm this fixes the issue for me, I did a couple dozen
suspend/resume cycles without seeing a failure.

I see the code you removed was added by Lan Tianyu in commit
ad493e5e5805 ("usb: add usb port auto power off mechanism"). I
wonder if your patch would break that? I don't know what that is
or how to test it.

In any case:
Tested-by: Paul Zimmerman <pauldzim@gmail.com>

\
 
 \ /
  Last update: 2020-01-23 00:55    [W:0.066 / U:0.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site