On Wed, 22 Jan 2020, Paul Zimmerman wrote:
> > 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
No, not really. The set_bit() call was added by me in a much earlier
commit (8808f00c7adf, merged in 2008). Lan Tianyu merely added the
"if" test, so that set_bit() doesn't always get called. Now with this
change, set_bit() never gets called.
> wonder if your patch would break that? I don't know what that is
> or how to test it.
While some code review by people who are familiar with this material
wouldn't hurt, I don't think you need to worry about any additional
testing.
> In any case:
> Tested-by: Paul Zimmerman <pauldzim@xxxxxxxxx>
Thank you. I'll submit the patch soon.
Alan Stern
