On Fri, 2017-09-22 at 22:18 +0200, Bjørn Mork wrote: > 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: <stable@xxxxxxxxxxxxxxx> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100938 > Reported-and-tested-by: Christian Ehrig <christian.ehrig@mediamarktsa > turn-bt.com> > Reported-and-tested-by: Patrick Chilton <chpatrick@xxxxxxxxx> > Reported-and-tested-by: Andreas Böhler <news@xxxxxxxxxxx> > Signed-off-by: Bjørn Mork <bjorn@xxxxxxx> > --- > The fallout of this problem has been reported by a number of people > for a long time. Many more than those being listed above. Apologies > to all who didn't get the appropriate credit. Sorry, I am lousy with > paper work and lost track of you all. > > Hoping the fix will make up for it... FWIW, tested on the oldest MBIM devices I have (Ericsson F5321 and Huawei EM820W for while with light traffic, browsing and git pull/push. Neither of these devices exhibit the original problem of course. Might find time to test on the newer ones this week or next, but it seems OK. Dan > > Bjørn > > drivers/usb/class/cdc-wdm.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc- > wdm.c > index 0b845e550fbd..9f001659807a 100644 > --- a/drivers/usb/class/cdc-wdm.c > +++ b/drivers/usb/class/cdc-wdm.c > @@ -194,8 +194,10 @@ static void wdm_in_callback(struct urb *urb) > /* > * only set a new error if there is no previous error. > * Errors are only cleared during read/open > + * Avoid propagating -EPIPE (stall) to userspace since it is > + * better handled as an empty read > */ > - if (desc->rerr == 0) > + if (desc->rerr == 0 && status != -EPIPE) > desc->rerr = status; > > if (length + desc->length > desc->wMaxCommand) { -- 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
