[PATCH v2 0/7] USB: don't recover device if suspend fails in system sleep

[PATCH v2 0/7] USB: don't recover device if suspend fails in system sleep

[Ming Lei]

Hi,

Sorry for my broken email client just now.

This patch adds comments on interface driver suspend callback
to emphasize that the failure return value is ignored by
USB core in system sleep context, so do not try to recover
device for this case, otherwise the recovery things may confuse
resume().

Also fixes the USB serial, HID and several usbnet drivers
which may recover device in suspend failure path of system sleep.

v2:
	- improve comments on suspend callback as suggested by Alan
	- update kerneldoc for usb_suspend_both as suggested by Alan
	- remove previous check of PMSG_IS_AUTO(message) in cdc_mbim/
	qmi_wwan and add comments on suspend failure case, since Bjørn
	doesn't like the check.
	- add comments on smsc95xx/smsc75xx
v1:
        - fix compile failure
        - add comments about handling suspend failure in resume()

 drivers/hid/usbhid/hid-core.c   |   14 +++++---------
 drivers/net/usb/cdc_mbim.c      |    5 +++++
 drivers/net/usb/qmi_wwan.c      |    5 +++++
 drivers/net/usb/smsc75xx.c      |    6 +++++-
 drivers/net/usb/smsc95xx.c      |    6 +++++-
 drivers/usb/core/driver.c       |   11 ++++++++---
 drivers/usb/serial/usb-serial.c |    3 ++-
 include/linux/usb.h             |    7 ++++++-
 8 files changed, 41 insertions(+), 16 deletions(-)


Thanks,
--
Ming Lei

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/7] USB: adds comment on suspend callback

[Ming Lei]

This patch adds comments on interface driver suspend callback
to emphasize that the failure return value is ignored by
USB core in system sleep context, so do not try to recover
device for this case and let resume/reset_resume callback
handle the suspend failure if needed.

Also kerneldoc for usb_suspend_both() is updated with the
fact.

Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/usb/core/driver.c |   11 ++++++++---
 include/linux/usb.h       |    7 ++++++-
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index d938b2b..eb1d00a 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -1196,9 +1196,14 @@ done:
  *
  * This is the central routine for suspending USB devices.  It calls the
  * suspend methods for all the interface drivers in @udev and then calls
- * the suspend method for @udev itself.  If an error occurs at any stage,
- * all the interfaces which were suspended are resumed so that they remain
- * in the same state as the device.
+ * the suspend method for @udev itself.  When the routine is called in
+ * autosuspend, if an error occurs at any stage, all the interfaces
+ * which were suspended are resumed so that they remain in the same
+ * state as the device, but when called from system sleep, all error
+ * from suspend methods of interfaces and the non-root-hub device itself
+ * are simply ignored, so all suspended interfaces are only resumed
+ * to the device's state when @udev is root-hub and its suspend method
+ * returns failure.
  *
  * Autosuspend requests originating from a child device or an interface
  * driver may be made without the protection of @udev's device lock, but
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 4d22d0f..f82ce57 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -978,7 +978,12 @@ struct usbdrv_wrap {
  *	the "usbfs" filesystem.  This lets devices provide ways to
  *	expose information to user space regardless of where they
  *	do (or don't) show up otherwise in the filesystem.
- * @suspend: Called when the device is going to be suspended by the system.
+ * @suspend: Called when the device is going to be suspended by the
+ *	system either from system sleep or runtime suspend context. The
+ *	return value will be ignored in system sleep context, so do NOT
+ *	try to continue using the device if suspend fails in this case.
+ *	Instead, let the resume or reset-resume routine recover from
+ *	the failure.
  * @resume: Called when the device is being resumed by the system.
  * @reset_resume: Called when the suspended device has been reset instead
  *	of being resumed.
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/7] USB: serial: handle suspend failure path correctly

[Ming Lei]

This patch kills traffic even though type->suspend returns
failure inside usb_serial_suspend from system sleep context
because USB core ignores the failiure and lets system sleep
go ahread, so the serial URB traffic need to be killed
in this case.

Cc: Johan Hovold <jhovold@gmail.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/usb/serial/usb-serial.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index a19ed74..9d0b9c8 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -1142,10 +1142,11 @@ int usb_serial_suspend(struct usb_interface *intf, pm_message_t message)
 
 	if (serial->type->suspend) {
 		r = serial->type->suspend(serial, message);
-		if (r < 0) {
+		if (r < 0 && PMSG_IS_AUTO(message)) {
 			serial->suspending = 0;
 			goto err_out;
 		}
+		/* TODO: resume() might need to handle suspend failure */
 	}
 
 	for (i = 0; i < serial->num_ports; ++i) {
-- 
1.7.9.5

[PATCH v2 3/7] USBHID: don't recover device if suspend fails in system sleep

[Ming Lei]

If suspend callback fails in system sleep context, usb core will
ignore the failure and let the system sleep go ahead further, so this
patch doesn't recover device under this situation, otherwise
may cause resume() confused.

Cc: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/hid/usbhid/hid-core.c |   14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 8e0c4bf94..1f9e56b 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1493,7 +1493,7 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct hid_device *hid = usb_get_intfdata(intf);
 	struct usbhid_device *usbhid = hid->driver_data;
-	int status;
+	int status = 0;
 	bool driver_suspended = false;
 
 	if (PMSG_IS_AUTO(message)) {
@@ -1520,19 +1520,15 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message)
 		}
 
 	} else {
-		if (hid->driver && hid->driver->suspend) {
+		/* TODO: resume() might need to handle suspend failure */
+		if (hid->driver && hid->driver->suspend)
 			status = hid->driver->suspend(hid, message);
-			if (status < 0)
-				return status;
-		}
 		driver_suspended = true;
 		spin_lock_irq(&usbhid->lock);
 		set_bit(HID_SUSPENDED, &usbhid->iofl);
 		spin_unlock_irq(&usbhid->lock);
-		if (usbhid_wait_io(hid) < 0) {
+		if (usbhid_wait_io(hid) < 0)
 			status = -EIO;
-			goto failed;
-		}
 	}
 
 	hid_cancel_delayed_stuff(usbhid);
@@ -1544,7 +1540,7 @@ static int hid_suspend(struct usb_interface *intf, pm_message_t message)
 		goto failed;
 	}
 	dev_dbg(&intf->dev, "suspend\n");
