Re: [RFC v2 11/11] USB: Fix connected device switch to Inactive state.

[Alan Stern]

On Tue, 4 Dec 2012, Sarah Sharp wrote:

 A USB 3.0 device can transition to the Inactive state if a U1 or U2 exit
 transition fails.  The current code in hub_events simply issues a warm
 reset, but does not call any pre-reset or post-reset driver methods (or
 unbind/rebind drivers without them).  Therefore the drivers won't know
 their device has just been reset.
 
 hub_events should instead call usb_reset_device.  This means
 hub_port_reset now needs to figure out whether it should issue a warm
 reset or a hot reset.
 
 Remove the FIXME note about needing disconnect() for a NOTATTACHED
 device.  This patch fixes that.

Actually, I think it was fixed many years ago but the FIXME comment 
was never removed.  :-)

 Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
 ---
  drivers/usb/core/hub.c |   29 -
  1 files changed, 24 insertions(+), 5 deletions(-)
 
 diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
 index a502857..9341302 100644
 --- a/drivers/usb/core/hub.c
 +++ b/drivers/usb/core/hub.c
 @@ -2609,7 +2609,6 @@ static void hub_port_finish_reset(struct usb_hub *hub, 
 int port1,
   case -ENODEV:
   clear_port_feature(hub-hdev,
   port1, USB_PORT_FEAT_C_RESET);
 - /* FIXME need disconnect() for NOTATTACHED device */
   if (hub_is_superspeed(hub-hdev)) {
   clear_port_feature(hub-hdev, port1,
   USB_PORT_FEAT_C_BH_PORT_RESET);
 @@ -2643,6 +2642,18 @@ static int hub_port_reset(struct usb_hub *hub, int 
 port1,
* Some companion controllers don't like it when they mix.
*/
   down_read(ehci_cf_port_reset_rwsem);
 + } else if (!warm) {
 + /*
 +  * If the caller hasn't explicitly requested a warm reset,
 +  * double check and see if one is needed.
 +  */
 + status = hub_port_status(hub, port1,
 + portstatus, portchange);
 + if (status  0)
 + goto done;
 +
 + if (hub_port_warm_reset_required(hub, portstatus))
 + warm = true;

Would it be better to turn warm into a 3-valued argument: true,
false, or dont_know?  Then this test could be skipped if the caller
wanted to do a regular reset.

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

[RFC v2 11/11] USB: Fix connected device switch to Inactive state.

[Sarah Sharp]

A USB 3.0 device can transition to the Inactive state if a U1 or U2 exit
transition fails.  The current code in hub_events simply issues a warm
reset, but does not call any pre-reset or post-reset driver methods (or
unbind/rebind drivers without them).  Therefore the drivers won't know
their device has just been reset.

hub_events should instead call usb_reset_device.  This means
hub_port_reset now needs to figure out whether it should issue a warm
reset or a hot reset.

Remove the FIXME note about needing disconnect() for a NOTATTACHED
device.  This patch fixes that.

Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
---
 drivers/usb/core/hub.c |   29 -
 1 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index a502857..9341302 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2609,7 +2609,6 @@ static void hub_port_finish_reset(struct usb_hub *hub, 
int port1,
case -ENODEV:
clear_port_feature(hub-hdev,
port1, USB_PORT_FEAT_C_RESET);
-   /* FIXME need disconnect() for NOTATTACHED device */
if (hub_is_superspeed(hub-hdev)) {
clear_port_feature(hub-hdev, port1,
USB_PORT_FEAT_C_BH_PORT_RESET);
@@ -2643,6 +2642,18 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
 * Some companion controllers don't like it when they mix.
 */
down_read(ehci_cf_port_reset_rwsem);
+   } else if (!warm) {
+   /*
+* If the caller hasn't explicitly requested a warm reset,
+* double check and see if one is needed.
+*/
+   status = hub_port_status(hub, port1,
+   portstatus, portchange);
+   if (status  0)
+   goto done;
+
+   if (hub_port_warm_reset_required(hub, portstatus))
+   warm = true;
}
 
/* Reset the port */
@@ -4694,12 +4705,20 @@ static void hub_events(void)
 */
if (hub_port_warm_reset_required(hub, portstatus)) {
int status;
+   struct usb_device *udev = hdev-children[i-1];
 
dev_dbg(hub_dev, warm reset port %d\n, i);
-   status = hub_port_reset(hub, i, NULL,
-   HUB_BH_RESET_TIME, true);
-   if (status  0)
-   hub_port_disable(hub, i, 1);
+   if (!udev) {
+   status = hub_port_reset(hub, i,
+   NULL, HUB_BH_RESET_TIME,
+   true);
+   if (status  0)
+   hub_port_disable(hub, i, 1);
+   } else {
+   usb_lock_device(udev);
+   status = usb_reset_device(udev);
+   usb_unlock_device(udev);
+   }
connect_change = 0;
}
 
-- 
1.7.9

--
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