From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org,
Sarah Sharp <sarah.a.sharp@linux.intel.com>,
Alan Stern <stern@rowland.harvard.edu>
Subject: [ 39/40] USB: Rip out recursive call on warm port reset.
Date: Tue, 12 Mar 2013 15:44:00 -0700 [thread overview]
Message-ID: <20130312223215.664489899@linuxfoundation.org> (raw)
In-Reply-To: <20130312223211.492954675@linuxfoundation.org>
3.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Sarah Sharp <sarah.a.sharp@linux.intel.com>
[This is upstream commit 24a6078754f28528bc91e7e7b3e6ae86bd936d8.
It needs to be backported to kernels as old as 3.2, because it fixes the
buggy commit 9dbcaec830cd97f44a0b91b315844e0d7144746b "USB: Handle warm
reset failure on empty port."]
When a hot reset fails on a USB 3.0 port, the current port reset code
recursively calls hub_port_reset inside hub_port_wait_reset. This isn't
ideal, since we should avoid recursive calls in the kernel, and it also
doesn't allow us to issue multiple warm resets on reset failures.
Rip out the recursive call. Instead, add code to hub_port_reset to
issue a warm reset if the hot reset fails, and try multiple warm resets
before giving up on the port.
In hub_port_wait_reset, remove the recursive call and re-indent. The
code is basically the same, except:
1. It bails out early if the port has transitioned to Inactive or
Compliance Mode after the reset completed.
2. It doesn't consider a connect status change to be a failed reset. If
multiple warm resets needed to be issued, the connect status may have
changed, so we need to ignore that and look at the port link state
instead. hub_port_reset will now do that.
3. It unconditionally sets udev->speed on all types of successful
resets. The old recursive code would set the port speed when the second
hub_port_reset returned.
The old code did not handle connected devices needing a warm reset well.
There were only two situations that the old code handled correctly: an
empty port needing a warm reset, and a hot reset that migrated to a warm
reset.
When an empty port needed a warm reset, hub_port_reset was called with
the warm variable set. The code in hub_port_finish_reset would skip
telling the USB core and the xHC host that the device was reset, because
otherwise that would result in a NULL pointer dereference.
When a USB 3.0 device reset migrated to a warm reset, the recursive call
made the call stack look like this:
hub_port_reset(warm = false)
hub_wait_port_reset(warm = false)
hub_port_reset(warm = true)
hub_wait_port_reset(warm = true)
hub_port_finish_reset(warm = true)
(return up the call stack to the first wait)
hub_port_finish_reset(warm = false)
The old code didn't want to notify the USB core or the xHC host of device reset
twice, so it only did it in the second call to hub_port_finish_reset,
when warm was set to false. This was necessary because
before patch two ("USB: Ignore xHCI Reset Device status."), the USB core
would pay attention to the xHC Reset Device command error status, and
the second call would always fail.
Now that we no longer have the recursive call, and warm can change from
false to true in hub_port_reset, we need to have hub_port_finish_reset
unconditionally notify the USB core and the xHC of the device reset.
In hub_port_finish_reset, unconditionally clear the connect status
change (CSC) bit for USB 3.0 hubs when the port reset is done. If we
had to issue multiple warm resets for a device, that bit may have been
set if the device went into SS.Inactive and then was successfully warm
reset.
Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/usb/core/hub.c | 148 ++++++++++++++++++++++---------------------------
1 file changed, 67 insertions(+), 81 deletions(-)
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2207,73 +2207,35 @@ static int hub_port_wait_reset(struct us
if ((portstatus & USB_PORT_STAT_RESET))
goto delay;
- /*
- * Some buggy devices require a warm reset to be issued even
- * when the port appears not to be connected.
+ if (hub_port_warm_reset_required(hub, portstatus))
+ return -ENOTCONN;
+
+ /* Device went away? */
+ if (!(portstatus & USB_PORT_STAT_CONNECTION))
+ return -ENOTCONN;
+
+ /* bomb out completely if the connection bounced. A USB 3.0
+ * connection may bounce if multiple warm resets were issued,
+ * but the device may have successfully re-connected. Ignore it.
*/
- if (!warm) {
- /*
- * Some buggy devices can cause an NEC host controller
- * to transition to the "Error" state after a hot port
- * reset. This will show up as the port state in
- * "Inactive", and the port may also report a
- * disconnect. Forcing a warm port reset seems to make
- * the device work.
- *
- * See https://bugzilla.kernel.org/show_bug.cgi?id=41752
- */
- if (hub_port_warm_reset_required(hub, portstatus)) {
- int ret;
+ if (!hub_is_superspeed(hub->hdev) &&
+ (portchange & USB_PORT_STAT_C_CONNECTION))
+ return -ENOTCONN;
- if ((portchange & USB_PORT_STAT_C_CONNECTION))
- clear_port_feature(hub->hdev, port1,
- USB_PORT_FEAT_C_CONNECTION);
- if (portchange & USB_PORT_STAT_C_LINK_STATE)
- clear_port_feature(hub->hdev, port1,
- USB_PORT_FEAT_C_PORT_LINK_STATE);
- if (portchange & USB_PORT_STAT_C_RESET)
- clear_port_feature(hub->hdev, port1,
- USB_PORT_FEAT_C_RESET);
- dev_dbg(hub->intfdev, "hot reset failed, warm reset port %d\n",
- port1);
- ret = hub_port_reset(hub, port1,
- udev, HUB_BH_RESET_TIME,
- true);
- if ((portchange & USB_PORT_STAT_C_CONNECTION))
- clear_port_feature(hub->hdev, port1,
- USB_PORT_FEAT_C_CONNECTION);
- return ret;
- }
- /* Device went away? */
- if (!(portstatus & USB_PORT_STAT_CONNECTION))
- return -ENOTCONN;
-
- /* bomb out completely if the connection bounced */
- if ((portchange & USB_PORT_STAT_C_CONNECTION))
- return -ENOTCONN;
-
- if ((portstatus & USB_PORT_STAT_ENABLE)) {
- if (!udev)
- return 0;
-
- if (hub_is_wusb(hub))
- udev->speed = USB_SPEED_WIRELESS;
- else if (hub_is_superspeed(hub->hdev))
- udev->speed = USB_SPEED_SUPER;
- else if (portstatus & USB_PORT_STAT_HIGH_SPEED)
- udev->speed = USB_SPEED_HIGH;
- else if (portstatus & USB_PORT_STAT_LOW_SPEED)
- udev->speed = USB_SPEED_LOW;
- else
- udev->speed = USB_SPEED_FULL;
+ if ((portstatus & USB_PORT_STAT_ENABLE)) {
+ if (!udev)
return 0;
- }
- } else {
- if (!(portstatus & USB_PORT_STAT_CONNECTION) ||
- hub_port_warm_reset_required(hub,
- portstatus))
- return -ENOTCONN;
+ if (hub_is_wusb(hub))
+ udev->speed = USB_SPEED_WIRELESS;
+ else if (hub_is_superspeed(hub->hdev))
+ udev->speed = USB_SPEED_SUPER;
+ else if (portstatus & USB_PORT_STAT_HIGH_SPEED)
+ udev->speed = USB_SPEED_HIGH;
+ else if (portstatus & USB_PORT_STAT_LOW_SPEED)
+ udev->speed = USB_SPEED_LOW;
+ else
+ udev->speed = USB_SPEED_FULL;
return 0;
}
@@ -2291,23 +2253,21 @@ delay:
}
static void hub_port_finish_reset(struct usb_hub *hub, int port1,
- struct usb_device *udev, int *status, bool warm)
+ struct usb_device *udev, int *status)
{
switch (*status) {
case 0:
- if (!warm) {
- struct usb_hcd *hcd;
- /* TRSTRCY = 10 ms; plus some extra */
- msleep(10 + 40);
- if (udev) {
- update_devnum(udev, 0);
- hcd = bus_to_hcd(udev->bus);
- /* The xHC may think the device is already
- * reset, so ignore the status.
- */
- if (hcd->driver->reset_device)
- hcd->driver->reset_device(hcd, udev);
- }
+ /* TRSTRCY = 10 ms; plus some extra */
+ msleep(10 + 40);
+ if (udev) {
+ struct usb_hcd *hcd = bus_to_hcd(udev->bus);
+
+ update_devnum(udev, 0);
+ /* The xHC may think the device is already reset,
+ * so ignore the status.
+ */
+ if (hcd->driver->reset_device)
+ hcd->driver->reset_device(hcd, udev);
}
/* FALL THROUGH */
case -ENOTCONN:
@@ -2320,8 +2280,10 @@ static void hub_port_finish_reset(struct
USB_PORT_FEAT_C_BH_PORT_RESET);
clear_port_feature(hub->hdev, port1,
USB_PORT_FEAT_C_PORT_LINK_STATE);
+ clear_port_feature(hub->hdev, port1,
+ USB_PORT_FEAT_C_CONNECTION);
}
- if (!warm && udev)
+ if (udev)
usb_set_device_state(udev, *status
? USB_STATE_NOTATTACHED
: USB_STATE_DEFAULT);
@@ -2334,6 +2296,7 @@ static int hub_port_reset(struct usb_hub
struct usb_device *udev, unsigned int delay, bool warm)
{
int i, status;
+ u16 portchange, portstatus;
if (!hub_is_superspeed(hub->hdev)) {
if (warm) {
@@ -2365,10 +2328,33 @@ static int hub_port_reset(struct usb_hub
status);
}
- /* return on disconnect or reset */
+ /* Check for disconnect or reset */
if (status == 0 || status == -ENOTCONN || status == -ENODEV) {
- hub_port_finish_reset(hub, port1, udev, &status, warm);
- goto done;
+ hub_port_finish_reset(hub, port1, udev, &status);
+
+ if (!hub_is_superspeed(hub->hdev))
+ goto done;
+
+ /*
+ * If a USB 3.0 device migrates from reset to an error
+ * state, re-issue the warm reset.
+ */
+ if (hub_port_status(hub, port1,
+ &portstatus, &portchange) < 0)
+ goto done;
+
+ if (!hub_port_warm_reset_required(hub, portstatus))
+ goto done;
+
+ /*
+ * If the port is in SS.Inactive or Compliance Mode, the
+ * hot or warm reset failed. Try another warm reset.
+ */
+ if (!warm) {
+ dev_dbg(hub->intfdev, "hot reset failed, warm reset port %d\n",
+ port1);
+ warm = true;
+ }
}
dev_dbg (hub->intfdev,
next prev parent reply other threads:[~2013-03-12 22:44 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-12 22:43 [ 00/40] 3.4.36-stable review Greg Kroah-Hartman
2013-03-12 22:43 ` [ 01/40] ARM: VFP: fix emulation of second VFP instruction Greg Kroah-Hartman
2013-03-12 22:43 ` [ 02/40] ARM: fix scheduling while atomic warning in alignment handling code Greg Kroah-Hartman
2013-03-12 22:43 ` [ 03/40] xen/pci: We dont do multiple MSIs Greg Kroah-Hartman
2013-03-12 22:43 ` [ 04/40] SCSI: dc395x: uninitialized variable in device_alloc() Greg Kroah-Hartman
2013-03-12 22:43 ` [ 05/40] SCSI: storvsc: Initialize the sglist Greg Kroah-Hartman
2013-03-12 22:43 ` [ 06/40] target/pscsi: Fix page increment Greg Kroah-Hartman
2013-03-12 22:43 ` [ 07/40] btrfs: Init io_lock after cloning btrfs device struct Greg Kroah-Hartman
2013-03-12 22:43 ` [ 08/40] cifs: ensure that cifs_get_root() only traverses directories Greg Kroah-Hartman
2013-03-12 22:43 ` [ 09/40] NFS: Dont allow NFS silly-renamed files to be deleted, no signal Greg Kroah-Hartman
2013-03-12 22:43 ` [ 10/40] SUNRPC: Dont start the retransmission timer when out of socket space Greg Kroah-Hartman
2013-03-12 22:43 ` [ 11/40] ata_piix: reenable MS Virtual PC guests Greg Kroah-Hartman
2013-03-12 22:43 ` [ 12/40] hw_random: make buffer usable in scatterlist Greg Kroah-Hartman
2013-03-13 22:54 ` Satoru Takeuchi
2013-03-14 6:41 ` Rusty Russell
2013-03-14 13:24 ` [PATCH] hw_random: free rng_buffer at module exit Satoru Takeuchi
2013-03-15 5:05 ` Rusty Russell
2013-03-17 2:14 ` Ben Hutchings
2013-03-18 2:40 ` Rusty Russell
2013-03-20 0:29 ` Satoru Takeuchi
2013-03-12 22:43 ` [ 13/40] mwifiex: correct sleep delay counter Greg Kroah-Hartman
2013-03-12 22:43 ` [ 14/40] ath9k: fix RSSI dummy marker value Greg Kroah-Hartman
2013-03-12 22:43 ` [ 15/40] ath9k_hw: improve reset reliability after errors Greg Kroah-Hartman
2013-03-12 22:43 ` [ 16/40] md: protect against crash upon fsync on ro array Greg Kroah-Hartman
2013-03-12 22:43 ` [ 17/40] md: fix two bugs when attempting to resize RAID0 array Greg Kroah-Hartman
2013-03-12 22:43 ` [ 18/40] md: raid0: fix error return from create_stripe_zones Greg Kroah-Hartman
2013-03-12 22:43 ` [ 19/40] hwmon: (sht15) Check return value of regulator_enable() Greg Kroah-Hartman
2013-03-12 22:43 ` [ 20/40] hwmon: (pmbus/ltc2978) Fix peak attribute handling Greg Kroah-Hartman
2013-03-12 22:43 ` [ 21/40] hwmon: (pmbus/ltc2978) Use detected chip ID to select supported functionality Greg Kroah-Hartman
2013-03-12 22:43 ` [ 22/40] drm/radeon: add primary dac adj quirk for R200 board Greg Kroah-Hartman
2013-03-12 22:43 ` [ 23/40] ARM: 7663/1: perf: fix ARMv7 EVTYPE_MASK to include NSH bit Greg Kroah-Hartman
2013-03-12 22:43 ` [ 24/40] ALSA: ice1712: Initialize card->private_data properly Greg Kroah-Hartman
2013-03-12 22:43 ` [ 25/40] ALSA: vmaster: Fix slave change notification Greg Kroah-Hartman
2013-03-12 22:43 ` [ 26/40] e1000e: fix pci-device enable-counter balance Greg Kroah-Hartman
2013-03-12 22:43 ` [ 27/40] HID: logitech-dj: do not directly call hid_output_raw_report() during probe Greg Kroah-Hartman
2013-03-12 22:43 ` [ 28/40] xen/pat: Disable PAT using pat_enabled value Greg Kroah-Hartman
2013-03-12 22:43 ` [ 29/40] crypto: user - fix info leaks in report API Greg Kroah-Hartman
2013-03-12 22:43 ` [ 30/40] keys: fix race with concurrent install_user_keyrings() Greg Kroah-Hartman
2013-03-12 22:43 ` [ 31/40] Fix: compat_rw_copy_check_uvector() misuse in aio, readv, writev, and security keys Greg Kroah-Hartman
2013-03-12 22:43 ` [ 32/40] vfs: fix pipe counter breakage Greg Kroah-Hartman
2013-03-12 22:43 ` [ 33/40] rtc: rtc-mv: Add support for clk to avoid lockups Greg Kroah-Hartman
2013-03-12 23:08 ` Jason Cooper
2013-03-12 23:15 ` Greg Kroah-Hartman
2013-03-12 22:43 ` [ 34/40] Fix memory leak in cpufreq stats Greg Kroah-Hartman
2013-03-12 22:43 ` [ 35/40] ftrace: Update the kconfig for DYNAMIC_FTRACE Greg Kroah-Hartman
2013-03-12 22:43 ` [ 36/40] dmi_scan: fix missing check for _DMI_ signature in smbios_present() Greg Kroah-Hartman
2013-03-12 22:43 ` [ 37/40] USB: Dont use EHCI port sempahore for USB 3.0 hubs Greg Kroah-Hartman
2013-03-12 22:43 ` [ 38/40] USB: Prepare for refactoring by adding extra udev checks Greg Kroah-Hartman
2013-03-12 22:44 ` Greg Kroah-Hartman [this message]
2013-03-12 22:44 ` [ 40/40] Revert "ALSA: hda - hdmi: Make jacks phantom, if theyre not detectable" Greg Kroah-Hartman
2013-03-13 3:57 ` [ 00/40] 3.4.36-stable review Shuah Khan
2013-03-14 13:39 ` Satoru Takeuchi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130312223215.664489899@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sarah.a.sharp@linux.intel.com \
--cc=stable@vger.kernel.org \
--cc=stern@rowland.harvard.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).