On 06/12/2014 08:11 PM, Alan Stern wrote:
> On Wed, 11 Jun 2014, Oliver Neukum wrote:
>
>> Alan,
>>
>> I don't like this, but it might be enough to simply
>> revert 0aa2832dd0d9d8609fd8f15139bc7572541a1215.
>> I am afraid we have to deal with real hardware, not specs.
>
> Toralf and Adam:
>
> Does this patch (for 3.14 or 3.15) fix the problems you observed?
>
At my system (32 bit stable Gentoo at a ThinkkPad T420) it is not fixed
- tested against 3.14.7 and 3.15.0.
> Alan Stern
>
>
>
> Index: usb-3.15/drivers/usb/core/hub.c
> ===================================================================
> --- usb-3.15.orig/drivers/usb/core/hub.c
> +++ usb-3.15/drivers/usb/core/hub.c
> @@ -2922,15 +2922,6 @@ static int usb_disable_remote_wakeup(str
> USB_CTRL_SET_TIMEOUT);
> }
>
> -/* Count of wakeup-enabled devices at or below udev */
> -static unsigned wakeup_enabled_descendants(struct usb_device *udev)
> -{
> - struct usb_hub *hub = usb_hub_to_struct_hub(udev);
> -
> - return udev->do_remote_wakeup +
> - (hub ? hub->wakeup_enabled_descendants : 0);
> -}
> -
> /*
> * usb_port_suspend - suspend a usb device's upstream port
> * @udev: device that's no longer in active use, not a root hub
> @@ -2971,12 +2962,6 @@ static unsigned wakeup_enabled_descendan
> * Linux (2.6) currently has NO mechanisms to initiate that: no khubd
> * timer, no SRP, no requests through sysfs.
> *
> - * If Runtime PM isn't enabled or used, non-SuperSpeed devices may not get
> - * suspended until their bus goes into global suspend (i.e., the root
> - * hub is suspended). Nevertheless, we change @udev->state to
> - * USB_STATE_SUSPENDED as this is the device's "logical" state. The actual
> - * upstream port setting is stored in @udev->port_is_suspended.
> - *
> * Returns 0 on success, else negative errno.
> */
> int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
> @@ -2985,7 +2970,6 @@ int usb_port_suspend(struct usb_device *
> struct usb_port *port_dev = hub->ports[udev->portnum - 1];
> int port1 = udev->portnum;
> int status;
> - bool really_suspend = true;
>
> /* enable remote wakeup when appropriate; this lets the device
> * wake up the upstream hub (including maybe the root hub).
> @@ -3024,25 +3008,9 @@ int usb_port_suspend(struct usb_device *
> /* see 7.1.7.6 */
> if (hub_is_superspeed(hub->hdev))
> status = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_U3);
> -
> - /*
> - * For system suspend, we do not need to enable the suspend feature
> - * on individual USB-2 ports. The devices will automatically go
> - * into suspend a few ms after the root hub stops sending packets.
> - * The USB 2.0 spec calls this "global suspend".
> - *
> - * However, many USB hubs have a bug: They don't relay wakeup requests
> - * from a downstream port if the port's suspend feature isn't on.
> - * Therefore we will turn on the suspend feature if udev or any of its
> - * descendants is enabled for remote wakeup.
> - */
> - else if (PMSG_IS_AUTO(msg) || wakeup_enabled_descendants(udev) > 0)
> + else
> status = set_port_feature(hub->hdev, port1,
> - USB_PORT_FEAT_SUSPEND);
> - else {
> - really_suspend = false;
> - status = 0;
> - }
> + USB_PORT_FEAT_SUSPEND);
> if (status) {
> dev_dbg(hub->intfdev, "can't suspend port %d, status %d\n",
> port1, status);
> @@ -3067,12 +3035,10 @@ int usb_port_suspend(struct usb_device *
> dev_dbg(&udev->dev, "usb %ssuspend, wakeup %d\n",
> (PMSG_IS_AUTO(msg) ? "auto-" : ""),
> udev->do_remote_wakeup);
> - if (really_suspend) {
> - udev->port_is_suspended = 1;
> + udev->port_is_suspended = 1;
>
> - /* device has up to 10 msec to fully suspend */
> - msleep(10);
> - }
> + /* device has up to 10 msec to fully suspend */
> + msleep(10);
> usb_set_device_state(udev, USB_STATE_SUSPENDED);
> }
>
> @@ -3343,11 +3309,7 @@ static int hub_suspend(struct usb_interf
> unsigned port1;
> int status;
>
> - /*
> - * Warn if children aren't already suspended.
> - * Also, add up the number of wakeup-enabled descendants.
> - */
> - hub->wakeup_enabled_descendants = 0;
> + /* Warn if children aren't already suspended */
> for (port1 = 1; port1 <= hdev->maxchild; port1++) {
> struct usb_device *udev;
>
> @@ -3358,9 +3320,6 @@ static int hub_suspend(struct usb_interf
> if (PMSG_IS_AUTO(msg))
> return -EBUSY;
> }
> - if (udev)
> - hub->wakeup_enabled_descendants +=
> - wakeup_enabled_descendants(udev);
> }
>
> if (hdev->do_remote_wakeup && hub->quirk_check_port_auto_suspend) {
> Index: usb-3.15/drivers/usb/core/hub.h
> ===================================================================
> --- usb-3.15.orig/drivers/usb/core/hub.h
> +++ usb-3.15/drivers/usb/core/hub.h
> @@ -59,9 +59,6 @@ struct usb_hub {
> struct usb_tt tt; /* Transaction Translator */
>
> unsigned mA_per_port; /* current for each child */
> -#ifdef CONFIG_PM
> - unsigned wakeup_enabled_descendants;
> -#endif
>
> unsigned limited_power:1;
> unsigned quiescing:1;
>
>
--
Toralf
--
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