-	return 0;
+	return status;
 
  failed:
 	hid_resume_common(hid, driver_suspended);
-- 
1.7.9.5

[PATCH v2 4/7] usbnet: cdc_mbim: comments on suspend failure

[Ming Lei]

If suspend callback fails in system sleep context, usb core will
ignore the failure and let system sleep go ahead further, so
this patch comments on the case and requires that both
usbnet_suspend() and subdriver->suspend() MUST return 0 in
system sleep context.

Cc: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/net/usb/cdc_mbim.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 248d2dc..406a34d 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -332,6 +332,11 @@ static int cdc_mbim_suspend(struct usb_interface *intf, pm_message_t message)
 		goto error;
 	}
 
+	/*
+	 * Both usbnet_suspend() and subdriver->suspend() MUST return 0
+	 * in system sleep context, otherwise, the resume callback has
+	 * to recover device from previous suspend failure.
+	 */
 	ret = usbnet_suspend(intf, message);
 	if (ret < 0)
 		goto error;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 5/7] usbnet: qmi_wwan: comments on suspend failure

[Ming Lei]

If suspend callback fails in system sleep context, usb core will
ignore the failure and let system sleep go ahead further, so
this patch comments on the case and requires that both
usbnet_suspend() and subdriver->suspend() MUST return 0 in
system sleep context.

Cc: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/net/usb/qmi_wwan.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index efb5c7c..8a60bdd 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -288,6 +288,11 @@ static int qmi_wwan_suspend(struct usb_interface *intf, pm_message_t message)
 	struct qmi_wwan_state *info = (void *)&dev->data;
 	int ret;
 
+	/*
+	 * Both usbnet_suspend() and subdriver->suspend() MUST return 0
+	 * in system sleep context, otherwise, the resume callback has
+	 * to recover device from previous suspend failure.
+	 */
 	ret = usbnet_suspend(intf, message);
 	if (ret < 0)
 		goto err;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 6/7] usbnet: smsc95xx: don't recover device if suspend fails in system sleep

[Ming Lei]

If suspend callback fails in system sleep context, usb core will
ignore the failure and let system sleep go ahead further, so
this patch doesn't recover device under this situation.

Also add comments on the case.

Cc: Steve Glendinning <steve.glendinning@shawell.net>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/usb/smsc95xx.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index e6d2dea..3f38ba8 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1660,7 +1660,11 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 	ret = smsc95xx_enter_suspend0(dev);
 
 done:
-	if (ret)
+	/*
+	 * TODO: resume() might need to handle the suspend failure
+	 * in system sleep
+	 */
+	if (ret && PMSG_IS_AUTO(message))
 		usbnet_resume(intf);
 	return ret;
 }
-- 
1.7.9.5

[PATCH v2 7/7] usbnet: smsc75xx: don't recover device if suspend fails in system sleep

[Ming Lei]

If suspend callback fails in system sleep context, usb core will
ignore the failure and let system sleep go ahead further, so
this patch doesn't recover device under this situation.

Also add comments on this case.

Cc: Steve Glendinning <steve.glendinning@shawell.net>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/usb/smsc75xx.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 9abe517..21b607a 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -2011,7 +2011,11 @@ static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
 	ret = smsc75xx_enter_suspend0(dev);
 
 done:
-	if (ret)
+	/*
+	 * TODO: resume() might need to handle the suspend failure
+	 * in system sleep
+	 */
+	if (ret && PMSG_IS_AUTO(message))
 		usbnet_resume(intf);
 	return ret;
 }
-- 
1.7.9.5

Re: [PATCH v2 4/7] usbnet: cdc_mbim: comments on suspend failure

[Bjørn Mork]

Ming Lei <ming.lei@canonical.com> writes:

> If suspend callback fails in system sleep context, usb core will
> ignore the failure and let system sleep go ahead further, so
> this patch comments on the case and requires that both
> usbnet_suspend() and subdriver->suspend() MUST return 0 in
> system sleep context.
>
> Cc: Bjørn Mork <bjorn@mork.no>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  drivers/net/usb/cdc_mbim.c |    5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
> index 248d2dc..406a34d 100644
> --- a/drivers/net/usb/cdc_mbim.c
> +++ b/drivers/net/usb/cdc_mbim.c
> @@ -332,6 +332,11 @@ static int cdc_mbim_suspend(struct usb_interface *intf, pm_message_t message)
>  		goto error;
>  	}
>  
> +	/*
> +	 * Both usbnet_suspend() and subdriver->suspend() MUST return 0
> +	 * in system sleep context, otherwise, the resume callback has
> +	 * to recover device from previous suspend failure.
> +	 */
>  	ret = usbnet_suspend(intf, message);
>  	if (ret < 0)
>  		goto error;

Looks fine.  Thanks

Acked-by: Bjørn Mork <bjorn@mork.no>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 5/7] usbnet: qmi_wwan: comments on suspend failure

[Bjørn Mork]

Ming Lei <ming.lei@canonical.com> writes:

> If suspend callback fails in system sleep context, usb core will
> ignore the failure and let system sleep go ahead further, so
> this patch comments on the case and requires that both
> usbnet_suspend() and subdriver->suspend() MUST return 0 in
> system sleep context.
>
> Cc: Bjørn Mork <bjorn@mork.no>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  drivers/net/usb/qmi_wwan.c |    5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index efb5c7c..8a60bdd 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -288,6 +288,11 @@ static int qmi_wwan_suspend(struct usb_interface *intf, pm_message_t message)
>  	struct qmi_wwan_state *info = (void *)&dev->data;
>  	int ret;
>  
> +	/*
> +	 * Both usbnet_suspend() and subdriver->suspend() MUST return 0
> +	 * in system sleep context, otherwise, the resume callback has
> +	 * to recover device from previous suspend failure.
> +	 */
>  	ret = usbnet_suspend(intf, message);
>  	if (ret < 0)
>  		goto err;

Acked-by: Bjørn Mork <bjorn@mork.no>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/7] USB: adds comment on suspend callback

[Alan Stern]

On Fri, 8 Mar 2013, Ming Lei wrote:

