* [PATCH] Revert "USB / PM: Allow USB devices to remain runtime-suspended when sleeping"
@ 2016-05-02 13:35 Johan Hovold
2016-05-02 15:13 ` Alan Stern
0 siblings, 1 reply; 6+ messages in thread
From: Johan Hovold @ 2016-05-02 13:35 UTC (permalink / raw)
To: Greg Kroah-Hartman, Alan Stern
Cc: linux-usb, linux-kernel, Tomeu Vizoso, Rafael J . Wysocki,
stable, Johan Hovold
This reverts commit e3345db85068ddb937fc0ba40dfc39c293dad977, which
broke system resume for a large class of devices.
Devices that after having been reset during resume need to be rebound
due to a missing reset_resume callback, are now left in a suspended
state. This specifically broke resume of common USB-serial devices,
which are now unusable after system suspend (until disconnected and
reconnected) when USB persist is enabled.
During resume, usb_resume_interface will set the needs_binding flag for
such interfaces, but unlike system resume, run-time resume does not
honour it.
Cc: stable <stable@vger.kernel.org> # 4.5
Signed-off-by: Johan Hovold <johan@kernel.org>
---
Greg, Alan,
This patch for v4.6-rc7 fixes a 4.5-regression that broke system suspend
for a large class of devices, including USB-serial devices, for example
when USB persist is enabled.
We may be able to find a way around this, but since it's a user-visible
regression and late in the rc-cycle, I believe reverting the offending
commit is the right thing to do.
Thanks,
Johan
drivers/usb/core/port.c | 6 ------
drivers/usb/core/usb.c | 8 +-------
2 files changed, 1 insertion(+), 13 deletions(-)
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 14718a9ffcfb..460c855be0d0 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -249,18 +249,12 @@ static int usb_port_runtime_suspend(struct device *dev)
return retval;
}
-
-static int usb_port_prepare(struct device *dev)
-{
- return 1;
-}
#endif
static const struct dev_pm_ops usb_port_pm_ops = {
#ifdef CONFIG_PM
.runtime_suspend = usb_port_runtime_suspend,
.runtime_resume = usb_port_runtime_resume,
- .prepare = usb_port_prepare,
#endif
};
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index dcb85e3cd5a7..479187c32571 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -312,13 +312,7 @@ static int usb_dev_uevent(struct device *dev, struct kobj_uevent_env *env)
static int usb_dev_prepare(struct device *dev)
{
- struct usb_device *udev = to_usb_device(dev);
-
- /* Return 0 if the current wakeup setting is wrong, otherwise 1 */
- if (udev->do_remote_wakeup != device_may_wakeup(dev))
- return 0;
-
- return 1;
+ return 0; /* Implement eventually? */
}
static void usb_dev_complete(struct device *dev)
--
2.7.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Revert "USB / PM: Allow USB devices to remain runtime-suspended when sleeping"
2016-05-02 13:35 [PATCH] Revert "USB / PM: Allow USB devices to remain runtime-suspended when sleeping" Johan Hovold
@ 2016-05-02 15:13 ` Alan Stern
2016-05-02 15:22 ` Greg Kroah-Hartman
2016-05-03 7:51 ` Johan Hovold
0 siblings, 2 replies; 6+ messages in thread
From: Alan Stern @ 2016-05-02 15:13 UTC (permalink / raw)
To: Johan Hovold
Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Tomeu Vizoso,
Rafael J . Wysocki, stable
On Mon, 2 May 2016, Johan Hovold wrote:
> This reverts commit e3345db85068ddb937fc0ba40dfc39c293dad977, which
> broke system resume for a large class of devices.
>
> Devices that after having been reset during resume need to be rebound
> due to a missing reset_resume callback, are now left in a suspended
> state. This specifically broke resume of common USB-serial devices,
> which are now unusable after system suspend (until disconnected and
> reconnected) when USB persist is enabled.
>
> During resume, usb_resume_interface will set the needs_binding flag for
> such interfaces, but unlike system resume, run-time resume does not
> honour it.
>
> Cc: stable <stable@vger.kernel.org> # 4.5
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>
> Greg, Alan,
>
> This patch for v4.6-rc7 fixes a 4.5-regression that broke system suspend
> for a large class of devices, including USB-serial devices, for example
> when USB persist is enabled.
>
> We may be able to find a way around this, but since it's a user-visible
> regression and late in the rc-cycle, I believe reverting the offending
> commit is the right thing to do.
The description of the problem doesn't sound right to me. For
instance, would it help if usb_runtime_resume() did honor the
needs_binding flag? I doubt it. Things like the wakeup setting would
still be lost before the runtime resume occurred.
I suspect the right answer is always to resume a USB device if it needs
a reset-resume, but otherwise allow it to remain in runtime suspend.
Reverting the patch for now is okay with me. Tomeu may want to work on
a better solution. Part of the difficulty is that the PM core wants to
know before suspending whether skipping resume will be okay, but the
USB stack doesn't know until after the host controller has been
resumed.
In the end, we'll probably have the PM core call usb_resume all the
time, but usb_resume will leave the device in runtime suspend if it
can. This isn't ideal but it may be the best we can do.
Alan Stern
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Revert "USB / PM: Allow USB devices to remain runtime-suspended when sleeping"
2016-05-02 15:13 ` Alan Stern
@ 2016-05-02 15:22 ` Greg Kroah-Hartman
2016-05-02 15:26 ` Alan Stern
2016-05-03 7:51 ` Johan Hovold
1 sibling, 1 reply; 6+ messages in thread
From: Greg Kroah-Hartman @ 2016-05-02 15:22 UTC (permalink / raw)
To: Alan Stern
Cc: Johan Hovold, linux-usb, linux-kernel, Tomeu Vizoso,
Rafael J . Wysocki, stable
On Mon, May 02, 2016 at 11:13:10AM -0400, Alan Stern wrote:
> On Mon, 2 May 2016, Johan Hovold wrote:
>
> > This reverts commit e3345db85068ddb937fc0ba40dfc39c293dad977, which
> > broke system resume for a large class of devices.
> >
> > Devices that after having been reset during resume need to be rebound
> > due to a missing reset_resume callback, are now left in a suspended
> > state. This specifically broke resume of common USB-serial devices,
> > which are now unusable after system suspend (until disconnected and
> > reconnected) when USB persist is enabled.
> >
> > During resume, usb_resume_interface will set the needs_binding flag for
> > such interfaces, but unlike system resume, run-time resume does not
> > honour it.
> >
> > Cc: stable <stable@vger.kernel.org> # 4.5
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
> >
> > Greg, Alan,
> >
> > This patch for v4.6-rc7 fixes a 4.5-regression that broke system suspend
> > for a large class of devices, including USB-serial devices, for example
> > when USB persist is enabled.
> >
> > We may be able to find a way around this, but since it's a user-visible
> > regression and late in the rc-cycle, I believe reverting the offending
> > commit is the right thing to do.
>
> The description of the problem doesn't sound right to me. For
> instance, would it help if usb_runtime_resume() did honor the
> needs_binding flag? I doubt it. Things like the wakeup setting would
> still be lost before the runtime resume occurred.
>
> I suspect the right answer is always to resume a USB device if it needs
> a reset-resume, but otherwise allow it to remain in runtime suspend.
>
> Reverting the patch for now is okay with me. Tomeu may want to work on
> a better solution. Part of the difficulty is that the PM core wants to
> know before suspending whether skipping resume will be okay, but the
> USB stack doesn't know until after the host controller has been
> resumed.
>
> In the end, we'll probably have the PM core call usb_resume all the
> time, but usb_resume will leave the device in runtime suspend if it
> can. This isn't ideal but it may be the best we can do.
So is that an "acked-by" for this revert?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Revert "USB / PM: Allow USB devices to remain runtime-suspended when sleeping"
2016-05-02 15:22 ` Greg Kroah-Hartman
@ 2016-05-02 15:26 ` Alan Stern
2016-05-02 15:54 ` Greg Kroah-Hartman
0 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2016-05-02 15:26 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Johan Hovold, linux-usb, linux-kernel, Tomeu Vizoso,
Rafael J . Wysocki, stable
On Mon, 2 May 2016, Greg Kroah-Hartman wrote:
> On Mon, May 02, 2016 at 11:13:10AM -0400, Alan Stern wrote:
> > On Mon, 2 May 2016, Johan Hovold wrote:
> >
> > > This reverts commit e3345db85068ddb937fc0ba40dfc39c293dad977, which
> > > broke system resume for a large class of devices.
> > >
> > > Devices that after having been reset during resume need to be rebound
> > > due to a missing reset_resume callback, are now left in a suspended
> > > state. This specifically broke resume of common USB-serial devices,
> > > which are now unusable after system suspend (until disconnected and
> > > reconnected) when USB persist is enabled.
> > >
> > > During resume, usb_resume_interface will set the needs_binding flag for
> > > such interfaces, but unlike system resume, run-time resume does not
> > > honour it.
> > >
> > > Cc: stable <stable@vger.kernel.org> # 4.5
> > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > ---
> > >
> > > Greg, Alan,
> > >
> > > This patch for v4.6-rc7 fixes a 4.5-regression that broke system suspend
> > > for a large class of devices, including USB-serial devices, for example
> > > when USB persist is enabled.
> > >
> > > We may be able to find a way around this, but since it's a user-visible
> > > regression and late in the rc-cycle, I believe reverting the offending
> > > commit is the right thing to do.
> >
> > The description of the problem doesn't sound right to me. For
> > instance, would it help if usb_runtime_resume() did honor the
> > needs_binding flag? I doubt it. Things like the wakeup setting would
> > still be lost before the runtime resume occurred.
> >
> > I suspect the right answer is always to resume a USB device if it needs
> > a reset-resume, but otherwise allow it to remain in runtime suspend.
> >
> > Reverting the patch for now is okay with me. Tomeu may want to work on
> > a better solution. Part of the difficulty is that the PM core wants to
> > know before suspending whether skipping resume will be okay, but the
> > USB stack doesn't know until after the host controller has been
> > resumed.
> >
> > In the end, we'll probably have the PM core call usb_resume all the
> > time, but usb_resume will leave the device in runtime suspend if it
> > can. This isn't ideal but it may be the best we can do.
>
> So is that an "acked-by" for this revert?
Yes.
Alan Stern
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Revert "USB / PM: Allow USB devices to remain runtime-suspended when sleeping"
2016-05-02 15:26 ` Alan Stern
@ 2016-05-02 15:54 ` Greg Kroah-Hartman
0 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2016-05-02 15:54 UTC (permalink / raw)
To: Alan Stern
Cc: Johan Hovold, linux-usb, linux-kernel, Tomeu Vizoso,
Rafael J . Wysocki, stable
On Mon, May 02, 2016 at 11:26:01AM -0400, Alan Stern wrote:
> On Mon, 2 May 2016, Greg Kroah-Hartman wrote:
>
> > On Mon, May 02, 2016 at 11:13:10AM -0400, Alan Stern wrote:
> > > On Mon, 2 May 2016, Johan Hovold wrote:
> > >
> > > > This reverts commit e3345db85068ddb937fc0ba40dfc39c293dad977, which
> > > > broke system resume for a large class of devices.
> > > >
> > > > Devices that after having been reset during resume need to be rebound
> > > > due to a missing reset_resume callback, are now left in a suspended
> > > > state. This specifically broke resume of common USB-serial devices,
> > > > which are now unusable after system suspend (until disconnected and
> > > > reconnected) when USB persist is enabled.
> > > >
> > > > During resume, usb_resume_interface will set the needs_binding flag for
> > > > such interfaces, but unlike system resume, run-time resume does not
> > > > honour it.
> > > >
> > > > Cc: stable <stable@vger.kernel.org> # 4.5
> > > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > > ---
> > > >
> > > > Greg, Alan,
> > > >
> > > > This patch for v4.6-rc7 fixes a 4.5-regression that broke system suspend
> > > > for a large class of devices, including USB-serial devices, for example
> > > > when USB persist is enabled.
> > > >
> > > > We may be able to find a way around this, but since it's a user-visible
> > > > regression and late in the rc-cycle, I believe reverting the offending
> > > > commit is the right thing to do.
> > >
> > > The description of the problem doesn't sound right to me. For
> > > instance, would it help if usb_runtime_resume() did honor the
> > > needs_binding flag? I doubt it. Things like the wakeup setting would
> > > still be lost before the runtime resume occurred.
> > >
> > > I suspect the right answer is always to resume a USB device if it needs
> > > a reset-resume, but otherwise allow it to remain in runtime suspend.
> > >
> > > Reverting the patch for now is okay with me. Tomeu may want to work on
> > > a better solution. Part of the difficulty is that the PM core wants to
> > > know before suspending whether skipping resume will be okay, but the
> > > USB stack doesn't know until after the host controller has been
> > > resumed.
> > >
> > > In the end, we'll probably have the PM core call usb_resume all the
> > > time, but usb_resume will leave the device in runtime suspend if it
> > > can. This isn't ideal but it may be the best we can do.
> >
> > So is that an "acked-by" for this revert?
>
> Yes.
Great, now applied, thanks.
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Revert "USB / PM: Allow USB devices to remain runtime-suspended when sleeping"
2016-05-02 15:13 ` Alan Stern
2016-05-02 15:22 ` Greg Kroah-Hartman
@ 2016-05-03 7:51 ` Johan Hovold
1 sibling, 0 replies; 6+ messages in thread
From: Johan Hovold @ 2016-05-03 7:51 UTC (permalink / raw)
To: Alan Stern
Cc: Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel,
Tomeu Vizoso, Rafael J . Wysocki, stable
On Mon, May 02, 2016 at 11:13:10AM -0400, Alan Stern wrote:
> On Mon, 2 May 2016, Johan Hovold wrote:
>
> > This reverts commit e3345db85068ddb937fc0ba40dfc39c293dad977, which
> > broke system resume for a large class of devices.
> >
> > Devices that after having been reset during resume need to be rebound
> > due to a missing reset_resume callback, are now left in a suspended
> > state. This specifically broke resume of common USB-serial devices,
> > which are now unusable after system suspend (until disconnected and
> > reconnected) when USB persist is enabled.
> >
> > During resume, usb_resume_interface will set the needs_binding flag for
> > such interfaces, but unlike system resume, run-time resume does not
> > honour it.
> >
> > Cc: stable <stable@vger.kernel.org> # 4.5
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
> >
> > Greg, Alan,
> >
> > This patch for v4.6-rc7 fixes a 4.5-regression that broke system suspend
> > for a large class of devices, including USB-serial devices, for example
> > when USB persist is enabled.
> >
> > We may be able to find a way around this, but since it's a user-visible
> > regression and late in the rc-cycle, I believe reverting the offending
> > commit is the right thing to do.
>
> The description of the problem doesn't sound right to me. For
> instance, would it help if usb_runtime_resume() did honor the
> needs_binding flag? I doubt it. Things like the wakeup setting would
> still be lost before the runtime resume occurred.
>
> I suspect the right answer is always to resume a USB device if it needs
> a reset-resume, but otherwise allow it to remain in runtime suspend.
I was trying to describe how things broke but not necessarily to suggest
how this should be fixed. Could have put the last paragraph below the
cut-off line I guess.
> Reverting the patch for now is okay with me. Tomeu may want to work on
> a better solution. Part of the difficulty is that the PM core wants to
> know before suspending whether skipping resume will be okay, but the
> USB stack doesn't know until after the host controller has been
> resumed.
>
> In the end, we'll probably have the PM core call usb_resume all the
> time, but usb_resume will leave the device in runtime suspend if it
> can. This isn't ideal but it may be the best we can do.
Thanks,
Johan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-05-03 7:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-02 13:35 [PATCH] Revert "USB / PM: Allow USB devices to remain runtime-suspended when sleeping" Johan Hovold
2016-05-02 15:13 ` Alan Stern
2016-05-02 15:22 ` Greg Kroah-Hartman
2016-05-02 15:26 ` Alan Stern
2016-05-02 15:54 ` Greg Kroah-Hartman
2016-05-03 7:51 ` Johan Hovold
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).