On Thu, Oct 18, 2012 at 03:16:04PM -0400, Alan Stern wrote:
> On Thu, 18 Oct 2012, Peter Chen wrote:
>
> > > I decided to use a different approach. The core should know which
> > > ports are suspended without asking the HCD. Can you test this patch in
> > > place of the other one?
> > It shows below oops, you may not consider there is no device at port
> > condition.
>
> Quite right. I consider this to be a bug in hub_for_each_child; the
> first patch below fixes that bug.
>
> > Besides, how about using usleep_range instead of msleep? msleep is not
> > precise, and should not be used at (<20ms) condition, at my platform,
> > msleep 10 consumes about 19-20ms.
>
> Good idea; the second patch below is updated to use usleep_range
> instead of msleep. These two patches together should do what you want.
>
> Alan Stern
>
>
>
> First patch: Fix hub_for_each_child
>
> Index: usb-3.6/include/linux/usb.h
> ===================================================================
> --- usb-3.6.orig/include/linux/usb.h
> +++ usb-3.6/include/linux/usb.h
> @@ -588,8 +588,9 @@ extern struct usb_device *usb_hub_find_c
> */
> #define usb_hub_for_each_child(hdev, port1, child) \
> for (port1 = 1, child = usb_hub_find_child(hdev, port1); \
> - port1 <= hdev->maxchild; \
> - child = usb_hub_find_child(hdev, ++port1))
> + port1 <= hdev->maxchild; \
> + child = usb_hub_find_child(hdev, ++port1)) \
> + if (!child) continue; else
>
> /* USB device locking */
> #define usb_lock_device(udev) device_lock(&(udev)->dev)
>
>
>
> Second patch: Speed up bus resumes
>
> Index: usb-3.6/include/linux/usb.h
> ===================================================================
> --- usb-3.6.orig/include/linux/usb.h
> +++ usb-3.6/include/linux/usb.h
> @@ -482,6 +482,7 @@ struct usb3_lpm_parameters {
> * @connect_time: time device was first connected
> * @do_remote_wakeup: remote wakeup should be enabled
> * @reset_resume: needs reset instead of resume
> + * @port_is_suspended: the upstream port is suspended (L2 or U3)
> * @wusb_dev: if this is a Wireless USB device, link to the WUSB
> * specific data for the device.
> * @slot_id: Slot ID assigned by xHCI
> @@ -560,6 +561,7 @@ struct usb_device {
>
> unsigned do_remote_wakeup:1;
> unsigned reset_resume:1;
> + unsigned port_is_suspended:1;
> #endif
> struct wusb_dev *wusb_dev;
> int slot_id;
> Index: usb-3.6/drivers/usb/core/hub.c
> ===================================================================
> --- usb-3.6.orig/drivers/usb/core/hub.c
> +++ usb-3.6/drivers/usb/core/hub.c
> @@ -2876,6 +2876,7 @@ int usb_port_suspend(struct usb_device *
> (PMSG_IS_AUTO(msg) ? "auto-" : ""),
> udev->do_remote_wakeup);
> usb_set_device_state(udev, USB_STATE_SUSPENDED);
> + udev->port_is_suspended = 1;
> msleep(10);
> }
> usb_mark_last_busy(hub->hdev);
> @@ -3040,6 +3041,7 @@ int usb_port_resume(struct usb_device *u
>
> SuspendCleared:
> if (status == 0) {
> + udev->port_is_suspended = 0;
> if (hub_is_superspeed(hub->hdev)) {
> if (portchange & USB_PORT_STAT_C_LINK_STATE)
> clear_port_feature(hub->hdev, port1,
> Index: usb-3.6/drivers/usb/core/hcd.c
> ===================================================================
> --- usb-3.6.orig/drivers/usb/core/hcd.c
> +++ usb-3.6/drivers/usb/core/hcd.c
> @@ -2039,8 +2039,9 @@ int hcd_bus_resume(struct usb_device *rh
> status = hcd->driver->bus_resume(hcd);
> clear_bit(HCD_FLAG_WAKEUP_PENDING, &hcd->flags);
> if (status == 0) {
> - /* TRSMRCY = 10 msec */
> - msleep(10);
> + struct usb_device *udev;
> + int port1;
> +
> spin_lock_irq(&hcd_root_hub_lock);
> if (!HCD_DEAD(hcd)) {
> usb_set_device_state(rhdev, rhdev->actconfig
> @@ -2050,6 +2051,20 @@ int hcd_bus_resume(struct usb_device *rh
> hcd->state = HC_STATE_RUNNING;
> }
> spin_unlock_irq(&hcd_root_hub_lock);
> +
> + /*
> + * Check whether any of the enabled ports on the root hub are
> + * unsuspended. If they are then a TRSMRCY delay is needed
> + * (this is what the USB-2 spec calls a "global resume").
> + * Otherwise we can skip the delay.
> + */
> + usb_hub_for_each_child(rhdev, port1, udev) {
> + if (udev->state != USB_STATE_NOTATTACHED &&
> + !udev->port_is_suspended) {
> + usleep_range(10000, 11000); /* TRSMRCY */
> + break;
> + }
> + }
> } else {
> hcd->state = old_state;
> dev_dbg(&rhdev->dev, "bus %s fail, err %d\n",
>
>
Reviewed-by: Peter Chen <peter.chen@xxxxxxxxxxxxx>
Tested-by: Peter Chen <peter.chen@xxxxxxxxxxxxx>
--
Best Regards,
Peter Chen
--
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
