On Thu, 27 Sep 2012, Ming Lei wrote: > From: Ming Lei <tom.leiming@xxxxxxxxx> > > The hub status endpoint has a long 'bInterval', which is 255ms > for FS/LS device and 256ms for HS device according to USB 2.0 spec, > so the device connection change may be reported later more than 255ms > via status pipe. > > The connection change in hub may have been happened already on the > downstream ports, but no status URB completes when it is killed > in hub_suspend(auto), so the connection change may be missed by some > buggy hub devices, which won't generate remote wakeup signal after > their remote wakeup is enabled and they are put into suspend state. > > The problem can be observed at least on the below Genesys Logic, Inc. > hub devices: > > 0x05e3,0x0606 > 0x05e3,0x0608 > > In theory, there is no way to fix the problem completely, but we > can make it less likely to occur by this patch. > > This patch introduces one quirk of HUB_QUIRK_CHECK_PORTS_AUTOSUSPEND > to check ports' change during hub_suspend(auto) for the buggy > devices. If ports' change is found, terminate the auto suspend and > return to working state. > > So for the buggy hubs, if the connection change happend before > the ports' check, it can be handled correctly. If it happens between > the ports' check and enabling remote wakeup/entering suspend, it > will be missed. Considered the interval is quite short, it is very > less likely to happen during the window. One remark and one tiny problem... > @@ -3176,6 +3198,17 @@ static int hub_suspend(struct usb_interface *intf, pm_message_t msg) > return -EBUSY; > } > } > + > + if (hdev->do_remote_wakeup && hub->quirk_check_port_auto_suspend) { > + /* check if there are changes pending on hub ports */ > + if (check_ports_changed(hub)) { > + if (!PMSG_IS_AUTO(msg) && > + device_may_wakeup(&hdev->dev)) You don't need to check device_may_wakeup(); the core has already taken care of it. If device_may_wakeup() returns 0 then do_remote_wakeup will be 0 also. > + pm_wakeup_event(&hdev->dev, 2000); > + return -EBUSY; > + } > + } The !PMSG_IS_AUTO case should not return -EBUSY, because usb_suspend_both() will ignore the return value. It should proceed with the rest of the suspend. Alan Stern -- 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