| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
syzbot is reporting hung task at wdm_flush() [1], for there is a circular
dependency that wdm_flush() from flip_close() for /dev/cdc-wdm0 forever
waits for /dev/raw-gadget to be closed while close() for /dev/raw-gadget
cannot be called unless close() for /dev/cdc-wdm0 completes.
Tetsuo Handa considered that such circular dependency is an usage error [2]
which corresponds to an unresponding broken hardware [3]. But Alan Stern
responded that we should be prepared for such hardware [4]. Therefore,
this patch changes wdm_flush() to use wait_event_interruptible_timeout()
which gives up after 30 seconds, for hardware that remains silent must be
ignored. The 30 seconds are coming out of thin air.
Changing wait_event() to wait_event_interruptible_timeout() makes error
reporting from close() syscall less reliable. To compensate it, this patch
also implements wdm_fsync() which does not use timeout. Those who want to
be very sure that data has gone out to the device are now advised to call
fsync(), with a caveat that fsync() can return -EINVAL when running on
older kernels which do not implement wdm_fsync().
This patch also fixes three more problems (listed below) found during
exhaustive discussion and testing.
Since multiple threads can concurrently call wdm_write()/wdm_flush(),
we need to use wake_up_all() whenever clearing WDM_IN_USE in order to
make sure that all waiters are woken up. Also, error reporting needs
to use fetch-and-clear approach in order not to report same error for
multiple times.
Since wdm_flush() checks WDM_DISCONNECTING, wdm_write() should as well
check WDM_DISCONNECTING.
In wdm_flush(), since locks are not held, it is not safe to dereference
desc->intf after checking that WDM_DISCONNECTING is not set [5]. Thus,
remove dev_err() from wdm_flush().
[1] https://syzkaller.appspot.com/bug?id=e7b761593b23eb50855b9ea31e3be5472b711186
[2] https://lkml.kernel.org/r/[email protected]
[3] https://lkml.kernel.org/r/[email protected]
[4] https://lkml.kernel.org/r/[email protected]
[5] https://lkml.kernel.org/r/[email protected]
Reported-by: syzbot <[email protected]>
Cc: stable <[email protected]>
Co-developed-by: Tetsuo Handa <[email protected]>
Signed-off-by: Tetsuo Handa <[email protected]>
Signed-off-by: Oliver Neukum <[email protected]>
Cc: Alan Stern <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
manage_power arg
A good attempt was made to document everything else.
Fixes the following W=1 kernel build warning(s):
drivers/usb/class/cdc-wdm.c:961: warning: Function parameter or member 'manage_power' not described in 'usb_cdc_wdm_register'
Cc: Oliver Neukum <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
The .ioctl and .compat_ioctl file operations have the same prototype so
they can both point to the same function, which works great almost all
the time when all the commands are compatible.
One exception is the s390 architecture, where a compat pointer is only
31 bit wide, and converting it into a 64-bit pointer requires calling
compat_ptr(). Most drivers here will never run in s390, but since we now
have a generic helper for it, it's easy enough to use it consistently.
I double-checked all these drivers to ensure that all ioctl arguments
are used as pointers or are ignored, but are not interpreted as integer
values.
Acked-by: Jason Gunthorpe <[email protected]>
Acked-by: Daniel Vetter <[email protected]>
Acked-by: Mauro Carvalho Chehab <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
Acked-by: David Sterba <[email protected]>
Acked-by: Darren Hart (VMware) <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
Acked-by: Bjorn Andersson <[email protected]>
Acked-by: Dan Williams <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
|
|
In case of a disconnect an ongoing flush() has to be made fail.
Nevertheless we cannot be sure that any pending URB has already
finished, so although they will never succeed, they still must
not be touched.
The clean solution for this is to check for WDM_IN_USE
and WDM_DISCONNECTED in flush(). There is no point in ever
clearing WDM_IN_USE, as no further writes make sense.
The issue is as old as the driver.
Fixes: afba937e540c9 ("USB: CDC WDM driver")
Reported-by: [email protected]
Signed-off-by: Oliver Neukum <[email protected]>
Cc: stable <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
The variable rv is assigned with a value that is never read and
it is re-assigned a new value on the next statement. The
assignment is redundant and can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
'rv' is the correct return value, pass it upstream instead of 0
Fixes: 17d80d562fd7 ("USB: autosuspend for cdc-wdm")
Signed-off-by: YueHaibing <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
service_outstanding_interrupt()"
This reverts commit 6e22e3af7bb3a7b9dc53cb4687659f6e63fca427.
The bug the patch describes to, has been already fixed in commit
2df6948428542 ("USB: cdc-wdm: don't enable interrupts in USB-giveback")
so need to this, revert it.
Fixes: 6e22e3af7bb3 ("usb: cdc-wdm: Fix a sleep-in-atomic-context bug in service_outstanding_interrupt()")
Cc: stable <[email protected]>
Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
service_outstanding_interrupt()
wdm_in_callback() is a completion handler function for the USB driver.
So it should not sleep. But it calls service_outstanding_interrupt(),
which calls usb_submit_urb() with GFP_KERNEL.
To fix this bug, GFP_KERNEL is replaced with GFP_ATOMIC.
This bug is found by my static analysis tool DSAC.
Signed-off-by: Jia-Ju Bai <[email protected]>
Cc: stable <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
The USB completion callback does not disable interrupts while acquiring
the lock. We want to remove the local_irq_disable() invocation from
__usb_hcd_giveback_urb() and therefore it is required for the callback
handler to disable the interrupts while acquiring the lock.
The callback may be invoked either in IRQ or BH context depending on the
USB host controller.
Use the _irqsave() variant of the locking primitives.
Cc: Oliver Neukum <[email protected]>
Cc: "Bjørn Mork" <[email protected]>
Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
In the code path
__usb_hcd_giveback_urb()
-> wdm_in_callback()
-> service_outstanding_interrupt()
The function service_outstanding_interrupt() will unconditionally enable
interrupts during unlock and invoke usb_submit_urb() with GFP_KERNEL.
If the HCD completes in BH (like ehci does) then the context remains
atomic due local_bh_disable() and enabling interrupts does not change
this.
Defer the error case handling to a workqueue as suggested by Oliver
Neukum. In case of an error the worker performs the read out and wakes
the user.
Fixes: c1da59dad0eb ("cdc-wdm: Clear read pipeline in case of error")
Cc: Robert Foss <[email protected]>
Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
Acked-by: Oliver Neukum <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
This is the mindless scripted replacement of kernel use of POLL*
variables as described by Al, done by this script:
for V in IN OUT PRI ERR RDNORM RDBAND WRNORM WRBAND HUP RDHUP NVAL MSG; do
L=`git grep -l -w POLL$V | grep -v '^t' | grep -v /um/ | grep -v '^sa' | grep -v '/poll.h$'|grep -v '^D'`
for f in $L; do sed -i "-es/^\([^\"]*\)\(\<POLL$V\>\)/\\1E\\2/" $f; done
done
with de-mangling cleanups yet to come.
NOTE! On almost all architectures, the EPOLL* constants have the same
values as the POLL* constants do. But they keyword here is "almost".
For various bad reasons they aren't the same, and epoll() doesn't
actually work quite correctly in some cases due to this on Sparc et al.
The next patch from Al will sort out the final differences, and we
should be all done.
Scripted-by: Al Viro <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
|
|
Signed-off-by: Al Viro <[email protected]>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
Pull USB/PHY updates from Greg KH:
"Here is the big set of USB and PHY driver updates for 4.15-rc1.
There is the usual amount of gadget and xhci driver updates, along
with phy and chipidea enhancements. There's also a lot of SPDX tags
and license boilerplate cleanups as well, which provide some churn in
the diffstat.
Other major thing is the typec code that moved out of staging and into
the "real" part of the drivers/usb/ tree, which was nice to see
happen.
All of these have been in linux-next with no reported issues for a
while"
* tag 'usb-4.15-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb: (263 commits)
usb: gadget: f_fs: Fix use-after-free in ffs_free_inst
USB: usbfs: compute urb->actual_length for isochronous
usb: core: message: remember to reset 'ret' to 0 when necessary
USB: typec: Remove remaining redundant license text
USB: typec: add SPDX identifiers to some files
USB: renesas_usbhs: rcar?.h: add SPDX tags
USB: chipidea: ci_hdrc_tegra.c: add SPDX line
USB: host: xhci-debugfs: add SPDX lines
USB: add SPDX identifiers to all remaining Makefiles
usb: host: isp1362-hcd: remove a couple of redundant assignments
USB: adutux: remove redundant variable minor
usb: core: add a new usb_get_ptm_status() helper
usb: core: add a 'type' parameter to usb_get_status()
usb: core: introduce a new usb_get_std_status() helper
usb: core: rename usb_get_status() 'type' argument to 'recip'
usb: core: add Status Type definitions
USB: gadget: Remove redundant license text
USB: gadget: function: Remove redundant license text
USB: gadget: udc: Remove redundant license text
USB: gadget: legacy: Remove redundant license text
...
|
|
It's good to have SPDX identifiers in all files to make it easier to
audit the kernel tree for correct licenses.
Update the drivers/usb/ and include/linux/usb* files with the correct
SPDX license identifier based on the license text in the file itself.
The SPDX identifier is a legally binding shorthand, which can be used
instead of the full boiler plate text.
This work is based on a script and data from Thomas Gleixner, Philippe
Ombredanne, and Kate Stewart.
Cc: Thomas Gleixner <[email protected]>
Cc: Kate Stewart <[email protected]>
Cc: Philippe Ombredanne <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
Acked-by: Felipe Balbi <[email protected]>
Acked-by: Johan Hovold <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
to READ_ONCE()/WRITE_ONCE()
Please do not apply this to mainline directly, instead please re-run the
coccinelle script shown below and apply its output.
For several reasons, it is desirable to use {READ,WRITE}_ONCE() in
preference to ACCESS_ONCE(), and new code is expected to use one of the
former. So far, there's been no reason to change most existing uses of
ACCESS_ONCE(), as these aren't harmful, and changing them results in
churn.
However, for some features, the read/write distinction is critical to
correct operation. To distinguish these cases, separate read/write
accessors must be used. This patch migrates (most) remaining
ACCESS_ONCE() instances to {READ,WRITE}_ONCE(), using the following
coccinelle script:
----
// Convert trivial ACCESS_ONCE() uses to equivalent READ_ONCE() and
// WRITE_ONCE()
// $ make coccicheck COCCI=/home/mark/once.cocci SPFLAGS="--include-headers" MODE=patch
virtual patch
@ depends on patch @
expression E1, E2;
@@
- ACCESS_ONCE(E1) = E2
+ WRITE_ONCE(E1, E2)
@ depends on patch @
expression E;
@@
- ACCESS_ONCE(E)
+ READ_ONCE(E)
----
Signed-off-by: Mark Rutland <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
|
|
The driver will forward errors to userspace after turning most of them
into -EIO. But all status codes are not equal. The -EPIPE (stall) in
particular can be seen more as a result of normal USB signaling than
an actual error. The state is automatically cleared by the USB core
without intervention from either driver or userspace.
And most devices and firmwares will never trigger a stall as a result
of GetEncapsulatedResponse. This is in fact a requirement for CDC WDM
devices. Quoting from section 7.1 of the CDC WMC spec revision 1.1:
The function shall not return STALL in response to
GetEncapsulatedResponse.
But this driver is also handling GetEncapsulatedResponse on behalf of
the qmi_wwan and cdc_mbim drivers. Unfortunately the relevant specs
are not as clear wrt stall. So some QMI and MBIM devices *will*
occasionally stall, causing the GetEncapsulatedResponse to return an
-EPIPE status. Translating this into -EIO for userspace has proven to
be harmful. Treating it as an empty read is safer, making the driver
behave as if the device was conforming to the CDC WDM spec.
There have been numerous reports of issues related to -EPIPE errors
from some newer CDC MBIM devices in particular, like for example the
Fibocom L831-EAU. Testing on this device has shown that the issues
go away if we simply ignore the -EPIPE status. Similar handling of
-EPIPE is already known from e.g. usb_get_string()
The -EPIPE log message is still kept to let us track devices with this
unexpected behaviour, hoping that it attracts attention from firmware
developers.
Cc: <[email protected]>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100938
Reported-and-tested-by: Christian Ehrig <[email protected]>
Reported-and-tested-by: Patrick Chilton <[email protected]>
Reported-and-tested-by: Andreas Böhler <[email protected]>
Signed-off-by: Bjørn Mork <[email protected]>
Acked-by: Oliver Neukum <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
No one uses the DRIVER_VERSION define in this driver, so just delete it.
Cc: Oliver Neukum <[email protected]>
Cc: "Bjørn Mork" <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Use memdup_user() helper instead of open-coding to simplify the code.
Signed-off-by: Geliang Tang <[email protected]>
Acked-by: Oliver Neukum <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
This reverts commit 833415a3e781 ("cdc-wdm: fix "out-of-sync" due to
missing notifications")
There have been several reports of wdm_read returning unexpected EIO
errors with QMI devices using the qmi_wwan driver. The reporters
confirm that reverting prevents these errors. I have been unable to
reproduce the bug myself, and have no explanation to offer either. But
reverting is the safe choice here, given that the commit was an
attempt to work around a firmware problem. Living with a firmware
problem is still better than adding driver bugs.
Reported-by: Kasper Holtze <[email protected]>
Reported-by: Aleksander Morgado <[email protected]>
Reported-by: Daniele Palmas <[email protected]>
Cc: <[email protected]> # v4.9+
Fixes: 833415a3e781 ("cdc-wdm: fix "out-of-sync" due to missing notifications")
Signed-off-by: Bjørn Mork <[email protected]>
Acked-by: Oliver Neukum <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Remove logically dead code.
'cntr' is always equal to zero when the following line of code is executed:
rv = cntr ? cntr : -EAGAIN;
Addresses-Coverity-ID: 113227
Signed-off-by: Gustavo A. R. Silva <[email protected]>
Acked-by: Oliver Neukum <[email protected]>
Reviewed-by: Peter Senna Tschudin <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Debug messages should be properly terminated.
Signed-off-by: Oliver Neukum <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Dynamic debugging will already add the function (and the line number)
to a debug message if one requests that. It makes no sense to add
them unconditionally in a driver.
Signed-off-by: Oliver Neukum <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Implemented queued response handling. This queue is processed every time the
WDM_READ flag is cleared.
In case of a read error, userspace may not actually read the data, since the
driver returns an error through wdm_poll. After this, the underlying device may
attempt to send us more data, but the queue is not processed. While userspace is
also blocked, because the read error is never cleared.
After this patch, we proactively process the queue on a read error. If there was
an outstanding response to handle, that will clear the error (or go through the
same logic again, if another read error occurs). If there was no outstanding
response, this will bring the queue size back to 0, unblocking a future response
from the underlying device.
Signed-off-by: Robert Foss <[email protected]>
Tested-by: Robert Foss <[email protected]>
Acked-by: Oliver Neukum <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
The driver enforces a strict one-to-one relationship between the
received RESPONSE_AVAILABLE notifications and messages read from
the device. At the same time, it will cancel the interrupt URB
when there is no client holding the character device open.
Many devices do not cope well with this behaviour. They maintain
a FIFO queue of messages, and send notifications on a best effort
basis. Messages are queued regardless of whether the notification
is successful or not. So if the driver loses a single notification,
which can easily happen when the interrupt URB is cancelled, then
the device and driver becomes out-of-sync. New messages end up
at the end of the queue, while the associated notification makes
the driver read only the first message from the queue.
This state is permanent from a user point of view. There is no
no way to flush the device queue without resetting the device or
using another driver.
The problem is easy to hit with current QMI and MBIM command line
tools, which typically close the character device after seeing
the reply they expect. Any pending unsolicited messages from the
device will then trigger the driver bug.
Fix by always reading all queued messages from the device when
the notification URB is first submitted. This is expected to
end with an -EPIPE status when there are no more pending
messages, so demote the printk associated with -EPIPE to debug
level.
The workaround has been tested on a large number of different MBIM
and QMI devices, as well as the Ericsson F5521gw and H5321gw modems
with real Device Management functions.
Signed-off-by: Bjørn Mork <[email protected]>
Acked-by: Oliver Neukum <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Now that the common parser resides in USB core, it can
be used for CDC-WDM.
Signed-off-by: Oliver Neukum <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
One more case of error codes not correctly being
correctly returned to user space.
Signed-off-by: Olive Neukum <[email protected]>0
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Values directly from descriptors given in debug statements
must be converted to native endianness.
Signed-off-by: Oliver Neukum <[email protected]>
CC: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
This makes sure the error handling path is the same for
all error conditions, thus reducing code duplication.
Signed-off-by: Oliver Neukum <[email protected]>0
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Lieing to user space is wrong. The real reason for a failure
to write should be returned to user space.
Signed-off-by: Oliver Neukum <[email protected]>0
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Do not decrement resp_count if it's already 0.
We set resp_count to 0 when the device is closed. The next open and
read will try to clear the WDM_READ flag if there was leftover data
in the read buffer. This fix is necessary to prevent resubmitting
the read URB in a tight loop because resp_count becomes negative.
The bug can easily be triggered from userspace by not reading all
data in the read buffer, and then closing and reopening the chardev.
Fixes: 8dd5cd5395b9 ("usb: cdc-wdm: avoid hanging on zero length reads")
Cc: <[email protected]> # 3.13
Signed-off-by: Bjørn Mork <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
This resolves the merge issue with drivers/usb/host/ohci-at91.c
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
commit 73e06865ead1 ("USB: cdc-wdm: support back-to-back
USB_CDC_NOTIFY_RESPONSE_AVAILABLE notifications") implemented
queued response handling. This added a new requirement: The read
urb must be resubmitted every time we clear the WDM_READ flag if
the response counter indicates that the device is waiting for a
read.
Fix by factoring out the code handling the WMD_READ clearing and
possible urb submission, calling it everywhere we clear the flag.
Without this fix, the driver ends up in a state where the read urb
is inactive, but the response counter is positive after a zero
length read. This prevents the read urb from ever being submitted
again and the driver appears to be hanging.
Fixes: 73e06865ead1 ("USB: cdc-wdm: support back-to-back USB_CDC_NOTIFY_RESPONSE_AVAILABLE notifications")
Cc: Greg Suarez <[email protected]>
Signed-off-by: Bjørn Mork <[email protected]>
Cc: stable <[email protected]> # 3.13
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Cc: stable <[email protected]>
Reported-by: Oliver Neukum <[email protected]>
Signed-off-by: Bjørn Mork <[email protected]>
Acked-by: Oliver Neukum <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
The only notification supported by the Device Management class is
Response Available. But this driver is also used as a subdriver of
other CDC classes, allowing notifications like Speed Change and
Network Connection. This results in log messages which are only
confusing to an end user:
[66255.801874] cdc_mbim 1-3:1.5: unknown notification 42 received: index 5 len 8
These drivers use cdc-wdm as a subdriver to allow access to an
embedded management protocol, and all management is expected to
use this protocol. There is therefore no need to handle any of
these optional CDC notifications. Instead we can let the cdc-wdm
driver recognize them and log a debug level message instead of an
error.
Reported-by: Rob Gardner <[email protected]>
Signed-off-by: Bjørn Mork <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
notifications
Some MBIM devices send back-to-back USB_CDC_NOTIFY_RESPONSE_AVAILABLE notifications
when sending a message over multiple fragments or when there are unsolicited
messages available.
Count up the number of USB_CDC_NOTIFY_RESPONSE_AVAILABLE notifications received
and decrement the count and submit the urb for the next response each time userspace
completes a read the response.
Signed-off-by: Greg Suarez <[email protected]>
Acked-by: Bjørn Mork <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Both could want to submit the same URB. Some checks of the flag
intended to prevent that were missing.
Signed-off-by: Oliver Neukum <[email protected]>
CC: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Userspace applications need to know the maximum supported message
size.
The cdc-wdm driver translates between a character device stream
and a message based protocol. Each message is transported as a
usb control message with no further encapsulation or syncronization.
Each read or write on the character device should translate to
exactly one usb control message to ensure that message boundaries
are kept intact. That means that the userspace application must
know the maximum message size supported by the device and driver,
making this size a vital part of the cdc-wdm character device API.
CDC WDM and CDC MBIM functions export the maximum supported
message size through CDC functional descriptors. The cdc-wdm and
cdc_mbim drivers will parse these descriptors and use the value
chosen by the device. The only current way for a userspace
application to retrive the value is by duplicating the descriptor
parsing. This is an unnecessary complex task, and application
writers are likely to postpone it, using a fixed value and adding
a "todo" item.
QMI functions have no way to tell the host what message size they
support. The qmi_wwan driver use a fixed value based on protocol
recommendations and observed device behaviour. Userspace
applications must know and hard code the same value. This scheme
will break if we ever encounter a QMI device needing a device
specific message size quirk. We are currently unable to support
such a device because using a non default size would break the
implicit userspace API.
The message size is currently a hidden attribute of the cdc-wdm
userspace API. Retrieving it is unnecessarily complex, increasing
the possibility of drivers and applications using different limits.
The resulting errors are hard to debug, and can only be replicated
on identical hardware.
Exporting the maximum message size from the driver simplifies the
task for the userspace application, and creates a unified
information source independent of device and function class. It also
serves to document that the message size is part of the cdc-wdm
userspace API.
This proposed API extension has been presented for the authors of
userspace applications and libraries using the current API: libmbim,
libqmi, uqmi, oFono and ModemManager. The replies were:
Aleksander Morgado:
"We do really need max message size for MBIM; and as you say, it may be
good to have the max message size info also for QMI, so the new ioctl
seems a good addition. So +1 from my side, for what it's worth."
Dan Williams:
"Yeah, +1 here. I'd prefer the sysfs file, but the fact that that
doesn't work for fd passing pretty much kills it."
No negative replies are so far received.
Cc: Aleksander Morgado <[email protected]>
Cc: Dan Williams <[email protected]>
Signed-off-by: Bjørn Mork <[email protected]>
Acked-by: Oliver Neukum <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
The buffer for responses must not overflow.
If this would happen, set a flag, drop the data and return
an error after user space has read all remaining data.
Signed-off-by: Oliver Neukum <[email protected]>
CC: [email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
A logic error made the wdm_find_device* functions
return a bogus pointer into static data instead of
the intended NULL no matching device was found.
Cc: stable <[email protected]> # v3.4+
Cc: Oliver Neukum <[email protected]>
Signed-off-by: Bjørn Mork <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
This resolves the merge issue with the drivers/usb/host/ehci-omap.c
file.
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Clear the WDM_READ flag on empty reads to avoid running
forever in an infinite tight loop, causing lockups:
Jul 1 21:58:11 nemi kernel: [ 3658.898647] qmi_wwan 2-1:1.2: Unexpected error -71
Jul 1 21:58:36 nemi kernel: [ 3684.072021] BUG: soft lockup - CPU#0 stuck for 23s! [qmi.pl:12235]
Jul 1 21:58:36 nemi kernel: [ 3684.072212] CPU 0
Jul 1 21:58:36 nemi kernel: [ 3684.072355]
Jul 1 21:58:36 nemi kernel: [ 3684.072367] Pid: 12235, comm: qmi.pl Tainted: P O 3.5.0-rc2+ #13 LENOVO 2776LEG/2776LEG
Jul 1 21:58:36 nemi kernel: [ 3684.072383] RIP: 0010:[<ffffffffa0635008>] [<ffffffffa0635008>] spin_unlock_irq+0x8/0xc [cdc_wdm]
Jul 1 21:58:36 nemi kernel: [ 3684.072388] RSP: 0018:ffff88022dca1e70 EFLAGS: 00000282
Jul 1 21:58:36 nemi kernel: [ 3684.072393] RAX: ffff88022fc3f650 RBX: ffffffff811c56f7 RCX: 00000001000ce8c1
Jul 1 21:58:36 nemi kernel: [ 3684.072398] RDX: 0000000000000010 RSI: 000000000267d810 RDI: ffff88022fc3f650
Jul 1 21:58:36 nemi kernel: [ 3684.072403] RBP: ffff88022dca1eb0 R08: ffffffffa063578e R09: 0000000000000000
Jul 1 21:58:36 nemi kernel: [ 3684.072407] R10: 0000000000000008 R11: 0000000000000246 R12: 0000000000000002
Jul 1 21:58:36 nemi kernel: [ 3684.072412] R13: 0000000000000246 R14: ffffffff00000002 R15: ffff8802281d8c88
Jul 1 21:58:36 nemi kernel: [ 3684.072418] FS: 00007f666a260700(0000) GS:ffff88023bc00000(0000) knlGS:0000000000000000
Jul 1 21:58:36 nemi kernel: [ 3684.072423] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Jul 1 21:58:36 nemi kernel: [ 3684.072428] CR2: 000000000270d9d8 CR3: 000000022e865000 CR4: 00000000000007f0
Jul 1 21:58:36 nemi kernel: [ 3684.072433] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
Jul 1 21:58:36 nemi kernel: [ 3684.072438] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Jul 1 21:58:36 nemi kernel: [ 3684.072444] Process qmi.pl (pid: 12235, threadinfo ffff88022dca0000, task ffff88022ff76380)
Jul 1 21:58:36 nemi kernel: [ 3684.072448] Stack:
Jul 1 21:58:36 nemi kernel: [ 3684.072458] ffffffffa063592e 0000000100020000 ffff88022fc3f650 ffff88022fc3f6a8
Jul 1 21:58:36 nemi kernel: [ 3684.072466] 0000000000000200 0000000100000000 000000000267d810 0000000000000000
Jul 1 21:58:36 nemi kernel: [ 3684.072475] 0000000000000000 ffff880212cfb6d0 0000000000000200 ffff880212cfb6c0
Jul 1 21:58:36 nemi kernel: [ 3684.072479] Call Trace:
Jul 1 21:58:36 nemi kernel: [ 3684.072489] [<ffffffffa063592e>] ? wdm_read+0x1a0/0x263 [cdc_wdm]
Jul 1 21:58:36 nemi kernel: [ 3684.072500] [<ffffffff8110adb7>] ? vfs_read+0xa1/0xfb
Jul 1 21:58:36 nemi kernel: [ 3684.072509] [<ffffffff81040589>] ? alarm_setitimer+0x35/0x64
Jul 1 21:58:36 nemi kernel: [ 3684.072517] [<ffffffff8110aec7>] ? sys_read+0x45/0x6e
Jul 1 21:58:36 nemi kernel: [ 3684.072525] [<ffffffff813725f9>] ? system_call_fastpath+0x16/0x1b
Jul 1 21:58:36 nemi kernel: [ 3684.072557] Code: <66> 66 90 c3 83 ff ed 89 f8 74 16 7f 06 83 ff a1 75 0a c3 83 ff f4
The WDM_READ flag is normally cleared by wdm_int_callback
before resubmitting the read urb, and set by wdm_in_callback
when this urb returns with data or an error. But a crashing
device may cause both a read error and cancelling all urbs.
Make sure that the flag is cleared by wdm_read if the buffer
is empty.
We don't clear the flag on errors, as there may be pending
data in the buffer which should be processed. The flag will
instead be cleared on the next wdm_read call.
Cc: stable <[email protected]>
Signed-off-by: Bjørn Mork <[email protected]>
Acked-by: Oliver Neukum <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
qmi_wwan has been changed to drive both the control and data
interface for all QMI/wwan devices, using cdc-wdm as a subdriver.
Remove the stale device ID entries from cdc-wdm.
>From now on new QMI/wwan devices will only need to be added to
the qmi_wwan driver, regardless of the USB descriptor layout
Note that this is not appropriate for stable/longterm kernels
despite being a device ID patch.
Cc: Oliver Neukum <[email protected]>
Signed-off-by: Bjørn Mork <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Tested-by: Thomas Schäfer <[email protected]>
Signed-off-by: Bjørn Mork <[email protected]>
Cc: stable <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Hub-initiated LPM is not good for USB communications devices. Comms
devices should be able to tell when their link can go into a lower power
state, because they know when an incoming transmission is finished.
Ideally, these devices would slam their links into a lower power state,
using the device-initiated LPM, after finishing the last packet of their
data transfer.
If we enable the idle timeouts for the parent hubs to enable
hub-initiated LPM, we will get a lot of useless LPM packets on the bus
as the devices reject LPM transitions when they're in the middle of
receiving data. Worse, some devices might blindly accept the
hub-initiated LPM and power down their radios while they're in the
middle of receiving a transmission.
The Intel Windows folks are disabling hub-initiated LPM for all USB
communications devices under a xHCI USB 3.0 host. In order to keep
the Linux behavior as close as possible to Windows, we need to do the
same in Linux.
Set the disable_hub_initiated_lpm flag for for all USB communications
drivers. I know there aren't currently any USB 3.0 devices that
implement these class specifications, but we should be ready if they do.
Signed-off-by: Sarah Sharp <[email protected]>
Cc: Marcel Holtmann <[email protected]>
Cc: Gustavo Padovan <[email protected]>
Cc: Johan Hedberg <[email protected]>
Cc: Hansjoerg Lipp <[email protected]>
Cc: Tilman Schmidt <[email protected]>
Cc: Karsten Keil <[email protected]>
Cc: Peter Korsgaard <[email protected]>
Cc: Jan Dumon <[email protected]>
Cc: Petko Manolov <[email protected]>
Cc: Steve Glendinning <[email protected]>
Cc: "John W. Linville" <[email protected]>
Cc: Kalle Valo <[email protected]>
Cc: "Luis R. Rodriguez" <[email protected]>
Cc: Jouni Malinen <[email protected]>
Cc: Vasanthakumar Thiagarajan <[email protected]>
Cc: Senthil Balasubramanian <[email protected]>
Cc: Christian Lamparter <[email protected]>
Cc: Brett Rudley <[email protected]>
Cc: Roland Vossen <[email protected]>
Cc: Arend van Spriel <[email protected]>
Cc: "Franky (Zhenhui) Lin" <[email protected]>
Cc: Kan Yan <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Jussi Kivilinna <[email protected]>
Cc: Ivo van Doorn <[email protected]>
Cc: Gertjan van Wingerde <[email protected]>
Cc: Helmut Schaa <[email protected]>
Cc: Herton Ronaldo Krzesinski <[email protected]>
Cc: Hin-Tak Leung <[email protected]>
Cc: Larry Finger <[email protected]>
Cc: Chaoming Li <[email protected]>
Cc: Daniel Drake <[email protected]>
Cc: Ulrich Kunitz <[email protected]>
Signed-off-by: Sarah Sharp <[email protected]>
|
|
Prevents dereferencing an invalid struct usb_interface
pointer.
Always delete entry from device list whether or not the
rest of the device state cleanup is postponed. The device
list uses desc->intf as key, and wdm_open will dereference
this key while searching for a matching device. A device
should not appear in the list unless probe() has succeeded
and disconnect() has not finished.
Cc: Oliver Neukum <[email protected]>
Cc: stable <[email protected]>
Signed-off-by: Bjørn Mork <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
We cannot dereference a removed USB interface for
dev_printk. Use pr_debug instead where necessary.
Flush errors are expected if device is unplugged and are
therefore best ingored at this point.
Move the kill_urbs() call in wdm_release with dev_dbg()
for the non disconnect, as we know it has already been
called if WDM_DISCONNECTING is set. This does not
actually fix anything, but keeps the code more consistent.
Cc: Oliver Neukum <[email protected]>
Cc: stable <[email protected]>
Signed-off-by: Bjørn Mork <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Else the poll will be restarted indefinitely in a tight loop,
preventing final device cleanup.
Cc: Oliver Neukum <[email protected]>
Cc: stable <[email protected]>
Signed-off-by: Bjørn Mork <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
This resolves the conflict with:
drivers/usb/host/ehci-tegra.c
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The internal error codes returned in the write() code
path cannot be simply passed on to user space.
Signed-off-by: Oliver Neukum <[email protected]>
Tested-by: Bjørn Mork <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|
|
Device state cleanup is done in either wdm_disconnect or
wdm_release depending on the order they are called. Adding
a couple of debug messages to document the program flow.
Signed-off-by: Bjørn Mork <[email protected]>
Acked-by: Oliver Neukum <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
|