> This patch adds comments on interface driver suspend callback
> to emphasize that the failure return value is ignored by
> USB core in system sleep context, so do not try to recover
> device for this case and let resume/reset_resume callback
> handle the suspend failure if needed.
> 
> Also kerneldoc for usb_suspend_both() is updated with the
> fact.
> 
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  drivers/usb/core/driver.c |   11 ++++++++---
>  include/linux/usb.h       |    7 ++++++-
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> index d938b2b..eb1d00a 100644
> --- a/drivers/usb/core/driver.c
> +++ b/drivers/usb/core/driver.c
> @@ -1196,9 +1196,14 @@ done:
>   *
>   * This is the central routine for suspending USB devices.  It calls the
>   * suspend methods for all the interface drivers in @udev and then calls
> - * the suspend method for @udev itself.  If an error occurs at any stage,
> - * all the interfaces which were suspended are resumed so that they remain
> - * in the same state as the device.
> + * the suspend method for @udev itself.  When the routine is called in
> + * autosuspend, if an error occurs at any stage, all the interfaces
> + * which were suspended are resumed so that they remain in the same
> + * state as the device, but when called from system sleep, all error
> + * from suspend methods of interfaces and the non-root-hub device itself
> + * are simply ignored, so all suspended interfaces are only resumed
> + * to the device's state when @udev is root-hub and its suspend method
> + * returns failure.
>   *
>   * Autosuspend requests originating from a child device or an interface
>   * driver may be made without the protection of @udev's device lock, but
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index 4d22d0f..f82ce57 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -978,7 +978,12 @@ struct usbdrv_wrap {
>   *	the "usbfs" filesystem.  This lets devices provide ways to
>   *	expose information to user space regardless of where they
>   *	do (or don't) show up otherwise in the filesystem.
> - * @suspend: Called when the device is going to be suspended by the system.
> + * @suspend: Called when the device is going to be suspended by the
> + *	system either from system sleep or runtime suspend context. The
> + *	return value will be ignored in system sleep context, so do NOT
> + *	try to continue using the device if suspend fails in this case.
> + *	Instead, let the resume or reset-resume routine recover from
> + *	the failure.
>   * @resume: Called when the device is being resumed by the system.
>   * @reset_resume: Called when the suspended device has been reset instead
>   *	of being resumed.

Acked-by: Alan Stern <stern@rowland.harvard.edu>

Re: [PATCH v2 0/7] USB: don't recover device if suspend fails in system sleep

[David Miller]

From: Ming Lei <ming.lei@canonical.com>
Date: Fri,  8 Mar 2013 00:15:17 +0800

> This patch adds comments on interface driver suspend callback
> to emphasize that the failure return value is ignored by
> USB core in system sleep context, so do not try to recover
> device for this case, otherwise the recovery things may confuse
> resume().
> 
> Also fixes the USB serial, HID and several usbnet drivers
> which may recover device in suspend failure path of system sleep.
> 
> v2:
> 	- improve comments on suspend callback as suggested by Alan
> 	- update kerneldoc for usb_suspend_both as suggested by Alan
> 	- remove previous check of PMSG_IS_AUTO(message) in cdc_mbim/
> 	qmi_wwan and add comments on suspend failure case, since Bjørn
> 	doesn't like the check.
> 	- add comments on smsc95xx/smsc75xx
> v1:
>         - fix compile failure
>         - add comments about handling suspend failure in resume()

Feel free to merge this via the USB tree and to add my ACK to the
networking driver bits:

Acked-by: David S. Miller <davem@davemloft.net>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/7] USBHID: don't recover device if suspend fails in system sleep

[Jiri Kosina]

On Fri, 8 Mar 2013, Ming Lei wrote:

> If suspend callback fails in system sleep context, usb core will
> ignore the failure and let the system sleep go ahead further, so this
> patch doesn't recover device under this situation, otherwise
> may cause resume() confused.
> 
> Cc: Jiri Kosina <jkosina@suse.cz>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>

Acked-by: Jiri Kosina <jkosina@suse.cz>

I guess this will go through USB tree as a series ...

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v2 0/7] USB: don't recover device if suspend fails in system sleep

[Greg KH]

On Thu, Mar 07, 2013 at 03:28:07PM -0500, David Miller wrote:
> From: Ming Lei <ming.lei@canonical.com>
> Date: Fri,  8 Mar 2013 00:15:17 +0800
> 
> > This patch adds comments on interface driver suspend callback
> > to emphasize that the failure return value is ignored by
> > USB core in system sleep context, so do not try to recover
> > device for this case, otherwise the recovery things may confuse
> > resume().
> > 
> > Also fixes the USB serial, HID and several usbnet drivers
> > which may recover device in suspend failure path of system sleep.
> > 
> > v2:
> > 	- improve comments on suspend callback as suggested by Alan
> > 	- update kerneldoc for usb_suspend_both as suggested by Alan
> > 	- remove previous check of PMSG_IS_AUTO(message) in cdc_mbim/
> > 	qmi_wwan and add comments on suspend failure case, since Bjørn
> > 	doesn't like the check.
> > 	- add comments on smsc95xx/smsc75xx
> > v1:
> >         - fix compile failure
> >         - add comments about handling suspend failure in resume()
> 
> Feel free to merge this via the USB tree and to add my ACK to the
> networking driver bits:
> 
> Acked-by: David S. Miller <davem@davemloft.net>

