[PATCH] USB: Avoid PM error messages during resume if a device was disconnected

[PATCH] USB: Avoid PM error messages during resume if a device was disconnected

[Rafael J. Wysocki]

From: Frans Pop <elendil@planet.nl>

Currently if a laptop is suspended e.g. while docked and then resumed after
undocking it, the following errors get generated because the USB hub in the
docking station and the devices connected to it are no longer available:
pm_op(): usb_dev_resume+0x0/0x10 returns -19
PM: Device 1-2 failed to resume: error -19
pm_op(): usb_dev_resume+0x0/0x10 returns -19
PM: Device 1-2.2 failed to resume: error -19
pm_op(): usb_dev_resume+0x0/0x10 returns -19
PM: Device 1-2.3 failed to resume: error -19

As the removal of USB devices while a system is suspended is a relatively
common use case and in most cases not an error, just return success on
-ENODEV. The user gets informed anyway as the USB subsystem generates
regular disconnect messages for the devices shortly afterwards:
usb 1-2: USB disconnect, address 3
usb 1-2.2: USB disconnect, address 4
usblp0: removed
usb 1-2.3: USB disconnect, address 5

Signed-off-by: Frans Pop <elendil@planet.nl>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
Hi Greg,

I can take this patch to the suspend tree, but if you think it should go
through USB, please consider including it into the 2.6.31 push.

Best,
Rafael
---
 drivers/usb/core/driver.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/usb/core/driver.c
