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>
This patch (as1330) fixes a bug in khbud's handling of remote
wakeups. When a device sends a remote-wakeup request, the parent hub
(or the host controller driver, for directly attached devices) begins
the resume sequence and notifies khubd when the sequence finishes. At
this point the port's SUSPEND feature is automatically turned off.
However the device needs an additional 10-ms resume-recovery time
(TRSMRCY in the USB spec). Khubd does not wait for this delay if the
SUSPEND feature is off, and as a result some devices fail to behave
properly following a remote wakeup. This patch adds the missing
delay to the remote-wakeup path.
It also extends the resume-signalling delay used by ehci-hcd and
uhci-hcd from 20 ms (the value in the spec) to 25 ms (the value we use
for non-remote-wakeup resumes). The extra time appears to help some
devices.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Cc: stable <stable@kernel.org>
Cc: Rickard Bellini <rickard.bellini@ericsson.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Memory allocations with GFP_KERNEL can cause IO to a storage
device which can fail resulting in a need to reset the device.
Therefore GFP_KERNEL cannot be safely used between usb_lock_device()
and usb_unlock_device(). Replace by GFP_NOIO.
Signed-off-by: Oliver Neukum <oliver@neukum.org>
Cc: stable <stable@kernel.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Fix new kernel-doc warnings in usb core:
Warning(drivers/usb/core/usb.c:79): No description found for parameter 'config'
Warning(drivers/usb/core/usb.c:79): No description found for parameter 'iface_num'
Warning(drivers/usb/core/usb.c:79): No description found for parameter 'alt_num'
Warning(drivers/usb/core/hcd.c:1622): No description found for parameter 'udev'
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1315) fixes some bugs in the USB core authorization
code:
usb_deauthorize_device() should deallocate the device strings
instead of leaking them, and it should invoke
usb_destroy_configuration() (which does proper reference
counting) instead of freeing the config information directly.
usb_authorize_device() shouldn't change the device strings
until it knows that the authorization will succeed, and it should
autosuspend the device at the end (having autoresumed the
device at the start).
Because the device strings can be changed, the sysfs routines
to display the strings must protect the string pointers by
locking the device.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: Inaky Perez-Gonzalez <inaky@linux.intel.com>
Acked-by: David Vrabel <david.vrabel@csr.com>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1314) renames usb_configure_device() and
usb_configure_device_otg() in the hub driver. Neither name is
appropriate because these routines enumerate devices, they don't
configure them. That's handled by usb_choose_configuration() and
usb_set_configuration().
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
USB drivers that create character devices call usb_register_dev in their
probe function. This associates the usb_interface device with that minor
number and creates the character device and announces it to the world.
However, the driver's probe function is called before the new
usb_interface is added to the driver's klist_devices.
This is a problem because userspace will respond to the character device
creation announcement by opening the character device. The driver's open
function will the call usb_find_interface to find the usb_interface
associated with that minor number. usb_find_interface will walk the
driver's list of devices and find the usb_interface with the matching
minor number.
Because the announcement happens before the usb_interface is added to the
driver's klist_devices, a race condition exists. A straightforward fix
is to walk the list of devices on usb_bus_type instead since the device
is added to that list before the announcement occurs.
bus_find_device calls get_device to bump the reference count on the found
device. It is arguable that the reference count should be dropped by the
caller of usb_find_interface instead of usb_find_interface, however,
the current users of usb_find_interface do not expect this.
The original version of this patch only matched against minor number
instead of driver and minor number. This version matches against both.
Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Fix the following sparse warning:
drivers/usb/core/message.c:1583:6: warning: symbol '__usb_queue_reset_device' was not declared. Should it be static?
Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Fix the following sparse warning:
drivers/usb/core/hub.c:1664:37: warning: Using plain integer as NULL pointer
Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Fix the following sparse warning:
drivers/usb/core/usb.c:1033:15: warning: symbol 'usb_debug_devices' was not declared. Should it be static?
Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Make the USB core check the bandwidth when switching from one
interface alternate setting to another. Also check the bandwidth
when resetting a configuration (so that alt setting 0 is used). If
this check fails, the device's state is unchanged. If the device
refuses the new alt setting, re-instate the old alt setting in the
host controller hardware.
If a USB device doesn't have an alternate interface setting 0, install
the first alt setting in its descriptors when a new configuration is
requested, or the device is reset.
Add a mutex per root hub to protect bandwidth operations:
adding/reseting/changing configurations, and changing alternate interface
settings. We want to ensure that the xHCI host controller and the USB
device are set up for the same configurations and alternate settings.
There are two (possibly three) steps to do this:
1. The host controller needs to check that bandwidth is available for a
different setting, by issuing and waiting for a configure endpoint
command.
2. Once that returns successfully, a control message is sent to the
device.
3. If that fails, the host controller must be notified through another
configure endpoint command.
The mutex is used to make these three operations seem atomic, to prevent
another driver from using more bandwidth for a different device while
we're in the middle of these operations.
While we're touching the bandwidth code, rename usb_hcd_check_bandwidth()
to usb_hcd_alloc_bandwidth(). This function does more than just check
that the bandwidth change won't exceed the bus bandwidth; it actually
changes the bandwidth configuration in the xHCI host controller.
Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Refactor out the code to find alternate interface settings into
usb_find_alt_setting(). Print a debugging message and return null if the
alt setting is not found.
While we're at it, correct a bug in the refactored code. The interfaces
in the configuration's interface cache are not necessarily in numerical
order, so we can't just use the interface number as an array index. Loop
through the interface caches, looking for the correct interface.
Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Accroding commit 0994375e, which is adding remove_id sysfs attr
for pci drivers, for management tools dynamically bind/unbind
a pci/usb devices to a specified drivers; with this patch,
the management tools can be simplied.
And the original code didn't handle the failure of
usb_create_newid_file, fixed in this patch.
Signed-off-by: CHENG Renquan <rqcheng@smu.edu.sg>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1303) revises the USB Power Management infrastructure to
make it compatible with the new driver-model Runtime PM framework:
Drivers are no longer allowed to access intf->pm_usage_cnt
directly; the PM framework manages its own usage counters.
usb_autopm_set_interface() is eliminated, because it directly
sets intf->pm_usage_cnt.
usb_autopm_enable() and usb_autopm_disable() are eliminated,
because they call usb_autopm_set_interface().
usb_autopm_get_interface_no_resume() and
usb_autopm_put_interface_no_suspend() are added. They
correspond to pm_runtime_get_noresume() and
pm_runtime_put_noidle() in the PM framework.
The power/level attribute no longer accepts "suspend", only
"on" and "auto". The PM framework doesn't allow devices to be
forced into a suspended mode.
The hub driver contains the only code that violates the new
guidelines. It is updated to use the new interface routines instead.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This is not exported from the usb core, yet we rely on it to create
paths to interfaces for this device in sysfs. Export it to make
userspace tools have an easier time to figure things out.
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Add a missing goto. We dereference usb_class on the next line.
Found by smatch static checker.
Signed-off-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This patch (as1302) removes the auto_pm flag from struct usb_device.
The flag's only purpose was to distinguish between autosuspends and
external suspends, but that information is now available in the
pm_message_t argument passed to suspend methods.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
USB drivers that create character devices call usb_register_dev in their
probe function. This associates the usb_interface device with that minor
number and creates the character device and announces it to the world.
However, the driver's probe function is called before the new
usb_interface is added to the driver's klist_devices.
This is a problem because userspace will respond to the character device
creation announcement by opening the character device. The driver's open
function will the call usb_find_interface to find the usb_interface
associated with that minor number. usb_find_interface will walk the
driver's list of devices and find the usb_interface with the matching
minor number.
Because the announcement happens before the usb_interface is added to the
driver's klist_devices, a race condition exists. A straightforward fix
is to walk the list of devices on usb_bus_type instead since the device
is added to that list before the announcement occurs.
bus_find_device calls get_device to bump the reference count on the found
device. It is arguable that the reference count should be dropped by the
caller of usb_find_interface instead of usb_find_interface, however,
the current users of usb_find_interface do not expect this.
Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
In map_urb_for_dma(), the DMA address returned by dma_map_single()
is not checked to determine if it is legal. This lack of checking
contributed to a problem with the libertas wireless driver
(http://marc.info/?l=linux-wireless&m=125695331205062&w=2). The
difficulty was not detected until the buffer was unmapped. By this time
memory corruption had occurred.
The situation is fixed by testing the returned DMA address, and
returning -EAGAIN if the address is invalid.
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
