From 62f0342de1f012f3e90607d39e20fce811391169 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Fri, 13 Feb 2015 14:34:25 -0600 Subject: [PATCH 01/14] usb: define a generic USB_RESUME_TIMEOUT macro Every USB Host controller should use this new macro to define for how long resume signalling should be driven on the bus. Currently, almost every single USB controller is using a 20ms timeout for resume signalling. That's problematic for two reasons: a) sometimes that 20ms timer expires a little before 20ms, which makes us fail certification b) some (many) devices actually need more than 20ms resume signalling. Sure, in case of (b) we can state that the device is against the USB spec, but the fact is that we have no control over which device the certification lab will use. We also have no control over which host they will use. Most likely they'll be using a Windows PC which, again, we have no control over how that USB stack is written and how long resume signalling they are using. At the end of the day, we must make sure Linux passes electrical compliance when working as Host or as Device and currently we don't pass compliance as host because we're driving resume signallig for exactly 20ms and that confuses certification test setup resulting in Certification failure. Cc: # v3.10+ Acked-by: Greg Kroah-Hartman Acked-by: Peter Chen Signed-off-by: Felipe Balbi --- include/linux/usb.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/include/linux/usb.h b/include/linux/usb.h index 7ee1b5c3b4cb..447fe29b55b4 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -205,6 +205,32 @@ void usb_put_intf(struct usb_interface *intf); #define USB_MAXINTERFACES 32 #define USB_MAXIADS (USB_MAXINTERFACES/2) +/* + * USB Resume Timer: Every Host controller driver should drive the resume + * signalling on the bus for the amount of time defined by this macro. + * + * That way we will have a 'stable' behavior among all HCDs supported by Linux. + * + * Note that the USB Specification states we should drive resume for *at least* + * 20 ms, but it doesn't give an upper bound. This creates two possible + * situations which we want to avoid: + * + * (a) sometimes an msleep(20) might expire slightly before 20 ms, which causes + * us to fail USB Electrical Tests, thus failing Certification + * + * (b) Some (many) devices actually need more than 20 ms of resume signalling, + * and while we can argue that's against the USB Specification, we don't have + * control over which devices a certification laboratory will be using for + * certification. If CertLab uses a device which was tested against Windows and + * that happens to have relaxed resume signalling rules, we might fall into + * situations where we fail interoperability and electrical tests. + * + * In order to avoid both conditions, we're using a 40 ms resume timeout, which + * should cope with both LPJ calibration errors and devices not following every + * detail of the USB Specification. + */ +#define USB_RESUME_TIMEOUT 40 /* ms */ + /** * struct usb_interface_cache - long-term representation of a device interface * @num_altsetting: number of altsettings defined. From b9e451885deb6262dbaf5cd14aa77d192d9ac759 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Fri, 13 Feb 2015 14:39:13 -0600 Subject: [PATCH 02/14] usb: host: xhci: use new USB_RESUME_TIMEOUT Make sure we're using the new macro, so our resume signaling will always pass certification. Cc: # v3.10+ Acked-by: Mathias Nyman Signed-off-by: Felipe Balbi --- drivers/usb/host/xhci-ring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 5fb66db89e05..23304817a89a 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1574,7 +1574,7 @@ static void handle_port_status(struct xhci_hcd *xhci, } else { xhci_dbg(xhci, "resume HS port %d\n", port_id); bus_state->resume_done[faked_port_index] = jiffies + - msecs_to_jiffies(20); + msecs_to_jiffies(USB_RESUME_TIMEOUT); set_bit(faked_port_index, &bus_state->resuming_ports); mod_timer(&hcd->rh_timer, bus_state->resume_done[faked_port_index]); From ea16328f80ca8d74434352157f37ef60e2f55ce2 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Fri, 13 Feb 2015 14:42:25 -0600 Subject: [PATCH 03/14] usb: host: ehci: use new USB_RESUME_TIMEOUT Make sure we're using the new macro, so our resume signaling will always pass certification. Cc: # v3.10+ Signed-off-by: Felipe Balbi --- drivers/usb/host/ehci-hcd.c | 10 +++++----- drivers/usb/host/ehci-hub.c | 9 ++++++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 85e56d1abd23..f4d88dfb26a7 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -792,12 +792,12 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd) ehci->reset_done[i] == 0)) continue; - /* start 20 msec resume signaling from this port, - * and make hub_wq collect PORT_STAT_C_SUSPEND to - * stop that signaling. Use 5 ms extra for safety, - * like usb_port_resume() does. + /* start USB_RESUME_TIMEOUT msec resume signaling from + * this port, and make hub_wq collect + * PORT_STAT_C_SUSPEND to stop that signaling. */ - ehci->reset_done[i] = jiffies + msecs_to_jiffies(25); + ehci->reset_done[i] = jiffies + + msecs_to_jiffies(USB_RESUME_TIMEOUT); set_bit(i, &ehci->resuming_ports); ehci_dbg (ehci, "port %d remote wakeup\n", i + 1); usb_hcd_start_port_resume(&hcd->self, i); diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c index 87cf86f38b36..7354d0129a72 100644 --- a/drivers/usb/host/ehci-hub.c +++ b/drivers/usb/host/ehci-hub.c @@ -471,10 +471,13 @@ static int ehci_bus_resume (struct usb_hcd *hcd) ehci_writel(ehci, temp, &ehci->regs->port_status [i]); } - /* msleep for 20ms only if code is trying to resume port */ + /* + * msleep for USB_RESUME_TIMEOUT ms only if code is trying to resume + * port + */ if (resume_needed) { spin_unlock_irq(&ehci->lock); - msleep(20); + msleep(USB_RESUME_TIMEOUT); spin_lock_irq(&ehci->lock); if (ehci->shutdown) goto shutdown; @@ -942,7 +945,7 @@ int ehci_hub_control( temp &= ~PORT_WAKE_BITS; ehci_writel(ehci, temp | PORT_RESUME, status_reg); ehci->reset_done[wIndex] = jiffies - + msecs_to_jiffies(20); + + msecs_to_jiffies(USB_RESUME_TIMEOUT); set_bit(wIndex, &ehci->resuming_ports); usb_hcd_start_port_resume(&hcd->self, wIndex); break; From b8fb6f79f76f478acbbffccc966daa878f172a0a Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Fri, 13 Feb 2015 14:44:17 -0600 Subject: [PATCH 04/14] usb: host: uhci: use new USB_RESUME_TIMEOUT Make sure we're using the new macro, so our resume signaling will always pass certification. Cc: # v3.10+ Signed-off-by: Felipe Balbi --- drivers/usb/host/uhci-hub.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/uhci-hub.c b/drivers/usb/host/uhci-hub.c index 19ba5eafb31e..7b3d1afcc14a 100644 --- a/drivers/usb/host/uhci-hub.c +++ b/drivers/usb/host/uhci-hub.c @@ -166,7 +166,7 @@ static void uhci_check_ports(struct uhci_hcd *uhci) /* Port received a wakeup request */ set_bit(port, &uhci->resuming_ports); uhci->ports_timeout = jiffies + - msecs_to_jiffies(25); + msecs_to_jiffies(USB_RESUME_TIMEOUT); usb_hcd_start_port_resume( &uhci_to_hcd(uhci)->self, port); @@ -338,7 +338,8 @@ static int uhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, uhci_finish_suspend(uhci, port, port_addr); /* USB v2.0 7.1.7.5 */ - uhci->ports_timeout = jiffies + msecs_to_jiffies(50); + uhci->ports_timeout = jiffies + + msecs_to_jiffies(USB_RESUME_TIMEOUT); break; case USB_PORT_FEAT_POWER: /* UHCI has no power switching */ From 309be239369609929d5d3833ee043f7c5afc95d1 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Fri, 13 Feb 2015 14:46:27 -0600 Subject: [PATCH 05/14] usb: musb: use new USB_RESUME_TIMEOUT Make sure we're using the new macro, so our resume signaling will always pass certification. Based on original work by Bin Liu > Cc: Bin Liu Cc: # v3.10+ Signed-off-by: Felipe Balbi --- drivers/usb/musb/musb_core.c | 7 ++++--- drivers/usb/musb/musb_virthub.c | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index a48b5a9c6c47..3789b08ef67b 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -99,6 +99,7 @@ #include #include #include +#include #include "musb_core.h" @@ -549,7 +550,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, (USB_PORT_STAT_C_SUSPEND << 16) | MUSB_PORT_STAT_RESUME; musb->rh_timer = jiffies - + msecs_to_jiffies(20); + + msecs_to_jiffies(USB_RESUME_TIMEOUT); musb->need_finish_resume = 1; musb->xceiv->otg->state = OTG_STATE_A_HOST; @@ -2463,7 +2464,7 @@ static int musb_resume(struct device *dev) if (musb->need_finish_resume) { musb->need_finish_resume = 0; schedule_delayed_work(&musb->finish_resume_work, - msecs_to_jiffies(20)); + msecs_to_jiffies(USB_RESUME_TIMEOUT)); } /* @@ -2506,7 +2507,7 @@ static int musb_runtime_resume(struct device *dev) if (musb->need_finish_resume) { musb->need_finish_resume = 0; schedule_delayed_work(&musb->finish_resume_work, - msecs_to_jiffies(20)); + msecs_to_jiffies(USB_RESUME_TIMEOUT)); } return 0; diff --git a/drivers/usb/musb/musb_virthub.c b/drivers/usb/musb/musb_virthub.c index 294e159f4afe..5428ed11440d 100644 --- a/drivers/usb/musb/musb_virthub.c +++ b/drivers/usb/musb/musb_virthub.c @@ -136,7 +136,7 @@ void musb_port_suspend(struct musb *musb, bool do_suspend) /* later, GetPortStatus will stop RESUME signaling */ musb->port1_status |= MUSB_PORT_STAT_RESUME; schedule_delayed_work(&musb->finish_resume_work, - msecs_to_jiffies(20)); + msecs_to_jiffies(USB_RESUME_TIMEOUT)); } } From 8c0ae6574ccfd3d619876a65829aad74c9d22ba5 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Fri, 13 Feb 2015 14:50:10 -0600 Subject: [PATCH 06/14] usb: host: isp116x: use new USB_RESUME_TIMEOUT Make sure we're using the new macro, so our resume signaling will always pass certification. Cc: # v3.10+ Signed-off-by: Felipe Balbi --- drivers/usb/host/isp116x-hcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/isp116x-hcd.c b/drivers/usb/host/isp116x-hcd.c index 113d0cc6cc43..9ef56443446d 100644 --- a/drivers/usb/host/isp116x-hcd.c +++ b/drivers/usb/host/isp116x-hcd.c @@ -1490,7 +1490,7 @@ static int isp116x_bus_resume(struct usb_hcd *hcd) spin_unlock_irq(&isp116x->lock); hcd->state = HC_STATE_RESUMING; - msleep(20); + msleep(USB_RESUME_TIMEOUT); /* Go operational */ spin_lock_irq(&isp116x->lock); From 7e136bb71a08e8b8be3bc492f041d9b0bea3856d Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Fri, 13 Feb 2015 14:54:38 -0600 Subject: [PATCH 07/14] usb: host: fotg210: use new USB_RESUME_TIMEOUT Make sure we're using the new macro, so our resume signaling will always pass certification. Cc: # v3.10+ Signed-off-by: Felipe Balbi --- drivers/usb/host/fotg210-hcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c index 475b21fd373b..7a6681fb7675 100644 --- a/drivers/usb/host/fotg210-hcd.c +++ b/drivers/usb/host/fotg210-hcd.c @@ -1595,7 +1595,7 @@ static int fotg210_hub_control( /* resume signaling for 20 msec */ fotg210_writel(fotg210, temp | PORT_RESUME, status_reg); fotg210->reset_done[wIndex] = jiffies - + msecs_to_jiffies(20); + + msecs_to_jiffies(USB_RESUME_TIMEOUT); break; case USB_PORT_FEAT_C_SUSPEND: clear_bit(wIndex, &fotg210->port_c_suspend); From 595227db1f2d98bfc33f02a55842f268e12b247d Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Fri, 13 Feb 2015 14:55:34 -0600 Subject: [PATCH 08/14] usb: host: fusbh200: use new USB_RESUME_TIMEOUT Make sure we're using the new macro, so our resume signaling will always pass certification. Cc: # v3.10+ Signed-off-by: Felipe Balbi --- drivers/usb/host/fusbh200-hcd.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c index a83eefefffda..ba77e2e43f62 100644 --- a/drivers/usb/host/fusbh200-hcd.c +++ b/drivers/usb/host/fusbh200-hcd.c @@ -1550,10 +1550,9 @@ static int fusbh200_hub_control ( if ((temp & PORT_PE) == 0) goto error; - /* resume signaling for 20 msec */ fusbh200_writel(fusbh200, temp | PORT_RESUME, status_reg); fusbh200->reset_done[wIndex] = jiffies - + msecs_to_jiffies(20); + + msecs_to_jiffies(USB_RESUME_TIMEOUT); break; case USB_PORT_FEAT_C_SUSPEND: clear_bit(wIndex, &fusbh200->port_c_suspend); From 84c0d178eb9f3a3ae4d63dc97a440266cf17f7f5 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Fri, 13 Feb 2015 14:57:54 -0600 Subject: [PATCH 09/14] usb: host: oxu210hp: use new USB_RESUME_TIMEOUT Make sure we're using the new macro, so our resume signaling will always pass certification. Cc: # v3.10+ Signed-off-by: Felipe Balbi --- drivers/usb/host/oxu210hp-hcd.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c index ef7efb278b15..28a2866b6b16 100644 --- a/drivers/usb/host/oxu210hp-hcd.c +++ b/drivers/usb/host/oxu210hp-hcd.c @@ -2500,11 +2500,12 @@ static irqreturn_t oxu210_hcd_irq(struct usb_hcd *hcd) || oxu->reset_done[i] != 0) continue; - /* start 20 msec resume signaling from this port, - * and make hub_wq collect PORT_STAT_C_SUSPEND to + /* start USB_RESUME_TIMEOUT resume signaling from this + * port, and make hub_wq collect PORT_STAT_C_SUSPEND to * stop that signaling. */ - oxu->reset_done[i] = jiffies + msecs_to_jiffies(20); + oxu->reset_done[i] = jiffies + + msecs_to_jiffies(USB_RESUME_TIMEOUT); oxu_dbg(oxu, "port %d remote wakeup\n", i + 1); mod_timer(&hcd->rh_timer, oxu->reset_done[i]); } From 7a606ac29752a3e571b83f9b3fceb1eaa1d37781 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Fri, 13 Feb 2015 14:58:53 -0600 Subject: [PATCH 10/14] usb: host: r8a66597: use new USB_RESUME_TIMEOUT While this driver was already using a 50ms resume timeout, let's make sure everybody uses the same macro so it's easy to fix later should anything go wrong. It also gives a more "stable" expectation to Linux users. Cc: # v3.10+ Signed-off-by: Felipe Balbi --- drivers/usb/host/r8a66597-hcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/r8a66597-hcd.c b/drivers/usb/host/r8a66597-hcd.c index bdc82fea0a1f..54a417043e44 100644 --- a/drivers/usb/host/r8a66597-hcd.c +++ b/drivers/usb/host/r8a66597-hcd.c @@ -2301,7 +2301,7 @@ static int r8a66597_bus_resume(struct usb_hcd *hcd) rh->port &= ~USB_PORT_STAT_SUSPEND; rh->port |= USB_PORT_STAT_C_SUSPEND << 16; r8a66597_mdfy(r8a66597, RESUME, RESUME | UACT, dvstctr_reg); - msleep(50); + msleep(USB_RESUME_TIMEOUT); r8a66597_mdfy(r8a66597, UACT, RESUME | UACT, dvstctr_reg); } From 08debfb13b199716da6153940c31968c556b195d Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Fri, 13 Feb 2015 15:00:38 -0600 Subject: [PATCH 11/14] usb: host: sl811: use new USB_RESUME_TIMEOUT Make sure we're using the new macro, so our resume signaling will always pass certification. Cc: # v3.10+ Signed-off-by: Felipe Balbi --- drivers/usb/host/sl811-hcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/sl811-hcd.c b/drivers/usb/host/sl811-hcd.c index 4f4ba1ea9e9b..9118cd8de1a7 100644 --- a/drivers/usb/host/sl811-hcd.c +++ b/drivers/usb/host/sl811-hcd.c @@ -1259,7 +1259,7 @@ sl811h_hub_control( sl811_write(sl811, SL11H_CTLREG1, sl811->ctrl1); mod_timer(&sl811->timer, jiffies - + msecs_to_jiffies(20)); + + msecs_to_jiffies(USB_RESUME_TIMEOUT)); break; case USB_PORT_FEAT_POWER: port_power(sl811, 0); From 74bd7b69801819707713b88e9d0bc074efa2f5e7 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Fri, 13 Feb 2015 15:03:13 -0600 Subject: [PATCH 12/14] usb: dwc2: hcd: use new USB_RESUME_TIMEOUT Make sure we're using the new macro, so our resume signaling will always pass certification. Cc: # v3.10+ Signed-off-by: Felipe Balbi --- drivers/usb/dwc2/hcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 559b55e5debb..545e094e7449 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -1529,7 +1529,7 @@ static int dwc2_hcd_hub_control(struct dwc2_hsotg *hsotg, u16 typereq, dev_dbg(hsotg->dev, "ClearPortFeature USB_PORT_FEAT_SUSPEND\n"); writel(0, hsotg->regs + PCGCTL); - usleep_range(20000, 40000); + msleep(USB_RESUME_TIMEOUT); hprt0 = dwc2_read_hprt0(hsotg); hprt0 |= HPRT0_RES; From 59c9904cce77b55892e15f40791f1e66e4d3a1e6 Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Fri, 13 Feb 2015 15:04:06 -0600 Subject: [PATCH 13/14] usb: isp1760: hcd: use new USB_RESUME_TIMEOUT Make sure we're using the new macro, so our resume signaling will always pass certification. Cc: # v3.10+ Signed-off-by: Felipe Balbi --- drivers/usb/isp1760/isp1760-hcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/isp1760/isp1760-hcd.c b/drivers/usb/isp1760/isp1760-hcd.c index 3cb98b1d5d29..7911b6b6fe40 100644 --- a/drivers/usb/isp1760/isp1760-hcd.c +++ b/drivers/usb/isp1760/isp1760-hcd.c @@ -1869,7 +1869,7 @@ static int isp1760_hub_control(struct usb_hcd *hcd, u16 typeReq, reg_write32(hcd->regs, HC_PORTSC1, temp | PORT_RESUME); priv->reset_done = jiffies + - msecs_to_jiffies(20); + msecs_to_jiffies(USB_RESUME_TIMEOUT); } break; case USB_PORT_FEAT_C_SUSPEND: From bbc78c07a51f6fd29c227b1220a9016e585358ba Mon Sep 17 00:00:00 2001 From: Felipe Balbi Date: Fri, 13 Feb 2015 15:38:33 -0600 Subject: [PATCH 14/14] usb: core: hub: use new USB_RESUME_TIMEOUT Make sure we're using the new macro, so our resume signaling will always pass certification. Cc: # v3.10+ Signed-off-by: Felipe Balbi --- drivers/usb/core/hub.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index d7c3d5a35946..3b7151687776 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -3406,10 +3406,10 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg) if (status) { dev_dbg(&port_dev->dev, "can't resume, status %d\n", status); } else { - /* drive resume for at least 20 msec */ + /* drive resume for USB_RESUME_TIMEOUT msec */ dev_dbg(&udev->dev, "usb %sresume\n", (PMSG_IS_AUTO(msg) ? "auto-" : "")); - msleep(25); + msleep(USB_RESUME_TIMEOUT); /* Virtual root hubs can trigger on GET_PORT_STATUS to * stop resume signaling. Then finish the resume