===================================================================
--- linux-2.6.orig/drivers/usb/core/driver.c
+++ linux-2.6/drivers/usb/core/driver.c
@@ -1762,6 +1762,7 @@ int usb_suspend(struct device *dev, pm_m
 int usb_resume(struct device *dev, pm_message_t msg)
 {
 	struct usb_device	*udev;
+	int			status;
 
 	udev = to_usb_device(dev);
 
@@ -1771,7 +1772,14 @@ int usb_resume(struct device *dev, pm_me
 	 */
 	if (udev->skip_sys_resume)
 		return 0;
-	return usb_external_resume_device(udev, msg);
+	status = usb_external_resume_device(udev, msg);
+
+	/* Avoid PM error messages for devices disconnected while suspended
+	 * as we'll display regular disconnect messages just a bit later.
+	 */
+	if (status == -ENODEV)
+		return 0;
+	return status;
 }
 
 #endif /* CONFIG_PM */

Re: [PATCH] USB: Avoid PM error messages during resume if a device was disconnected

[Greg KH]

On Thu, Jun 04, 2009 at 10:30:48PM +0200, Rafael J. Wysocki wrote:
> From: Frans Pop <elendil@planet.nl>
> 
> Currently if a laptop is suspended e.g. while docked and then resumed after
> undocking it, the following errors get generated because the USB hub in the
> docking station and the devices connected to it are no longer available:
> pm_op(): usb_dev_resume+0x0/0x10 returns -19
> PM: Device 1-2 failed to resume: error -19
> pm_op(): usb_dev_resume+0x0/0x10 returns -19
> PM: Device 1-2.2 failed to resume: error -19
> pm_op(): usb_dev_resume+0x0/0x10 returns -19
> PM: Device 1-2.3 failed to resume: error -19
> 
> As the removal of USB devices while a system is suspended is a relatively
> common use case and in most cases not an error, just return success on
> -ENODEV. The user gets informed anyway as the USB subsystem generates
> regular disconnect messages for the devices shortly afterwards:
> usb 1-2: USB disconnect, address 3
> usb 1-2.2: USB disconnect, address 4
> usblp0: removed
> usb 1-2.3: USB disconnect, address 5
> 
> Signed-off-by: Frans Pop <elendil@planet.nl>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
> Hi Greg,
> 
> I can take this patch to the suspend tree, but if you think it should go
> through USB, please consider including it into the 2.6.31 push.

I've now queued it up in my usb tree, as that is probably the best place
for it.

thanks,

greg k-h

[PATCH] usb: avoid PM error messages during resume if a device was disconnected

[Frans Pop]

Currently if a laptop is suspended e.g. while docked and then resumed after
undocking it, the following errors get generated because the USB hub in the
docking station and the devices connected to it are no longer available:
pm_op(): usb_dev_resume+0x0/0x10 returns -19
PM: Device 1-2 failed to resume: error -19
pm_op(): usb_dev_resume+0x0/0x10 returns -19
PM: Device 1-2.2 failed to resume: error -19
pm_op(): usb_dev_resume+0x0/0x10 returns -19
PM: Device 1-2.3 failed to resume: error -19

As the removal of USB devices while a system is suspended is a relatively
common use case and in most cases not an error, just return success on
-ENODEV. The user gets informed anyway as the USB subsystem generates
regular disconnect messages for the devices shortly afterwards:
usb 1-2: USB disconnect, address 3
usb 1-2.2: USB disconnect, address 4
usblp0: removed
usb 1-2.3: USB disconnect, address 5

Signed-off-by: Frans Pop <elendil@planet.nl>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
---

I've walked through the code and AFAICT just returning 0 does not make any
difference for the PM core code.

In a first version of this patch I had the following included just before
the 'return 0':
	pr_info("usb %s: failed to resume: no longer connected\n",
						dev_name(&udev->dev));

Problem with that was that the message got printed twice for each device,
once for .resume and once for .complete. Given that the disconnects are
reported shortly after anyway, I decided the simpler patch might do just
as well.

Cheers,
FJP

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index d0a21a5..97ea69b 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -1727,6 +1727,10 @@ int usb_external_resume_device(struct usb_device *udev, pm_message_t msg)
 	status = usb_resume_both(udev, msg);
 	udev->last_busy = jiffies;
 	usb_pm_unlock(udev);
+
+	if (status == -ENODEV)
+		return 0;
+
 	if (status == 0)
 		do_unbind_rebind(udev, DO_REBIND);
 

Re: [PATCH] usb: avoid PM error messages during resume if a device was disconnected

[Frans Pop]

On Monday 23 March 2009, Frans Pop wrote:
> Currently if a laptop is suspended e.g. while docked and then resumed
> after undocking it, the following errors get generated because the USB
> hub in the docking station and the devices connected to it are no
> longer available:
> pm_op(): usb_dev_resume+0x0/0x10 returns -19 
> PM: Device 1-2 failed to resume: error -19
> pm_op(): usb_dev_resume+0x0/0x10 returns -19
> PM: Device 1-2.2 failed to resume: error -19
> pm_op(): usb_dev_resume+0x0/0x10 returns -19
> PM: Device 1-2.3 failed to resume: error -19
>
> As the removal of USB devices while a system is suspended is a
> relatively common use case and in most cases not an error, just return
> success on -ENODEV. The user gets informed anyway as the USB subsystem
> generates regular disconnect messages for the devices shortly
> afterwards:
> usb 1-2: USB disconnect, address 3 
> usb 1-2.2: USB disconnect, address 4
> usblp0: removed
> usb 1-2.3: USB disconnect, address 5
>
> Signed-off-by: Frans Pop <elendil@planet.nl>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>

Forgot to mention that the patch is intended on top of Rafael's "Rework 
disabling of interrupts during suspend-resume" series.

Re: [PATCH] usb: avoid PM error messages during resume if a device was disconnected

[Alan Stern]

On Mon, 23 Mar 2009, Frans Pop wrote:

> Currently if a laptop is suspended e.g. while docked and then resumed after
> undocking it, the following errors get generated because the USB hub in the
> docking station and the devices connected to it are no longer available:
> pm_op(): usb_dev_resume+0x0/0x10 returns -19
> PM: Device 1-2 failed to resume: error -19
> pm_op(): usb_dev_resume+0x0/0x10 returns -19
> PM: Device 1-2.2 failed to resume: error -19
> pm_op(): usb_dev_resume+0x0/0x10 returns -19
> PM: Device 1-2.3 failed to resume: error -19
> 
> As the removal of USB devices while a system is suspended is a relatively
> common use case and in most cases not an error, just return success on
> -ENODEV. The user gets informed anyway as the USB subsystem generates
> regular disconnect messages for the devices shortly afterwards:
> usb 1-2: USB disconnect, address 3
> usb 1-2.2: USB disconnect, address 4
> usblp0: removed
> usb 1-2.3: USB disconnect, address 5
> 
> Signed-off-by: Frans Pop <elendil@planet.nl>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: Rafael J. Wysocki <rjw@sisk.pl>
> ---
> 
> I've walked through the code and AFAICT just returning 0 does not make any
> difference for the PM core code.

That's right.  After all, there isn't much the PM core can do when an 
error occurs.

> In a first version of this patch I had the following included just before
> the 'return 0':
> 	pr_info("usb %s: failed to resume: no longer connected\n",
> 						dev_name(&udev->dev));
> 
> Problem with that was that the message got printed twice for each device,
> once for .resume and once for .complete. Given that the disconnects are
> reported shortly after anyway, I decided the simpler patch might do just
> as well.
> 
> Cheers,
> FJP
> 
> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> index d0a21a5..97ea69b 100644
> --- a/drivers/usb/core/driver.c
> +++ b/drivers/usb/core/driver.c
> @@ -1727,6 +1727,10 @@ int usb_external_resume_device(struct usb_device *udev, pm_message_t msg)
>  	status = usb_resume_both(udev, msg);
>  	udev->last_busy = jiffies;
>  	usb_pm_unlock(udev);
> +
> +	if (status == -ENODEV)
> +		return 0;
> +
>  	if (status == 0)
>  		do_unbind_rebind(udev, DO_REBIND);

Please include a short comment explaining the reason for this test 
(i.e., that there's no advantage in producing an error message since 
the regular disconnect messages will be generated shortly).

Or do you think maybe it would be better to move this test up into the
PM core?  After all, other subsystems will face the same issue.  I 
think that would be the best approach.  Yes?

Alan STern

Re: [PATCH] usb: avoid PM error messages during resume if a device was disconnected

[Frans Pop]

On Monday 23 March 2009, Alan Stern wrote:
> > diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> > index d0a21a5..97ea69b 100644
> > --- a/drivers/usb/core/driver.c
> > +++ b/drivers/usb/core/driver.c
> > @@ -1727,6 +1727,10 @@ int usb_external_resume_device(struct usb_device *udev, pm_message_t msg)
> >     status = usb_resume_both(udev, msg);
> >  	udev->last_busy = jiffies;
> >  	usb_pm_unlock(udev);
> > +
> > +	if (status == -ENODEV)
> > +		return 0;
> > +
> >  	if (status == 0)
> >  		do_unbind_rebind(udev, DO_REBIND);
>
> Please include a short comment explaining the reason for this test
> (i.e., that there's no advantage in producing an error message since
> the regular disconnect messages will be generated shortly).

Will do if there are no other comments.

> Or do you think maybe it would be better to move this test up into the
> PM core?  After all, other subsystems will face the same issue.  I
> think that would be the best approach.  Yes?

I did look at that option, but implementing it in the USB subsystem seemed
more logical to me, for example as other subsystems possibly would want to
display an info message.

And is -ENODEV safe to ignore in all cases? Would there be other errors that
should be ignored too?

if Rafael would be happy with a generic test for -ENODEV, it could be done.
If not, maybe some other special error code would need to be used but then
you'd still need to test in the subsystem to set that error.
Disadvantage is also that it would make resume_device() and related PM
driver core functions quite a bit less clean than they currently are.

Implementing the test in USB was quite a bit simpler (for me at least ;-)

Thanks,
FJP

Re: [PATCH] usb: avoid PM error messages during resume if a device was disconnected

[Alan Stern]

On Mon, 23 Mar 2009, Frans Pop wrote:

> > Or do you think maybe it would be better to move this test up into the
> > PM core?  After all, other subsystems will face the same issue.  I
> > think that would be the best approach.  Yes?
> 
> I did look at that option, but implementing it in the USB subsystem seemed
> more logical to me, for example as other subsystems possibly would want to
> display an info message.

They still can...

> And is -ENODEV safe to ignore in all cases? Would there be other errors that
> should be ignored too?

In general, the PM core ignores _all_ errors during resume -- in the
sense that it doesn't try to recover from them or do anything to handle
them.  All it does is put a message in the log.

So your question becomes: For which errors should a message be added to 
the system log?  The most logical answer seems to be that we want an 
error message whenever something bad or unexpected occurs.

Removal of a hot-unpluggable device isn't really bad or unexpected.  
Removal of a non-hot-unpluggable device might be bad, but on the other
hand the system isn't really "hot" while it is suspended.  Besides, the
appropriate subsystem can print an error message.  Furthermore the
kernel can't easily tell which devices are hot-unpluggable and which
aren't.

Anything else amounts to failure resuming a device that still exists.  
As such, it probably deserves an error message.

> if Rafael would be happy with a generic test for -ENODEV, it could be done.
> If not, maybe some other special error code would need to be used but then
> you'd still need to test in the subsystem to set that error.
> Disadvantage is also that it would make resume_device() and related PM
> driver core functions quite a bit less clean than they currently are.
> 
> Implementing the test in USB was quite a bit simpler (for me at least ;-)

We should get Rafael's opinion.

Alan Stern

Re: [PATCH] usb: avoid PM error messages during resume if a device was disconnected

[Rafael J. Wysocki]

First, sorry for the delayed response.  Frans has just reminded me about this
thread.

On Tuesday 24 March 2009, Alan Stern wrote:
> On Mon, 23 Mar 2009, Frans Pop wrote:
> 
> > > Or do you think maybe it would be better to move this test up into the
> > > PM core?  After all, other subsystems will face the same issue.  I
> > > think that would be the best approach.  Yes?
> > 
> > I did look at that option, but implementing it in the USB subsystem seemed
> > more logical to me, for example as other subsystems possibly would want to
> > display an info message.
> 
> They still can...
> 
> > And is -ENODEV safe to ignore in all cases? Would there be other errors that
> > should be ignored too?
> 
> In general, the PM core ignores _all_ errors during resume -- in the
> sense that it doesn't try to recover from them or do anything to handle
> them.  All it does is put a message in the log.
> 
> So your question becomes: For which errors should a message be added to 
> the system log?  The most logical answer seems to be that we want an 
> error message whenever something bad or unexpected occurs.
> 
> Removal of a hot-unpluggable device isn't really bad or unexpected.  
> Removal of a non-hot-unpluggable device might be bad, but on the other
> hand the system isn't really "hot" while it is suspended.  Besides, the
> appropriate subsystem can print an error message.  Furthermore the
> kernel can't easily tell which devices are hot-unpluggable and which
> aren't.
> 
> Anything else amounts to failure resuming a device that still exists.  
> As such, it probably deserves an error message.

Returning 0 from usb_external_resume_device() if the device is not present
any more doesn't seem wrong.  It's not really an error condition, IMO, because
it's rather normal that the devices may be removed while suspended.

OTOH, I don't think we can ignore -ENODEV universally in the core, because
its meaning may depend on the bus type.  For example, for PCI it sometimes
means a hardware problem has occured (other than the device being not present
any more).

> > if Rafael would be happy with a generic test for -ENODEV, it could be done.
> > If not, maybe some other special error code would need to be used but then
> > you'd still need to test in the subsystem to set that error.
> > Disadvantage is also that it would make resume_device() and related PM
> > driver core functions quite a bit less clean than they currently are.
> > 
> > Implementing the test in USB was quite a bit simpler (for me at least ;-)
> 
> We should get Rafael's opinion.

I'd vote in favor of the Frans' patch, at least for now.

So, Frans, please resubmit with the changelog modified as requested by Alan.

Best,
Rafael

Re: [PATCH] usb: avoid PM error messages during resume if a device was disconnected

[Alan Stern]

On Tue, 2 Jun 2009, Rafael J. Wysocki wrote:

> Returning 0 from usb_external_resume_device() if the device is not present
> any more doesn't seem wrong.  It's not really an error condition, IMO, because
> it's rather normal that the devices may be removed while suspended.
> 
> OTOH, I don't think we can ignore -ENODEV universally in the core, because
> its meaning may depend on the bus type.  For example, for PCI it sometimes
> means a hardware problem has occured (other than the device being not present
> any more).

> I'd vote in favor of the Frans' patch, at least for now.
> 
> So, Frans, please resubmit with the changelog modified as requested by Alan.

If we change -ENODEV to 0, the change should be made in usb_resume()
rather than usb_external_resume_device().

Alan Stern