Thanks, I will do that.

greg k-h

Re: [PATCH v2 3/7] USBHID: don't recover device if suspend fails in system sleep

[Greg Kroah-Hartman]

On Fri, Mar 08, 2013 at 12:16:08AM +0100, Jiri Kosina wrote:
> On Fri, 8 Mar 2013, Ming Lei wrote:
> 
> > If suspend callback fails in system sleep context, usb core will
> > ignore the failure and let the system sleep go ahead further, so this
> > patch doesn't recover device under this situation, otherwise
> > may cause resume() confused.
> > 
> > Cc: Jiri Kosina <jkosina@suse.cz>
> > Signed-off-by: Ming Lei <ming.lei@canonical.com>
> 
> Acked-by: Jiri Kosina <jkosina@suse.cz>
> 
> I guess this will go through USB tree as a series ...

Yes, I can take this, thanks for the ack.

greg k-h

Re: [PATCH v2 2/7] USB: serial: handle suspend failure path correctly

[Johan Hovold]

On Fri, Mar 08, 2013 at 12:15:19AM +0800, Ming Lei wrote:
> This patch kills traffic even though type->suspend returns
> failure inside usb_serial_suspend from system sleep context
> because USB core ignores the failiure and lets system sleep
> go ahread, so the serial URB traffic need to be killed
> in this case.
> 
> Cc: Johan Hovold <jhovold@gmail.com>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  drivers/usb/serial/usb-serial.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
> index a19ed74..9d0b9c8 100644
> --- a/drivers/usb/serial/usb-serial.c
> +++ b/drivers/usb/serial/usb-serial.c
> @@ -1142,10 +1142,11 @@ int usb_serial_suspend(struct usb_interface *intf, pm_message_t message)
>  
>  	if (serial->type->suspend) {
>  		r = serial->type->suspend(serial, message);
> -		if (r < 0) {
> +		if (r < 0 && PMSG_IS_AUTO(message)) {
>  			serial->suspending = 0;
>  			goto err_out;
>  		}
> +		/* TODO: resume() might need to handle suspend failure */
>  	}
>  
>  	for (i = 0; i < serial->num_ports; ++i) {

Sorry for the late reply.

The usb-serial subdriver suspend callbacks do not and must not return
non-zero if !PMSG_IS_AUTO(message) so adding code to handle that case
merely obfuscates this fact.

I'd rather see this documented with a comment just as Bjørn suggested
for cdc_mbim and qmi_wwan.

Thanks,
Johan

Re: [PATCH v2 2/7] USB: serial: handle suspend failure path correctly

[Ming Lei]

On Thu, Mar 14, 2013 at 7:10 PM, Johan Hovold <jhovold@gmail.com> wrote:
> On Fri, Mar 08, 2013 at 12:15:19AM +0800, Ming Lei wrote:
>> This patch kills traffic even though type->suspend returns
>> failure inside usb_serial_suspend from system sleep context
>> because USB core ignores the failiure and lets system sleep
>> go ahread, so the serial URB traffic need to be killed
>> in this case.
>>
>> Cc: Johan Hovold <jhovold@gmail.com>
>> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>> ---
>>  drivers/usb/serial/usb-serial.c |    3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
>> index a19ed74..9d0b9c8 100644
>> --- a/drivers/usb/serial/usb-serial.c
>> +++ b/drivers/usb/serial/usb-serial.c
>> @@ -1142,10 +1142,11 @@ int usb_serial_suspend(struct usb_interface *intf, pm_message_t message)
>>
>>       if (serial->type->suspend) {
>>               r = serial->type->suspend(serial, message);
>> -             if (r < 0) {
>> +             if (r < 0 && PMSG_IS_AUTO(message)) {
>>                       serial->suspending = 0;
>>                       goto err_out;
>>               }
>> +             /* TODO: resume() might need to handle suspend failure */
>>       }
>>
>>       for (i = 0; i < serial->num_ports; ++i) {
>
> Sorry for the late reply.
>
> The usb-serial subdriver suspend callbacks do not and must not return
> non-zero if !PMSG_IS_AUTO(message) so adding code to handle that case
> merely obfuscates this fact.
>
> I'd rather see this documented with a comment just as Bjørn suggested
> for cdc_mbim and qmi_wwan.

No problem, will do it in v3.

Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html