| Message ID | 20171011064753.11471-1-drake@endlessm.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Wed, 11 Oct 2017, Daniel Drake wrote: > From: Chris Chiu <chiu@endlessm.com> > > When going into S3 suspend, the Acer TravelMate P648-M and P648-G3 > laptops immediately wake up 3-4 seconds later for no obvious reason. > > Unbinding the integrated Huawei 4G LTE modem before suspend avoids > the issue, even though we are not using the modem at all (checked > from rescue.target/runlevel1). The problem also occurs when the option > and cdc-ether modem drivers aren't loaded; it reproduces just with the > base usb driver. Under Windows the system can suspend fine. > > Seeking a better fix, we've tried a lot of things, including: > - Check that the device's power/wakeup is disabled > - Check that remote wakeup is off at the USB level > - All the quirks in drivers/usb/core/quirks.c e.g. USB_QUIRK_RESET_RESUME, > USB_QUIRK_RESET, USB_QUIRK_IGNORE_REMOTE_WAKEUP, USB_QUIRK_NO_LPM. > > but none of that makes any difference. > > There are no errors in the logs showing any suspend/resume-related issues. > When the system wakes up due to the modem, log-wise it appears to be a > normal resume. > > Introduce a quirk to disable the port during suspend when the modem is > detected. > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -3160,6 +3160,9 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg) > goto err_ltm; > } > > + if (udev->quirks & USB_QUIRK_DISCONNECT_SUSPEND) > + usb_clear_port_feature(hub->hdev, port1, USB_PORT_FEAT_ENABLE); > + > /* see 7.1.7.6 */ > if (hub_is_superspeed(hub->hdev)) > status = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_U3); I'm not so sure this is the right way to do it. If you turn off the enable feature then the following calls to turn on the suspend feature will fail. At the very least, you should change the following line so that it does: else if (hub_is_superspeed(hub->hdev)) ... Also, you should check whether this is for a runtime suspend or a system suspend. You don't want to go around disconnecting a device whenever it gets runtime suspended! Alan Stern
On Wed, Oct 11, 2017 at 11:04 PM, Alan Stern <stern@rowland.harvard.edu> wrote: > Also, you should check whether this is for a runtime suspend or a > system suspend. You don't want to go around disconnecting a device > whenever it gets runtime suspended! Good point, I had not considered runtime suspend. It's not quite so simple though. I make your suggested change (testing PMSG_IS_AUTO in this codepath), then enable autosuspend with: echo auto > /sys/bus/usb/devices/1-9/power/control then the device gets suspended (no interface drivers are loaded) without the port disconnect happening. Now if I go into S3 suspend, the original problem returns: the system wakes up immediately. So that is an imperfection with this approach. Any suggestions for how to proceed? Daniel
On Thu, 12 Oct 2017, Daniel Drake wrote: > On Wed, Oct 11, 2017 at 11:04 PM, Alan Stern <stern@rowland.harvard.edu> wrote: > > Also, you should check whether this is for a runtime suspend or a > > system suspend. You don't want to go around disconnecting a device > > whenever it gets runtime suspended! > > Good point, I had not considered runtime suspend. It's not quite so > simple though. > > I make your suggested change (testing PMSG_IS_AUTO in this codepath), > then enable autosuspend with: > echo auto > /sys/bus/usb/devices/1-9/power/control > > then the device gets suspended (no interface drivers are loaded) > without the port disconnect happening. > > Now if I go into S3 suspend, the original problem returns: the system > wakes up immediately. Nasty! > So that is an imperfection with this approach. Any suggestions for how > to proceed? How about moving the test into usb_suspend() in driver.c? If the quirk flag is set at that point, call hub_port_disable(). You'll have to add a glue routine to hub.c for this purpose. Alan Stern
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index b5c733613823..0eb3d8191a26 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -3160,6 +3160,9 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg) goto err_ltm; } + if (udev->quirks & USB_QUIRK_DISCONNECT_SUSPEND) + usb_clear_port_feature(hub->hdev, port1, USB_PORT_FEAT_ENABLE); + /* see 7.1.7.6 */ if (hub_is_superspeed(hub->hdev)) status = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_U3); diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c index 82806e311202..746d2b19109c 100644 --- a/drivers/usb/core/quirks.c +++ b/drivers/usb/core/quirks.c @@ -203,6 +203,12 @@ static const struct usb_device_id usb_quirk_list[] = { { USB_DEVICE(0x10d6, 0x2200), .driver_info = USB_QUIRK_STRING_FETCH_255 }, + /* Huawei 4G LTE module */ + { USB_DEVICE(0x12d1, 0x15bb), .driver_info = + USB_QUIRK_DISCONNECT_SUSPEND }, + { USB_DEVICE(0x12d1, 0x15c3), .driver_info = + USB_QUIRK_DISCONNECT_SUSPEND }, + /* SKYMEDI USB_DRIVE */ { USB_DEVICE(0x1516, 0x8628), .driver_info = USB_QUIRK_RESET_RESUME }, diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h index de2a722fe3cf..bdc639cc80b4 100644 --- a/include/linux/usb/quirks.h +++ b/include/linux/usb/quirks.h @@ -56,4 +56,10 @@ */ #define USB_QUIRK_LINEAR_FRAME_INTR_BINTERVAL BIT(11) +/* + * Device needs to be disconnected before suspend to prevent spurious + * wakeup. + */ +#define USB_QUIRK_DISCONNECT_SUSPEND BIT(12) + #endif /* __LINUX_USB_QUIRKS_H */