[Rebase PATCH] usb: Using correct way to clear usb3.0 device's remote wakeup feature.

2013-01-18 Thread Lan Tianyu
Usb3.0 device defines function remote wakeup which is only for interface
recipient rather than device recipient. This is different with usb2.0 device's
remote wakeup feature which is defined for device recipient. According usb3.0
spec 9.4.5, the function remote wakeup can be modified by the SetFeature()
requests using the FUNCTION_SUSPEND feature selector. This patch is to use
correct way to disable usb3.0 device's function remote wakeup after suspend
error and resuming.

Signed-off-by: Lan Tianyu tianyu@intel.com
---
This patch is rebased on the usb-linus branch commit 1ee0a224bc9aa
USB: io_ti: Fix NULL dereference in chase_port()
---
 drivers/usb/core/hub.c   |   71 +++---
 include/uapi/linux/usb/ch9.h |6 
 2 files changed, 59 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 957ed2c..d1753d6 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2837,6 +2837,24 @@ void usb_enable_ltm(struct usb_device *udev)
 }
 EXPORT_SYMBOL_GPL(usb_enable_ltm);
 
+/*
+ * usb_disable_function_remotewakeup - disable usb3.0
+ * device's function remote wakeup
+ * @udev: target device
+ *
+ * Assume there's only one function on the USB 3.0
+ * device and disable remote wake for the first
+ * interface. FIXME if the interface association
+ * descriptor shows there's more than one function.
+ */
+static int usb_disable_function_remotewakeup(struct usb_device *udev)
+{
+   return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+   USB_REQ_CLEAR_FEATURE, USB_RECIP_INTERFACE,
+   USB_INTRF_FUNC_SUSPEND, 0, NULL, 0,
+   USB_CTRL_SET_TIMEOUT);
+}
+
 #ifdef CONFIG_USB_SUSPEND
 
 /*
@@ -2955,12 +2973,19 @@ int usb_port_suspend(struct usb_device *udev, 
pm_message_t msg)
dev_dbg(hub-intfdev, can't suspend port %d, status %d\n,
port1, status);
/* paranoia:  should not happen */
-   if (udev-do_remote_wakeup)
-   (void) usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
-   USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE,
-   USB_DEVICE_REMOTE_WAKEUP, 0,
-   NULL, 0,
-   USB_CTRL_SET_TIMEOUT);
+   if (udev-do_remote_wakeup) {
+   if (!hub_is_superspeed(hub-hdev)) {
+   (void) usb_control_msg(udev,
+   usb_sndctrlpipe(udev, 0),
+   USB_REQ_CLEAR_FEATURE,
+   USB_RECIP_DEVICE,
+   USB_DEVICE_REMOTE_WAKEUP, 0,
+   NULL, 0,
+   USB_CTRL_SET_TIMEOUT);
+   } else
+   (void) usb_disable_function_remotewakeup(udev);
+
+   }
 
