From c89686118c786a523eee3ef68ed38330319626d8 Mon Sep 17 00:00:00 2001 From: Kai-Heng Feng Date: Thu, 20 Jan 2022 15:05:16 +0800 Subject: usb: core: Bail out when port is stuck in reset loop Unplugging USB device may cause an incorrect warm reset loop and the port can no longer be used: [ 143.039019] xhci_hcd 0000:00:14.0: Port change event, 2-3, id 19, portsc: 0x4202c0 [ 143.039025] xhci_hcd 0000:00:14.0: handle_port_status: starting usb2 port polling. [ 143.039051] hub 2-0:1.0: state 7 ports 10 chg 0000 evt 0008 [ 143.039058] xhci_hcd 0000:00:14.0: Get port status 2-3 read: 0x4202c0, return 0x4102c0 [ 143.039092] xhci_hcd 0000:00:14.0: clear port3 connect change, portsc: 0x4002c0 [ 143.039096] usb usb2-port3: link state change [ 143.039099] xhci_hcd 0000:00:14.0: clear port3 link state change, portsc: 0x2c0 [ 143.039101] usb usb2-port3: do warm reset [ 143.096736] xhci_hcd 0000:00:14.0: Get port status 2-3 read: 0x2b0, return 0x2b0 [ 143.096751] usb usb2-port3: not warm reset yet, waiting 50ms [ 143.131500] xhci_hcd 0000:00:14.0: Can't queue urb, port error, link inactive [ 143.138260] xhci_hcd 0000:00:14.0: Port change event, 2-3, id 19, portsc: 0x2802a0 [ 143.138263] xhci_hcd 0000:00:14.0: handle_port_status: starting usb2 port polling. [ 143.160756] xhci_hcd 0000:00:14.0: Get port status 2-3 read: 0x2802a0, return 0x3002a0 [ 143.160798] usb usb2-port3: not warm reset yet, waiting 200ms The port status is PP=1, CCS=0, PED=0, PLS=Inactive, which is Error state per "USB3 Root Hub Port State Machine". It's reasonable to perform warm reset several times, but if the port is still not enabled after many attempts, consider it's gone and treat it as disconnected. Signed-off-by: Kai-Heng Feng Link: https://lore.kernel.org/r/20220120070518.1643873-1-kai.heng.feng@canonical.com 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 47a1c8bddf86..83b5aff25dd6 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2983,8 +2983,12 @@ static int hub_port_reset(struct usb_hub *hub, int port1, status); } - /* Check for disconnect or reset */ - if (status == 0 || status == -ENOTCONN || status == -ENODEV) { + /* + * Check for disconnect or reset, and bail out after several + * reset attempts to avoid warm reset loop. + */ + if (status == 0 || status == -ENOTCONN || status == -ENODEV || + (status == -EBUSY && i == PORT_RESET_TRIES - 1)) { usb_clear_port_feature(hub->hdev, port1, USB_PORT_FEAT_C_RESET); -- cgit v1.2.3 From 5d0c4393b207660fb9d6b08caed03ac85435fcd9 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Fri, 21 Jan 2022 12:33:30 -0600 Subject: USB: hcd-pci: Use PCI_STD_NUM_BARS when checking standard BARs usb_hcd_pci_probe() searches for an I/O BAR using a combination of PCI_STD_NUM_BARS (to control loop iteration) and PCI_ROM_RESOURCE (to check whether the loop exits without finding anything). Use PCI_STD_NUM_BARS consistently. No functional change since PCI_STD_NUM_BARS == PCI_ROM_RESOURCE, but this removes a dependency on that relationship and makes the code read better. Fixes: c9c13ba428ef ("PCI: Add PCI_STD_NUM_BARS for the number of standard BARs") Cc: Denis Efremov Signed-off-by: Bjorn Helgaas Link: https://lore.kernel.org/r/20220121183330.1141702-1-helgaas@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hcd-pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c index d630cccd2e6e..784466117c92 100644 --- a/drivers/usb/core/hcd-pci.c +++ b/drivers/usb/core/hcd-pci.c @@ -248,7 +248,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id, hcd->rsrc_len, driver->description)) break; } - if (region == PCI_ROM_RESOURCE) { + if (region == PCI_STD_NUM_BARS) { dev_dbg(&dev->dev, "no i/o regions available\n"); retval = -EBUSY; goto put_hcd; -- cgit v1.2.3 From c067688ae204ae4f0ad5044fe7345350fc6cd1b4 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 26 Jan 2022 20:53:38 +0100 Subject: USB: ACPI: Replace acpi_bus_get_device() Replace acpi_bus_get_device() that is going to be dropped with acpi_fetch_acpi_dev(). No intentional functional impact. Signed-off-by: Rafael J. Wysocki Link: https://lore.kernel.org/r/1899393.PYKUYFuaPT@kreacher Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/usb-acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c index 50b2fc7fcc0e..bb1da35eb891 100644 --- a/drivers/usb/core/usb-acpi.c +++ b/drivers/usb/core/usb-acpi.c @@ -166,7 +166,7 @@ usb_acpi_get_companion_for_port(struct usb_port *port_dev) if (!parent_handle) return NULL; - acpi_bus_get_device(parent_handle, &adev); + adev = acpi_fetch_acpi_dev(parent_handle); port1 = port_dev->portnum; } -- cgit v1.2.3 From 6a3cd5bef2531a1178234efa3bed788e3b3831f0 Mon Sep 17 00:00:00 2001 From: Ingo Rohloff Date: Wed, 9 Feb 2022 13:33:03 +0100 Subject: USB: usbfs: Use a spinlock instead of atomic accesses to tally used memory. While the existing code code imposes a limit on the used memory, it might be over pessimistic (even if this is unlikely). Example scenario: 8 threads running in parallel, all entering "usbfs_increase_memory_usage()" at the same time. The atomic accesses in "usbfs_increase_memory_usage()" could be serialized like this: 8 x "atomic64_add" 8 x "atomic64_read" If the 8 x "atomic64_add" raise "usbfs_memory_usage" above the limit, then all 8 calls of "usbfs_increase_memory_usage()" will return with -ENOMEM. If you instead serialize over the whole access to "usbfs_memory_usage" by using a spinlock, some of these calls will succeed. Acked-by: Alan Stern Signed-off-by: Ingo Rohloff Link: https://lore.kernel.org/r/20220209123303.103340-2-ingo.rohloff@lauterbach.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/devio.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index fa66e6e58792..6abb7294e919 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -139,30 +139,42 @@ MODULE_PARM_DESC(usbfs_memory_mb, /* Hard limit, necessary to avoid arithmetic overflow */ #define USBFS_XFER_MAX (UINT_MAX / 2 - 1000000) -static atomic64_t usbfs_memory_usage; /* Total memory currently allocated */ +static DEFINE_SPINLOCK(usbfs_memory_usage_lock); +static u64 usbfs_memory_usage; /* Total memory currently allocated */ /* Check whether it's okay to allocate more memory for a transfer */ static int usbfs_increase_memory_usage(u64 amount) { - u64 lim; + u64 lim, total_mem; + unsigned long flags; + int ret; lim = READ_ONCE(usbfs_memory_mb); lim <<= 20; - atomic64_add(amount, &usbfs_memory_usage); - - if (lim > 0 && atomic64_read(&usbfs_memory_usage) > lim) { - atomic64_sub(amount, &usbfs_memory_usage); - return -ENOMEM; - } + ret = 0; + spin_lock_irqsave(&usbfs_memory_usage_lock, flags); + total_mem = usbfs_memory_usage + amount; + if (lim > 0 && total_mem > lim) + ret = -ENOMEM; + else + usbfs_memory_usage = total_mem; + spin_unlock_irqrestore(&usbfs_memory_usage_lock, flags); - return 0; + return ret; } /* Memory for a transfer is being deallocated */ static void usbfs_decrease_memory_usage(u64 amount) { - atomic64_sub(amount, &usbfs_memory_usage); + unsigned long flags; + + spin_lock_irqsave(&usbfs_memory_usage_lock, flags); + if (amount > usbfs_memory_usage) + usbfs_memory_usage = 0; + else + usbfs_memory_usage -= amount; + spin_unlock_irqrestore(&usbfs_memory_usage_lock, flags); } static int connected(struct usb_dev_state *ps) -- cgit v1.2.3 From cd36facf104afbde7e8fa25cd6f5b6dd9fa97bb2 Mon Sep 17 00:00:00 2001 From: Mathias Nyman Date: Wed, 16 Feb 2022 11:51:51 +0200 Subject: usb: remove Link Powermanagement (LPM) disable before port reset. Trying to disable Link Powermanagement (LPM) before port reset is unnecessary and can cause additional delay if host can't communicate with the device, which is often the reason why device is reset in the first place. usb_disable_lpm() will - zero usb U1/U2 timeouts for the hub downstream port - send ENABLE U1/U2 clear feature requests to the connected device. - increase internal reference count for udev->lpm_disable_count There is no need to zero U1/U2 hub port timeouts, or clearing the U1/U2 enable for the connected device before reset. These are set to default by the reset. USB 3.1 section 10.2.2 "HUB Downstream port U1/U2 timers" states that: "the U1 and U2 timeout values for a downstream port reset to the default values when the port receives a SetPortFeature request for a port reset" Set the udev->lpm_disable_count to "1" after port reset, which is the default lpm_disable_count value when allocating udev, representing disabled LPM. Cc: Alan Stern Signed-off-by: Mathias Nyman Link: https://lore.kernel.org/r/20220216095153.1303105-8-mathias.nyman@linux.intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 83b5aff25dd6..1460857026e0 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -5009,6 +5009,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1, retval = usb_get_bos_descriptor(udev); if (!retval) { udev->lpm_capable = usb_device_supports_lpm(udev); + udev->lpm_disable_count = 1; usb_set_lpm_parameters(udev); } } @@ -5932,16 +5933,6 @@ static int usb_reset_and_verify_device(struct usb_device *udev) */ usb_disable_usb2_hardware_lpm(udev); - /* Disable LPM while we reset the device and reinstall the alt settings. - * Device-initiated LPM, and system exit latency settings are cleared - * when the device is reset, so we have to set them up again. - */ - ret = usb_unlocked_disable_lpm(udev); - if (ret) { - dev_err(&udev->dev, "%s Failed to disable LPM\n", __func__); - goto re_enumerate_no_bos; - } - bos = udev->bos; udev->bos = NULL; @@ -6046,8 +6037,6 @@ done: re_enumerate: usb_release_bos_descriptor(udev); udev->bos = bos; -re_enumerate_no_bos: - /* LPM state doesn't matter when we're about to destroy the device. */ hub_port_logical_disconnect(parent_hub, port1); return -ENODEV; } -- cgit v1.2.3 From f6a9a2d64dd168b7d71076c0e6b2be7db7cb7399 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Fri, 25 Feb 2022 09:38:25 -0500 Subject: USB: core: Update kerneldoc for usb_get_dev() and usb_get_intf() The kerneldoc for usb_get_dev() and usb_get_intf() says that drivers should always refcount the references they hold for the usb_device or usb_interface structure, respectively. But this is an overstatement: In many cases drivers do not access these references after they have been unbound, and in such cases refcounting is unnecessary. This patch updates the kerneldoc for the two routines, explaining when a driver does not need to increment and decrement the refcount. This should help dispel misconceptions which might otherwise afflict programmers new to the USB subsystem. Signed-off-by: Alan Stern Link: https://lore.kernel.org/r/Yhjp4Rp9Alipmwtq@rowland.harvard.edu Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/usb.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index 2ce3667ec6fa..2f71636af6e1 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -688,6 +688,10 @@ EXPORT_SYMBOL_GPL(usb_alloc_dev); * Drivers for USB interfaces should normally record such references in * their probe() methods, when they bind to an interface, and release * them by calling usb_put_dev(), in their disconnect() methods. + * However, if a driver does not access the usb_device structure after + * its disconnect() method returns then refcounting is not necessary, + * because the USB core guarantees that a usb_device will not be + * deallocated until after all of its interface drivers have been unbound. * * Return: A pointer to the device with the incremented reference counter. */ @@ -722,6 +726,10 @@ EXPORT_SYMBOL_GPL(usb_put_dev); * Drivers for USB interfaces should normally record such references in * their probe() methods, when they bind to an interface, and release * them by calling usb_put_intf(), in their disconnect() methods. + * However, if a driver does not access the usb_interface structure after + * its disconnect() method returns then refcounting is not necessary, + * because the USB core guarantees that a usb_interface will not be + * deallocated until after its driver has been unbound. * * Return: A pointer to the interface with the incremented reference counter. */ -- cgit v1.2.3