>-----Original Message----- >From: Sarah Sharp [mailto:sarah.a.sharp@xxxxxxxxxxxxxxx] >Sent: Tuesday, January 07, 2014 12:16 AM >To: Alan Stern >Cc: linux-usb@xxxxxxxxxxxxxxx; Nandibasappa, GirishX; Venkatesh Murthy, HemanthX >Subject: Re: [PATCH] usb: Skip U1/U2 LPM disable if device is disconnected. > >On Sat, Jan 04, 2014 at 12:09:27PM -0500, Alan Stern wrote: >> On Fri, 3 Jan 2014, Sarah Sharp wrote: >> >> > Occasionally when a USB 3.0 device is disconnected, the roothub port >> > goes into the SS.Inactive state, rather than reporting a disconnect. >> > A warm reset is the only way to get out of this state. khubd >> > notices the link state in hub_port_events(), >> >> Do you mean hub_events()? There is no hub_port_events() routine. > >Yes. > >> LPM is disabled before the port is reset. At that time, the port is >> still in the SS.Inactive state, and the port-connect bit is the same >> as it was when hub_events() first read the port status. So how will >> your new hub_is_device_disconnected() routine be able to do any better >> than the existing code already does? >> >> It sounds like what you really want to do is balance the LPM count but >> skip sending the actual request if the port is in the SS.Inactive >> state. > >Yes, that's what this patch is trying to do. It skips sending the >U1/U2 disable request if the device is disconnected. The connect status bit is always set to disconnected if the port is in SS.Inactive. > >Hmm, and now I see why you're confused. The current code in >hub_events() skips usb_reset_device() if the connect status is >disconnected: > > if (hub_port_warm_reset_required(hub, portstatus)) { > int status; > struct usb_device *udev = > hub->ports[i - 1]->child; > > dev_dbg(hub_dev, "warm reset port %d\n", i); > if (!udev || > !(portstatus & USB_PORT_STAT_CONNECTION) || > udev->state == USB_STATE_NOTATTACHED) { > 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; > } > >I missed that when preparing the patch. I now suspect that the only reason Girish and Hemanth ran into the U1/U2 timeout issue is >because they're working on an older kernel that doesn't have commit >f3e94aa15dc3d9155f8fc4a3295866d7a207b4e5 "usb: core: don't try to >reset_device() a port that got just disconnected" (added in 3.12). We tried pulling this commit to our tree, we still see below errors multiple times. Should hub_port_reset also be avoided in Inactive state. 7>[ 181.542976] hub 2-0:1.0: port 2 not reset yet, waiting 50ms <7>[ 181.593907] hub 2-0:1.0: port 2 not reset yet, waiting 200ms <7>[ 181.794881] hub 2-0:1.0: port 2 not reset yet, waiting 200ms <7>[ 181.995826] hub 2-0:1.0: port 2 not reset yet, waiting 200ms <7>[ 182.196746] hub 2-0:1.0: port 2 not reset yet, waiting 200ms <7>[ 182.196766] hub 2-0:1.0: port_wait_reset: err = -16 Thanks Hemanth -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
