diff options
author | Sarah Sharp <sarah.a.sharp@linux.intel.com> | 2010-01-06 13:16:51 -0500 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@suse.de> | 2010-01-20 18:24:35 -0500 |
commit | 04a723ea9c53ba608b0411aa36948bb57c51a08e (patch) | |
tree | 1a5d7395ccc864b8bdf8688396688951e28a4ca8 | |
parent | b132b04e193908a94d95065d0628f8fb0159cc55 (diff) |
USB: Fix duplicate sysfs problem after device reset.
Borislav Petkov reports issues with duplicate sysfs endpoint files after a
resume from a hibernate. It turns out that the code to support alternate
settings under xHCI has issues when a device with a non-default alternate
setting is reset during the hibernate:
[ 427.681810] Restarting tasks ...
[ 427.681995] hub 1-0:1.0: state 7 ports 6 chg 0004 evt 0000
[ 427.682019] usb usb3: usb resume
[ 427.682030] ohci_hcd 0000:00:12.0: wakeup root hub
[ 427.682191] hub 1-0:1.0: port 2, status 0501, change 0000, 480 Mb/s
[ 427.682205] usb 1-2: usb wakeup-resume
[ 427.682226] usb 1-2: finish reset-resume
[ 427.682886] done.
[ 427.734658] ehci_hcd 0000:00:12.2: port 2 high speed
[ 427.734663] ehci_hcd 0000:00:12.2: GetStatus port 2 status 001005 POWER sig=se0 PE CONNECT
[ 427.746682] hub 3-0:1.0: hub_reset_resume
[ 427.746693] hub 3-0:1.0: trying to enable port power on non-switchable hub
[ 427.786715] usb 1-2: reset high speed USB device using ehci_hcd and address 2
[ 427.839653] ehci_hcd 0000:00:12.2: port 2 high speed
[ 427.839666] ehci_hcd 0000:00:12.2: GetStatus port 2 status 001005 POWER sig=se0 PE CONNECT
[ 427.847717] ohci_hcd 0000:00:12.0: GetStatus roothub.portstatus [1] = 0x00010100 CSC PPS
[ 427.915497] hub 1-2:1.0: remove_intf_ep_devs: if: ffff88022f9e8800 ->ep_devs_created: 1
[ 427.915774] hub 1-2:1.0: remove_intf_ep_devs: bNumEndpoints: 1
[ 427.915934] hub 1-2:1.0: if: ffff88022f9e8800: endpoint devs removed.
[ 427.916158] hub 1-2:1.0: create_intf_ep_devs: if: ffff88022f9e8800 ->ep_devs_created: 0, ->unregistering: 0
[ 427.916434] hub 1-2:1.0: create_intf_ep_devs: bNumEndpoints: 1
[ 427.916609] ep_81: create, parent hub
[ 427.916632] ------------[ cut here ]------------
[ 427.916644] WARNING: at fs/sysfs/dir.c:477 sysfs_add_one+0x82/0x96()
[ 427.916649] Hardware name: System Product Name
[ 427.916653] sysfs: cannot create duplicate filename '/devices/pci0000:00/0000:00:12.2/usb1/1-2/1-2:1.0/ep_81'
[ 427.916658] Modules linked in: binfmt_misc kvm_amd kvm powernow_k8 cpufreq_ondemand cpufreq_powersave cpufreq_userspace freq_table cpufreq_conservative ipv6 vfat fat
+8250_pnp 8250 pcspkr ohci_hcd serial_core k10temp edac_core
[ 427.916694] Pid: 278, comm: khubd Not tainted 2.6.33-rc2-00187-g08d869a-dirty #13
[ 427.916699] Call Trace:
The problem is caused by a mismatch between the USB core's view of the
device state and the USB device and xHCI host's view of the device state.
After the device reset and re-configuration, the device and the xHCI host
think they are using alternate setting 0 of all interfaces. However, the
USB core keeps track of the old state, which may include non-zero
alternate settings. It uses intf->cur_altsetting to keep the endpoint
sysfs files for the old state across the reset.
The bandwidth allocation functions need to know what the xHCI host thinks
the current alternate settings are, so original patch set
intf->cur_altsetting to the alternate setting 0. This caused duplicate
endpoint files to be created.
The solution is to not set intf->cur_altsetting before calling
usb_set_interface() in usb_reset_and_verify_device(). Instead, we add a
new flag to struct usb_interface to tell usb_hcd_alloc_bandwidth() to use
alternate setting 0 as the currently installed alternate setting.
Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Tested-by: Borislav Petkov <petkovbb@googlemail.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
-rw-r--r-- | drivers/usb/core/hcd.c | 18 | ||||
-rw-r--r-- | drivers/usb/core/hub.c | 15 | ||||
-rw-r--r-- | include/linux/usb.h | 1 |
3 files changed, 24 insertions, 10 deletions
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 0495fa651225..80995ef0868c 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c | |||
@@ -1684,6 +1684,24 @@ int usb_hcd_alloc_bandwidth(struct usb_device *udev, | |||
1684 | } | 1684 | } |
1685 | } | 1685 | } |
1686 | if (cur_alt && new_alt) { | 1686 | if (cur_alt && new_alt) { |
1687 | struct usb_interface *iface = usb_ifnum_to_if(udev, | ||
1688 | cur_alt->desc.bInterfaceNumber); | ||
1689 | |||
1690 | if (iface->resetting_device) { | ||
1691 | /* | ||
1692 | * The USB core just reset the device, so the xHCI host | ||
1693 | * and the device will think alt setting 0 is installed. | ||
1694 | * However, the USB core will pass in the alternate | ||
1695 | * setting installed before the reset as cur_alt. Dig | ||
1696 | * out the alternate setting 0 structure, or the first | ||
1697 | * alternate setting if a broken device doesn't have alt | ||
1698 | * setting 0. | ||
1699 | */ | ||
1700 | cur_alt = usb_altnum_to_altsetting(iface, 0); | ||
1701 | if (!cur_alt) | ||
1702 | cur_alt = &iface->altsetting[0]; | ||
1703 | } | ||
1704 | |||
1687 | /* Drop all the endpoints in the current alt setting */ | 1705 | /* Drop all the endpoints in the current alt setting */ |
1688 | for (i = 0; i < cur_alt->desc.bNumEndpoints; i++) { | 1706 | for (i = 0; i < cur_alt->desc.bNumEndpoints; i++) { |
1689 | ret = hcd->driver->drop_endpoint(hcd, udev, | 1707 | ret = hcd->driver->drop_endpoint(hcd, udev, |
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index b9f5fcd713e2..35cc8b9ba1f5 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c | |||
@@ -3695,19 +3695,14 @@ static int usb_reset_and_verify_device(struct usb_device *udev) | |||
3695 | usb_enable_interface(udev, intf, true); | 3695 | usb_enable_interface(udev, intf, true); |
3696 | ret = 0; | 3696 | ret = 0; |
3697 | } else { | 3697 | } else { |
3698 | /* We've just reset the device, so it will think alt | 3698 | /* Let the bandwidth allocation function know that this |
3699 | * setting 0 is installed. For usb_set_interface() to | 3699 | * device has been reset, and it will have to use |
3700 | * work properly, we need to set the current alternate | 3700 | * alternate setting 0 as the current alternate setting. |
3701 | * interface setting to 0 (or the first alt setting, if | ||
3702 | * the device doesn't have alt setting 0). | ||
3703 | */ | 3701 | */ |
3704 | intf->cur_altsetting = | 3702 | intf->resetting_device = 1; |
3705 | usb_find_alt_setting(config, i, 0); | ||
3706 | if (!intf->cur_altsetting) | ||
3707 | intf->cur_altsetting = | ||
3708 | &config->intf_cache[i]->altsetting[0]; | ||
3709 | ret = usb_set_interface(udev, desc->bInterfaceNumber, | 3703 | ret = usb_set_interface(udev, desc->bInterfaceNumber, |
3710 | desc->bAlternateSetting); | 3704 | desc->bAlternateSetting); |
3705 | intf->resetting_device = 0; | ||
3711 | } | 3706 | } |
3712 | if (ret < 0) { | 3707 | if (ret < 0) { |
3713 | dev_err(&udev->dev, "failed to restore interface %d " | 3708 | dev_err(&udev->dev, "failed to restore interface %d " |
diff --git a/include/linux/usb.h b/include/linux/usb.h index e101a2d04d75..d7ace1b80f09 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h | |||
@@ -192,6 +192,7 @@ struct usb_interface { | |||
192 | unsigned needs_altsetting0:1; /* switch to altsetting 0 is pending */ | 192 | unsigned needs_altsetting0:1; /* switch to altsetting 0 is pending */ |
193 | unsigned needs_binding:1; /* needs delayed unbind/rebind */ | 193 | unsigned needs_binding:1; /* needs delayed unbind/rebind */ |
194 | unsigned reset_running:1; | 194 | unsigned reset_running:1; |
195 | unsigned resetting_device:1; /* true: bandwidth alloc after reset */ | ||
195 | 196 | ||
196 | struct device dev; /* interface specific device info */ | 197 | struct device dev; /* interface specific device info */ |
197 | struct device *usb_dev; | 198 | struct device *usb_dev; |