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. Cc: <stable@xxxxxxxxxxxxxxx> Signed-off-by: Bjørn Mork <bjorn@xxxxxxx> --- Uhm, well, time flies when you are having fun :) For those who cannot remember more than a month worth of list contents, and therefore cannot understand whyt this is "v2", please look at the thread here: http://www.spinics.net/lists/linux-usb/msg140856.html FWIW, I have been using this patch actively myself for all that time, mostly with a QMI modem. But also occasionally with the same modem in MBIM mode and with assorted other modems. I have also verified that this has no effect on any of the "proper" CDC WDM devices I have. v2 changes are: - avoid propagating any EPIPE caused by polling for data without notification. This is not a problem for standards complying CDC WDM devices, as they are required to support such polling, but proved to cause problems for some QMI devices. I hope this is acceptable for stable too, as it fixes a long standing usability problems with many MBIM devices. drivers/usb/class/cdc-wdm.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 61ea87917433..43e1cfb361f3 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -178,7 +178,7 @@ static void wdm_in_callback(struct urb *urb) "nonzero urb status received: -ESHUTDOWN"); goto skip_error; case -EPIPE: - dev_err(&desc->intf->dev, + dev_dbg(&desc->intf->dev, "nonzero urb status received: -EPIPE\n"); break; default: @@ -200,10 +200,29 @@ static void wdm_in_callback(struct urb *urb) desc->reslength = length; } } + + /* + * If desc->resp_count is unset, then the urb was submitted + * without a prior notification. If the device returned any + * data, then this implies that it had messages queued without + * notifying us. Continue reading until that queue is flushed. + */ + if (!desc->resp_count) { + if (!length) { + /* do not propagate the expected -EPIPE */ + desc->rerr = 0; + goto unlock; + } + dev_dbg(&desc->intf->dev, "got %d bytes without notification\n", length); + set_bit(WDM_RESPONDING, &desc->flags); + usb_submit_urb(desc->response, GFP_ATOMIC); + } + skip_error: wake_up(&desc->wait); set_bit(WDM_READ, &desc->flags); +unlock: spin_unlock(&desc->iuspin); } @@ -647,6 +666,16 @@ static int wdm_open(struct inode *inode, struct file *file) dev_err(&desc->intf->dev, "Error submitting int urb - %d\n", rv); rv = usb_translate_errors(rv); + } else { + /* + * Some devices keep pending messages queued + * without resending notifications. We must + * flush the message queue before we can + * assume a one-to-one relationship between + * notifications and messages in the queue + */ + set_bit(WDM_RESPONDING, &desc->flags); + rv = usb_submit_urb(desc->response, GFP_KERNEL); } } else { rv = 0; -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
