From 5efd2ea8c9f4f12916ffc8ba636792ce052f6911 Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Fri, 5 Dec 2014 15:13:54 +0100 Subject: usb: core: buffer: smallest buffer should start at ARCH_DMA_MINALIGN the following error pops up during "testusb -a -t 10" | musb-hdrc musb-hdrc.1.auto: dma_pool_free buffer-128, f134e000/be842000 (bad dma) hcd_buffer_create() creates a few buffers, the smallest has 32 bytes of size. ARCH_KMALLOC_MINALIGN is set to 64 bytes. This combo results in hcd_buffer_alloc() returning memory which is 32 bytes aligned and it might by identified by buffer_offset() as another buffer. This means the buffer which is on a 32 byte boundary will not get freed, instead it tries to free another buffer with the error message. This patch fixes the issue by creating the smallest DMA buffer with the size of ARCH_KMALLOC_MINALIGN (or 32 in case ARCH_KMALLOC_MINALIGN is smaller). This might be 32, 64 or even 128 bytes. The next three pools will have the size 128, 512 and 2048. In case the smallest pool is 128 bytes then we have only three pools instead of four (and zero the first entry in the array). The last pool size is always 2048 bytes which is the assumed PAGE_SIZE / 2 of 4096. I doubt it makes sense to continue using PAGE_SIZE / 2 where we would end up with 8KiB buffer in case we have 16KiB pages. Instead I think it makes sense to have a common size(s) and extend them if there is need to. There is a BUILD_BUG_ON() now in case someone has a minalign of more than 128 bytes. Cc: stable@vger.kernel.org Signed-off-by: Sebastian Andrzej Siewior Acked-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/buffer.c | 26 +++++++++++++++++--------- drivers/usb/core/usb.c | 1 + 2 files changed, 18 insertions(+), 9 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c index 684ef70..506b969 100644 --- a/drivers/usb/core/buffer.c +++ b/drivers/usb/core/buffer.c @@ -22,17 +22,25 @@ */ /* FIXME tune these based on pool statistics ... */ -static const size_t pool_max[HCD_BUFFER_POOLS] = { - /* platforms without dma-friendly caches might need to - * prevent cacheline sharing... - */ - 32, - 128, - 512, - PAGE_SIZE / 2 - /* bigger --> allocate pages */ +static size_t pool_max[HCD_BUFFER_POOLS] = { + 32, 128, 512, 2048, }; +void __init usb_init_pool_max(void) +{ + /* + * The pool_max values must never be smaller than + * ARCH_KMALLOC_MINALIGN. + */ + if (ARCH_KMALLOC_MINALIGN <= 32) + ; /* Original value is okay */ + else if (ARCH_KMALLOC_MINALIGN <= 64) + pool_max[0] = 64; + else if (ARCH_KMALLOC_MINALIGN <= 128) + pool_max[0] = 0; /* Don't use this pool */ + else + BUILD_BUG(); /* We don't allow this */ +} /* SETUP primitives */ diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index 2a92b97..b1fb9ae 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -1049,6 +1049,7 @@ static int __init usb_init(void) pr_info("%s: USB support disabled\n", usbcore_name); return 0; } + usb_init_pool_max(); retval = usb_debugfs_init(); if (retval) -- cgit v1.1 From 524134d422316a59d5464ccbc12036bbe90c5563 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Wed, 21 Jan 2015 14:02:43 -0500 Subject: USB: don't cancel queued resets when unbinding drivers The USB stack provides a mechanism for drivers to request an asynchronous device reset (usb_queue_reset_device()). The mechanism uses a work item (reset_ws) embedded in the usb_interface structure used by the driver, and the reset is carried out by a work queue routine. The asynchronous reset can race with driver unbinding. When this happens, we try to cancel the queued reset before unbinding the driver, on the theory that the driver won't care about any resets once it is unbound. However, thanks to the fact that lockdep now tracks work queue accesses, this can provoke a lockdep warning in situations where the device reset causes another interface's driver to be unbound; see http://marc.info/?l=linux-usb&m=141893165203776&w=2 for an example. The reason is that the work routine for reset_ws in one interface calls cancel_queued_work() for the reset_ws in another interface. Lockdep thinks this might lead to a work routine trying to cancel itself. The simplest solution is not to cancel queued resets when unbinding drivers. This means we now need to acquire a reference to the usb_interface when queuing a reset_ws work item and to drop the reference when the work routine finishes. We also need to make sure that the usb_interface structure doesn't outlive its parent usb_device; this means acquiring and dropping a reference when the interface is created and destroyed. In addition, cancelling a queued reset can fail (if the device is in the middle of an earlier reset), and this can cause usb_reset_device() to try to rebind an interface that has been deallocated (see http://marc.info/?l=linux-usb&m=142175717016628&w=2 for details). Acquiring the extra references prevents this failure. Signed-off-by: Alan Stern Reported-by: Russell King - ARM Linux Reported-by: Olivier Sobrie Tested-by: Olivier Sobrie Cc: stable # 3.19 Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/driver.c | 17 ----------------- drivers/usb/core/hub.c | 25 +++++++++---------------- drivers/usb/core/message.c | 23 +++-------------------- 3 files changed, 12 insertions(+), 53 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 874dec3..c76ec97 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -275,21 +275,6 @@ static int usb_unbind_device(struct device *dev) return 0; } -/* - * Cancel any pending scheduled resets - * - * [see usb_queue_reset_device()] - * - * Called after unconfiguring / when releasing interfaces. See - * comments in __usb_queue_reset_device() regarding - * udev->reset_running. - */ -static void usb_cancel_queued_reset(struct usb_interface *iface) -{ - if (iface->reset_running == 0) - cancel_work_sync(&iface->reset_ws); -} - /* called from driver core with dev locked */ static int usb_probe_interface(struct device *dev) { @@ -380,7 +365,6 @@ static int usb_probe_interface(struct device *dev) usb_set_intfdata(intf, NULL); intf->needs_remote_wakeup = 0; intf->condition = USB_INTERFACE_UNBOUND; - usb_cancel_queued_reset(intf); /* If the LPM disable succeeded, balance the ref counts. */ if (!lpm_disable_error) @@ -425,7 +409,6 @@ static int usb_unbind_interface(struct device *dev) usb_disable_interface(udev, intf, false); driver->disconnect(intf); - usb_cancel_queued_reset(intf); /* Free streams */ for (i = 0, j = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) { diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index aeb50bb..b4bfa3a 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -5589,26 +5589,19 @@ EXPORT_SYMBOL_GPL(usb_reset_device); * possible; depending on how the driver attached to each interface * handles ->pre_reset(), the second reset might happen or not. * - * - If a driver is unbound and it had a pending reset, the reset will - * be cancelled. + * - If the reset is delayed so long that the interface is unbound from + * its driver, the reset will be skipped. * - * - This function can be called during .probe() or .disconnect() - * times. On return from .disconnect(), any pending resets will be - * cancelled. - * - * There is no no need to lock/unlock the @reset_ws as schedule_work() - * does its own. - * - * NOTE: We don't do any reference count tracking because it is not - * needed. The lifecycle of the work_struct is tied to the - * usb_interface. Before destroying the interface we cancel the - * work_struct, so the fact that work_struct is queued and or - * running means the interface (and thus, the device) exist and - * are referenced. + * - This function can be called during .probe(). It can also be called + * during .disconnect(), but doing so is pointless because the reset + * will not occur. If you really want to reset the device during + * .disconnect(), call usb_reset_device() directly -- but watch out + * for nested unbinding issues! */ void usb_queue_reset_device(struct usb_interface *iface) { - schedule_work(&iface->reset_ws); + if (schedule_work(&iface->reset_ws)) + usb_get_intf(iface); } EXPORT_SYMBOL_GPL(usb_queue_reset_device); diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index f7b7713..f368d20 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1551,6 +1551,7 @@ static void usb_release_interface(struct device *dev) altsetting_to_usb_interface_cache(intf->altsetting); kref_put(&intfc->ref, usb_release_interface_cache); + usb_put_dev(interface_to_usbdev(intf)); kfree(intf); } @@ -1626,24 +1627,6 @@ static struct usb_interface_assoc_descriptor *find_iad(struct usb_device *dev, /* * Internal function to queue a device reset - * - * This is initialized into the workstruct in 'struct - * usb_device->reset_ws' that is launched by - * message.c:usb_set_configuration() when initializing each 'struct - * usb_interface'. - * - * It is safe to get the USB device without reference counts because - * the life cycle of @iface is bound to the life cycle of @udev. Then, - * this function will be ran only if @iface is alive (and before - * freeing it any scheduled instances of it will have been cancelled). - * - * We need to set a flag (usb_dev->reset_running) because when we call - * the reset, the interfaces might be unbound. The current interface - * cannot try to remove the queued work as it would cause a deadlock - * (you cannot remove your work from within your executing - * workqueue). This flag lets it know, so that - * usb_cancel_queued_reset() doesn't try to do it. - * * See usb_queue_reset_device() for more details */ static void __usb_queue_reset_device(struct work_struct *ws) @@ -1655,11 +1638,10 @@ static void __usb_queue_reset_device(struct work_struct *ws) rc = usb_lock_device_for_reset(udev, iface); if (rc >= 0) { - iface->reset_running = 1; usb_reset_device(udev); - iface->reset_running = 0; usb_unlock_device(udev); } + usb_put_intf(iface); /* Undo _get_ in usb_queue_reset_device() */ } @@ -1854,6 +1836,7 @@ free_interfaces: dev_set_name(&intf->dev, "%d-%s:%d.%d", dev->bus->busnum, dev->devpath, configuration, alt->desc.bInterfaceNumber); + usb_get_dev(dev); } kfree(new_interfaces); -- cgit v1.1 From fbaecff06a7db4defa899a664fe2758e5161b39d Mon Sep 17 00:00:00 2001 From: Deepak Das Date: Wed, 21 Jan 2015 23:39:58 +0530 Subject: usb: core: hub: modify hub reset logic in hub driver Currently if port power is turned off by user on hub port using USBDEVFS then port power is turned back ON by hub driver. This commit modifies hub reset logic in hub_port_connect() to prevent hub driver from turning back the port power ON if port is not owned by kernel. Signed-off-by: Deepak Das Acked-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index b4bfa3a..3e9c4d4 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -4655,9 +4655,13 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus, if (!(portstatus & USB_PORT_STAT_CONNECTION) || test_bit(port1, hub->removed_bits)) { - /* maybe switch power back on (e.g. root hub was reset) */ + /* + * maybe switch power back on (e.g. root hub was reset) + * but only if the port isn't owned by someone else. + */ if (hub_is_port_power_switchable(hub) - && !port_is_power_on(hub, portstatus)) + && !port_is_power_on(hub, portstatus) + && !port_dev->port_owner) set_port_feature(hdev, port1, USB_PORT_FEAT_POWER); if (portstatus & USB_PORT_STAT_ENABLE) -- cgit v1.1 From 3f2cee73b650921b2e214bf487b2061a1c266504 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Thu, 29 Jan 2015 11:29:13 -0500 Subject: USB: usbfs: allow URBs to be reaped after disconnection The usbfs API has a peculiar hole: Users are not allowed to reap their URBs after the device has been disconnected. There doesn't seem to be any good reason for this; it is an ad-hoc inconsistency. The patch allows users to issue the USBDEVFS_REAPURB and USBDEVFS_REAPURBNDELAY ioctls (together with their 32-bit counterparts on 64-bit systems) even after the device is gone. If no URBs are pending for a disconnected device then the ioctls will return -ENODEV rather than -EAGAIN, because obviously no new URBs will ever be able to complete. The patch also adds a new capability flag for USBDEVFS_GET_CAPABILITIES to indicate that the reap-after-disconnect feature is supported. Signed-off-by: Alan Stern Tested-by: Chris Dickens Acked-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/devio.c | 63 ++++++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 26 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 0b59731..66abdbc 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1689,7 +1689,7 @@ static struct async *reap_as(struct usb_dev_state *ps) for (;;) { __set_current_state(TASK_INTERRUPTIBLE); as = async_getcompleted(ps); - if (as) + if (as || !connected(ps)) break; if (signal_pending(current)) break; @@ -1712,7 +1712,7 @@ static int proc_reapurb(struct usb_dev_state *ps, void __user *arg) } if (signal_pending(current)) return -EINTR; - return -EIO; + return -ENODEV; } static int proc_reapurbnonblock(struct usb_dev_state *ps, void __user *arg) @@ -1721,10 +1721,11 @@ static int proc_reapurbnonblock(struct usb_dev_state *ps, void __user *arg) struct async *as; as = async_getcompleted(ps); - retval = -EAGAIN; if (as) { retval = processcompl(as, (void __user * __user *)arg); free_async(as); + } else { + retval = (connected(ps) ? -EAGAIN : -ENODEV); } return retval; } @@ -1854,7 +1855,7 @@ static int proc_reapurb_compat(struct usb_dev_state *ps, void __user *arg) } if (signal_pending(current)) return -EINTR; - return -EIO; + return -ENODEV; } static int proc_reapurbnonblock_compat(struct usb_dev_state *ps, void __user *arg) @@ -1862,11 +1863,12 @@ static int proc_reapurbnonblock_compat(struct usb_dev_state *ps, void __user *ar int retval; struct async *as; - retval = -EAGAIN; as = async_getcompleted(ps); if (as) { retval = processcompl_compat(as, (void __user * __user *)arg); free_async(as); + } else { + retval = (connected(ps) ? -EAGAIN : -ENODEV); } return retval; } @@ -2038,7 +2040,8 @@ static int proc_get_capabilities(struct usb_dev_state *ps, void __user *arg) { __u32 caps; - caps = USBDEVFS_CAP_ZERO_PACKET | USBDEVFS_CAP_NO_PACKET_SIZE_LIM; + caps = USBDEVFS_CAP_ZERO_PACKET | USBDEVFS_CAP_NO_PACKET_SIZE_LIM | + USBDEVFS_CAP_REAP_AFTER_DISCONNECT; if (!ps->dev->bus->no_stop_on_short) caps |= USBDEVFS_CAP_BULK_CONTINUATION; if (ps->dev->bus->sg_tablesize) @@ -2138,6 +2141,32 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd, return -EPERM; usb_lock_device(dev); + + /* Reap operations are allowed even after disconnection */ + switch (cmd) { + case USBDEVFS_REAPURB: + snoop(&dev->dev, "%s: REAPURB\n", __func__); + ret = proc_reapurb(ps, p); + goto done; + + case USBDEVFS_REAPURBNDELAY: + snoop(&dev->dev, "%s: REAPURBNDELAY\n", __func__); + ret = proc_reapurbnonblock(ps, p); + goto done; + +#ifdef CONFIG_COMPAT + case USBDEVFS_REAPURB32: + snoop(&dev->dev, "%s: REAPURB32\n", __func__); + ret = proc_reapurb_compat(ps, p); + goto done; + + case USBDEVFS_REAPURBNDELAY32: + snoop(&dev->dev, "%s: REAPURBNDELAY32\n", __func__); + ret = proc_reapurbnonblock_compat(ps, p); + goto done; +#endif + } + if (!connected(ps)) { usb_unlock_device(dev); return -ENODEV; @@ -2231,16 +2260,6 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd, inode->i_mtime = CURRENT_TIME; break; - case USBDEVFS_REAPURB32: - snoop(&dev->dev, "%s: REAPURB32\n", __func__); - ret = proc_reapurb_compat(ps, p); - break; - - case USBDEVFS_REAPURBNDELAY32: - snoop(&dev->dev, "%s: REAPURBNDELAY32\n", __func__); - ret = proc_reapurbnonblock_compat(ps, p); - break; - case USBDEVFS_IOCTL32: snoop(&dev->dev, "%s: IOCTL32\n", __func__); ret = proc_ioctl_compat(ps, ptr_to_compat(p)); @@ -2252,16 +2271,6 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd, ret = proc_unlinkurb(ps, p); break; - case USBDEVFS_REAPURB: - snoop(&dev->dev, "%s: REAPURB\n", __func__); - ret = proc_reapurb(ps, p); - break; - - case USBDEVFS_REAPURBNDELAY: - snoop(&dev->dev, "%s: REAPURBNDELAY\n", __func__); - ret = proc_reapurbnonblock(ps, p); - break; - case USBDEVFS_DISCSIGNAL: snoop(&dev->dev, "%s: DISCSIGNAL\n", __func__); ret = proc_disconnectsignal(ps, p); @@ -2304,6 +2313,8 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd, ret = proc_free_streams(ps, p); break; } + + done: usb_unlock_device(dev); if (ret >= 0) inode->i_atime = CURRENT_TIME; -- cgit v1.1 From 074f9dd55f9cab1b82690ed7e44bcf38b9616ce0 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Thu, 29 Jan 2015 15:05:04 -0500 Subject: USB: add flag for HCDs that can't receive wakeup requests (isp1760-hcd) Currently the USB stack assumes that all host controller drivers are capable of receiving wakeup requests from downstream devices. However, this isn't true for the isp1760-hcd driver, which means that it isn't safe to do a runtime suspend of any device attached to a root-hub port if the device requires wakeup. This patch adds a "cant_recv_wakeups" flag to the usb_hcd structure and sets the flag in isp1760-hcd. The core is modified to prevent a direct child of the root hub from being put into runtime suspend with wakeup enabled if the flag is set. Signed-off-by: Alan Stern Tested-by: Nicolas Pitre CC: Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/driver.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index c76ec97..818369a 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -1780,6 +1780,18 @@ static int autosuspend_check(struct usb_device *udev) dev_dbg(&udev->dev, "remote wakeup needed for autosuspend\n"); return -EOPNOTSUPP; } + + /* + * If the device is a direct child of the root hub and the HCD + * doesn't handle wakeup requests, don't allow autosuspend when + * wakeup is needed. + */ + if (w && udev->parent == udev->bus->root_hub && + bus_to_hcd(udev->bus)->cant_recv_wakeups) { + dev_dbg(&udev->dev, "HCD doesn't handle wakeup requests\n"); + return -EOPNOTSUPP; + } + udev->do_remote_wakeup = w; return 0; } -- cgit v1.1 From c99197902da284b4b723451c1471c45b18537cde Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Fri, 30 Jan 2015 12:58:26 -0500 Subject: USB: fix use-after-free bug in usb_hcd_unlink_urb() The usb_hcd_unlink_urb() routine in hcd.c contains two possible use-after-free errors. The dev_dbg() statement at the end of the routine dereferences urb and urb->dev even though both structures may have been deallocated. This patch fixes the problem by storing urb->dev in a local variable (avoiding the dereference of urb) and moving the dev_dbg() up before the usb_put_dev() call. Signed-off-by: Alan Stern Reported-by: Joe Lawrence Tested-by: Joe Lawrence CC: Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hcd.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 11cee55..45a915c 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1618,6 +1618,7 @@ static int unlink1(struct usb_hcd *hcd, struct urb *urb, int status) int usb_hcd_unlink_urb (struct urb *urb, int status) { struct usb_hcd *hcd; + struct usb_device *udev = urb->dev; int retval = -EIDRM; unsigned long flags; @@ -1629,20 +1630,19 @@ int usb_hcd_unlink_urb (struct urb *urb, int status) spin_lock_irqsave(&hcd_urb_unlink_lock, flags); if (atomic_read(&urb->use_count) > 0) { retval = 0; - usb_get_dev(urb->dev); + usb_get_dev(udev); } spin_unlock_irqrestore(&hcd_urb_unlink_lock, flags); if (retval == 0) { hcd = bus_to_hcd(urb->dev->bus); retval = unlink1(hcd, urb, status); - usb_put_dev(urb->dev); + if (retval == 0) + retval = -EINPROGRESS; + else if (retval != -EIDRM && retval != -EBUSY) + dev_dbg(&udev->dev, "hcd_unlink_urb %p fail %d\n", + urb, retval); + usb_put_dev(udev); } - - if (retval == 0) - retval = -EINPROGRESS; - else if (retval != -EIDRM && retval != -EBUSY) - dev_dbg(&urb->dev->dev, "hcd_unlink_urb %p fail %d\n", - urb, retval); return retval; } -- cgit v1.1 From 7671bd1e97b9fa09aea69e76375ada9534c735a3 Mon Sep 17 00:00:00 2001 From: Zhuang Jin Can Date: Mon, 26 Jan 2015 23:30:39 +0800 Subject: Revert "usb: Reset USB-3 devices on USB-3 link bounce" This revert a82b76f7fa6154e8ab2d8071842a3e38b9c0d0ff. The commit causes an extra reset in remote wakeup as described in: http://www.spinics.net/lists/linux-usb/msg119080.html Signed-off-by: Zhuang Jin Can Acked-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 34 +++++++++------------------------- 1 file changed, 9 insertions(+), 25 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 3e9c4d4..4c259ac 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -4886,7 +4886,7 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1, static void port_event(struct usb_hub *hub, int port1) __must_hold(&port_dev->status_lock) { - int connect_change, reset_device = 0; + int connect_change; struct usb_port *port_dev = hub->ports[port1 - 1]; struct usb_device *udev = port_dev->child; struct usb_device *hdev = hub->hdev; @@ -4974,30 +4974,14 @@ static void port_event(struct usb_hub *hub, int port1) if (hub_port_reset(hub, port1, NULL, HUB_BH_RESET_TIME, true) < 0) hub_port_disable(hub, port1, 1); - } else - reset_device = 1; - } - - /* - * On disconnect USB3 protocol ports transit from U0 to - * SS.Inactive to Rx.Detect. If this happens a warm- - * reset is not needed, but a (re)connect may happen - * before hub_wq runs and sees the disconnect, and the - * device may be an unknown state. - * - * If the port went through SS.Inactive without hub_wq - * seeing it the C_LINK_STATE change flag will be set, - * and we reset the dev to put it in a known state. - */ - if (reset_device || (udev && hub_is_superspeed(hub->hdev) - && (portchange & USB_PORT_STAT_C_LINK_STATE) - && (portstatus & USB_PORT_STAT_CONNECTION))) { - usb_unlock_port(port_dev); - usb_lock_device(udev); - usb_reset_device(udev); - usb_unlock_device(udev); - usb_lock_port(port_dev); - connect_change = 0; + } else { + usb_unlock_port(port_dev); + usb_lock_device(udev); + usb_reset_device(udev); + usb_unlock_device(udev); + usb_lock_port(port_dev); + connect_change = 0; + } } if (connect_change) -- cgit v1.1 From 7fa40910e0bf5ef32eca49595d950cb24f6402bf Mon Sep 17 00:00:00 2001 From: Julius Werner Date: Tue, 27 Jan 2015 13:20:12 -0800 Subject: usb: Retry port status check on resume to work around RH bugs The EHCI controller on the RK3288 SoC is violating basic parts of the USB spec and thereby unable to properly resume a suspended port. It does not start SOF generation within 3ms of finishing resume signaling, so the attached device will drop of the bus again. This is a particular problem with runtime PM, where accessing the device will trigger a resume that immediately makes it unavailable (and reenumerate with a new handle). Thankfully, the persist feature is generally able to work around stuff like that. Unfortunately, it doesn't quite work in this particular case because the controller will turn off the CurrentConnectStatus bit for an instant while the device is reconnecting, which causes the kernel to conclude that it permanently disappeared. This patch adds a tiny retry mechanism to the core port resume code which will catch this case and shouldn't have any notable impact on other controllers. Signed-off-by: Julius Werner Acked-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 4c259ac..58d2cde 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2896,10 +2896,12 @@ static int port_is_suspended(struct usb_hub *hub, unsigned portstatus) */ static int check_port_resume_type(struct usb_device *udev, struct usb_hub *hub, int port1, - int status, unsigned portchange, unsigned portstatus) + int status, u16 portchange, u16 portstatus) { struct usb_port *port_dev = hub->ports[port1 - 1]; + int retries = 3; + retry: /* Is a warm reset needed to recover the connection? */ if (status == 0 && udev->reset_resume && hub_port_warm_reset_required(hub, port1, portstatus)) { @@ -2907,10 +2909,17 @@ static int check_port_resume_type(struct usb_device *udev, } /* Is the device still present? */ else if (status || port_is_suspended(hub, portstatus) || - !port_is_power_on(hub, portstatus) || - !(portstatus & USB_PORT_STAT_CONNECTION)) { + !port_is_power_on(hub, portstatus)) { if (status >= 0) status = -ENODEV; + } else if (!(portstatus & USB_PORT_STAT_CONNECTION)) { + if (retries--) { + usleep_range(200, 300); + status = hub_port_status(hub, port1, &portstatus, + &portchange); + goto retry; + } + status = -ENODEV; } /* Can't do a normal resume if the port isn't enabled, -- cgit v1.1