/* Try to enable USB2 hardware LPM again */
if (udev-usb2_hw_lpm_capable == 1)
@@ -3052,20 +3077,30 @@ static int finish_port_resume(struct usb_device *udev)
 * udev-reset_resume
 */
} else if (udev-actconfig  !udev-reset_resume) {
-   le16_to_cpus(devstatus);
-   if (devstatus  (1  USB_DEVICE_REMOTE_WAKEUP)) {
-   status = usb_control_msg(udev,
-   usb_sndctrlpipe(udev, 0),
-   USB_REQ_CLEAR_FEATURE,
+   if (!hub_is_superspeed(udev-parent)) {
+   le16_to_cpus(devstatus);
+   if (devstatus  (1  USB_DEVICE_REMOTE_WAKEUP))
+   status = usb_control_msg(udev,
+   usb_sndctrlpipe(udev, 0),
+   USB_REQ_CLEAR_FEATURE,
USB_RECIP_DEVICE,
-   USB_DEVICE_REMOTE_WAKEUP, 0,
-   NULL, 0,
-   USB_CTRL_SET_TIMEOUT);
-   if (status)
-   dev_dbg(udev-dev,
-   disable remote wakeup, status %d\n,
-   status);
+   USB_DEVICE_REMOTE_WAKEUP, 0,
+   NULL, 0,
+   USB_CTRL_SET_TIMEOUT);
+   } else {
+   status = usb_get_status(udev, USB_RECIP_INTERFACE, 0,
+   devstatus);
+   le16_to_cpus(devstatus);
+   if (!status  devstatus  

Re: [Rebase PATCH] usb: Using correct way to clear usb3.0 device's remote wakeup feature.

2013-01-18 Thread Greg KH
On Sat, Jan 19, 2013 at 12:10:07AM +0800, Lan Tianyu wrote:
 Usb3.0 device defines function remote wakeup which is only for interface
 recipient rather than device recipient. This is different with usb2.0 device's
 remote wakeup feature which is defined for device recipient. According usb3.0
 spec 9.4.5, the function remote wakeup can be modified by the SetFeature()
 requests using the FUNCTION_SUSPEND feature selector. This patch is to use
 correct way to disable usb3.0 device's function remote wakeup after suspend
 error and resuming.
 
 Signed-off-by: Lan Tianyu tianyu@intel.com
 ---
 This patch is rebased on the usb-linus branch commit 1ee0a224bc9aa
 USB: io_ti: Fix NULL dereference in chase_port()

Hm, you should probably go off of usb-next, as usb-linus doesn't have
all of the development effort that is going into 3.9.

But I'll let Sarah take care of this :)

thanks,

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


Re: [PATCH] usb: Using correct way to clear usb3.0 device's remote wakeup feature.

2013-01-17 Thread Sarah Sharp
Hi Tianyu,

This patch looks fine, however, this doesn't apply against Greg's
usb-linus branch.  Can you please fix and resubmit?

Thanks,
Sarah Sharp

On Thu, Jan 17, 2013 at 03:47:51PM +0800, Lan Tianyu wrote:
 Usb3.0 device defines function remote wakeup which is only for interface
 recipient rather than device recipient. This is different with usb2.0 device's
 remote wakeup feature which is defined for device recipient. According usb3.0
 spec 9.4.5, the function remote wakeup can be modified by the SetFeature()
 requests using the FUNCTION_SUSPEND feature selector. This patch is to use
 correct way to disable usb3.0 device's function remote wakeup after suspend
 error and resuming.
 
 Signed-off-by: Lan Tianyu tianyu@intel.com
 ---
  drivers/usb/core/hub.c   |   71 
 +++---
  include/uapi/linux/usb/ch9.h |6 
  2 files changed, 59 insertions(+), 18 deletions(-)
 
 diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
 index 0ef512a..08c9c04 100644
 --- a/drivers/usb/core/hub.c
 +++ b/drivers/usb/core/hub.c
 @@ -2852,6 +2852,24 @@ void usb_enable_ltm(struct usb_device *udev)
  }
  EXPORT_SYMBOL_GPL(usb_enable_ltm);
  
 +/*
 + * usb_disable_function_remotewakeup - disable usb3.0
 + * device's function remote wakeup
 + * @udev: target device
 + *
 + * Assume there's only one function on the USB 3.0
 + * device and disable remote wake for the first
 + * interface. FIXME if the interface association
 + * descriptor shows there's more than one function.
 + */
 +static int usb_disable_function_remotewakeup(struct usb_device *udev)
 +{
 + return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
 + USB_REQ_CLEAR_FEATURE, USB_RECIP_INTERFACE,
 + USB_INTRF_FUNC_SUSPEND, 0, NULL, 0,
 + USB_CTRL_SET_TIMEOUT);
 +}
 +
  #ifdef   CONFIG_USB_SUSPEND
  
  /*
 @@ -2968,12 +2986,19 @@ int usb_port_suspend(struct usb_device *udev, 
 pm_message_t msg)
   dev_dbg(hub-intfdev, can't suspend port %d, status %d\n,
   port1, status);
   /* paranoia:  should not happen */
 - if (udev-do_remote_wakeup)
 - (void) usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
 - USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE,
 - USB_DEVICE_REMOTE_WAKEUP, 0,
 - NULL, 0,
 - USB_CTRL_SET_TIMEOUT);
 + if (udev-do_remote_wakeup) {
 + if (!hub_is_superspeed(hub-hdev)) {
 + (void) usb_control_msg(udev,
 + usb_sndctrlpipe(udev, 0),
 + USB_REQ_CLEAR_FEATURE,
 + USB_RECIP_DEVICE,
 + USB_DEVICE_REMOTE_WAKEUP, 0,
 + NULL, 0,
 + USB_CTRL_SET_TIMEOUT);
 + } else
 + (void) usb_disable_function_remotewakeup(udev);
 +
 + }
  
   /* Try to enable USB2 hardware LPM again */
   if (udev-usb2_hw_lpm_capable == 1)
 @@ -3059,20 +3084,30 @@ static int finish_port_resume(struct usb_device *udev)
   dev_dbg(udev-dev, gone after usb resume? status %d\n,
   status);
   } else if (udev-actconfig) {
 - le16_to_cpus(devstatus);
 - if (devstatus  (1  USB_DEVICE_REMOTE_WAKEUP)) {
 - status = usb_control_msg(udev,
 - usb_sndctrlpipe(udev, 0),
 - USB_REQ_CLEAR_FEATURE,
 + if (!hub_is_superspeed(udev-parent)) {
 + le16_to_cpus(devstatus);
 + if (devstatus  (1  USB_DEVICE_REMOTE_WAKEUP))
 + status = usb_control_msg(udev,
 + usb_sndctrlpipe(udev, 0),
 + USB_REQ_CLEAR_FEATURE,
   USB_RECIP_DEVICE,
 - USB_DEVICE_REMOTE_WAKEUP, 0,
 - NULL, 0,
 - USB_CTRL_SET_TIMEOUT);
 - if (status)
 - dev_dbg(udev-dev,
 - disable remote wakeup, status %d\n,
 - status);
 + USB_DEVICE_REMOTE_WAKEUP, 0,
 + NULL, 0,
 + USB_CTRL_SET_TIMEOUT);
 + } else {
 + status = usb_get_status(udev, USB_RECIP_INTERFACE, 0,
 +

Re: [PATCH] usb: Using correct way to clear usb3.0 device's remote wakeup feature.

2013-01-17 Thread Alan Stern
On Thu, 17 Jan 2013, Sarah Sharp wrote:

 Hi Tianyu,
 
 This patch looks fine, however, this doesn't apply against Greg's
 usb-linus branch.  Can you please fix and resubmit?

Actually, I think it would be better if there was a single function for 
disabling wakeup on all devices.  Then the division between USB-2 and 
USB-3 could be encapsulated entirely within that function.

Alan Stern

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


Re: [PATCH] usb: Using correct way to clear usb3.0 device's remote wakeup feature.

2013-01-17 Thread Sarah Sharp
On Thu, Jan 17, 2013 at 02:05:29PM -0500, Alan Stern wrote:
 On Thu, 17 Jan 2013, Sarah Sharp wrote:
 
  Hi Tianyu,
  
  This patch looks fine, however, this doesn't apply against Greg's
  usb-linus branch.  Can you please fix and resubmit?
 
 Actually, I think it would be better if there was a single function for 
 disabling wakeup on all devices.  Then the division between USB-2 and 
 USB-3 could be encapsulated entirely within that function.

I agree.  However, I feel like that should be a separate patch for
usb-next to keep the stable kernel changes to a minimum.

I think we need two functions: one for enabling remote wakeup, and one
for disabling remote wakeup.  The long arguments to the control message
are (usually) nested two or three indentation levels deep, so having
separate functions for both enable and disable would be good.

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


[PATCH] usb: Using correct way to clear usb3.0 device's remote wakeup feature.

2013-01-16 Thread Lan Tianyu
Usb3.0 device defines function remote wakeup which is only for interface
recipient rather than device recipient. This is different with usb2.0 device's
remote wakeup feature which is defined for device recipient. According usb3.0
spec 9.4.5, the function remote wakeup can be modified by the SetFeature()
requests using the FUNCTION_SUSPEND feature selector. This patch is to use
correct way to disable usb3.0 device's function remote wakeup after suspend
error and resuming.

Signed-off-by: Lan Tianyu tianyu@intel.com
---
 drivers/usb/core/hub.c   |   71 +++---
 include/uapi/linux/usb/ch9.h |6 
 2 files changed, 59 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 0ef512a..08c9c04 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2852,6 +2852,24 @@ void usb_enable_ltm(struct usb_device *udev)
 }
 EXPORT_SYMBOL_GPL(usb_enable_ltm);
 
+/*
+ * usb_disable_function_remotewakeup - disable usb3.0
+ * device's function remote wakeup
+ * @udev: target device
+ *
+ * Assume there's only one function on the USB 3.0
+ * device and disable remote wake for the first
+ * interface. FIXME if the interface association
+ * descriptor shows there's more than one function.
+ */
+static int usb_disable_function_remotewakeup(struct usb_device *udev)
+{
+   return usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+   USB_REQ_CLEAR_FEATURE, USB_RECIP_INTERFACE,
+   USB_INTRF_FUNC_SUSPEND, 0, NULL, 0,
+   USB_CTRL_SET_TIMEOUT);
+}
+
 #ifdef CONFIG_USB_SUSPEND
 
 /*
@@ -2968,12 +2986,19 @@ int usb_port_suspend(struct usb_device *udev, 
pm_message_t msg)
dev_dbg(hub-intfdev, can't suspend port %d, status %d\n,
port1, status);
/* paranoia:  should not happen */
-   if (udev-do_remote_wakeup)
-   (void) usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
-   USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE,
-   USB_DEVICE_REMOTE_WAKEUP, 0,
-   NULL, 0,
-   USB_CTRL_SET_TIMEOUT);
+   if (udev-do_remote_wakeup) {
+   if (!hub_is_superspeed(hub-hdev)) {
+   (void) usb_control_msg(udev,
+   usb_sndctrlpipe(udev, 0),
+   USB_REQ_CLEAR_FEATURE,
+   USB_RECIP_DEVICE,
+   USB_DEVICE_REMOTE_WAKEUP, 0,
+   NULL, 0,
+   USB_CTRL_SET_TIMEOUT);
+   } else
+   (void) usb_disable_function_remotewakeup(udev);
+
+   }
 
/* Try to enable USB2 hardware LPM again */
if (udev-usb2_hw_lpm_capable == 1)
@@ -3059,20 +3084,30 @@ static int finish_port_resume(struct usb_device *udev)
dev_dbg(udev-dev, gone after usb resume? status %d\n,
status);
} else if (udev-actconfig) {
-   le16_to_cpus(devstatus);
-   if (devstatus  (1  USB_DEVICE_REMOTE_WAKEUP)) {
-   status = usb_control_msg(udev,
-   usb_sndctrlpipe(udev, 0),
-   USB_REQ_CLEAR_FEATURE,
+   if (!hub_is_superspeed(udev-parent)) {
+   le16_to_cpus(devstatus);
+   if (devstatus  (1  USB_DEVICE_REMOTE_WAKEUP))
+   status = usb_control_msg(udev,
+   usb_sndctrlpipe(udev, 0),
+   USB_REQ_CLEAR_FEATURE,
USB_RECIP_DEVICE,
-   USB_DEVICE_REMOTE_WAKEUP, 0,
-   NULL, 0,
-   USB_CTRL_SET_TIMEOUT);
-   if (status)
-   dev_dbg(udev-dev,
-   disable remote wakeup, status %d\n,
-   status);
+   USB_DEVICE_REMOTE_WAKEUP, 0,
+   NULL, 0,
+   USB_CTRL_SET_TIMEOUT);
+   } else {
+   status = usb_get_status(udev, USB_RECIP_INTERFACE, 0,
+   devstatus);
+   le16_to_cpus(devstatus);
+   if (!status  devstatus  (USB_INTRF_STAT_FUNC_RW_CAP
+   |