2016-11-04 09:58:03

by Kai-Heng Feng

[permalink] [raw]
Subject: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

Sometimes cdc_mbim failed to probe if runtime pm is enabled:
[ 9.305626] cdc_mbim: probe of 2-2:1.12 failed with error -22

This can be solved by increase its pm usage counter.

Signed-off-by: Kai-Heng Feng <[email protected]>
---
drivers/net/usb/usbnet.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index d5071e3..f77b4bf 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1674,12 +1674,15 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
net->watchdog_timeo = TX_TIMEOUT_JIFFIES;
net->ethtool_ops = &usbnet_ethtool_ops;

+ if (usb_autopm_get_interface(dev->intf) < 0)
+ goto out1;
+
// allow device-specific bind/init procedures
// NOTE net->name still not usable ...
if (info->bind) {
status = info->bind (dev, udev);
if (status < 0)
- goto out1;
+ goto out2;

// heuristic: "usb%d" for links we know are two-host,
// else "eth%d" when there's reasonable doubt. userspace
@@ -1772,6 +1775,8 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
out3:
if (info->unbind)
info->unbind (dev, udev);
+out2:
+ usb_autopm_put_interface(dev->intf);
out1:
/* subdrivers must undo all they did in bind() if they
* fail it, but we may fail later and a deferred kevent
--
2.7.4


2016-11-04 13:26:23

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

On Fri, 4 Nov 2016, Kai-Heng Feng wrote:

> Sometimes cdc_mbim failed to probe if runtime pm is enabled:
> [ 9.305626] cdc_mbim: probe of 2-2:1.12 failed with error -22
>
> This can be solved by increase its pm usage counter.

This should not be needed. The USB core increments the PM usage
counter of a device before probing its interfaces.

Alan Stern

> Signed-off-by: Kai-Heng Feng <[email protected]>
> ---
> drivers/net/usb/usbnet.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index d5071e3..f77b4bf 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -1674,12 +1674,15 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
> net->watchdog_timeo = TX_TIMEOUT_JIFFIES;
> net->ethtool_ops = &usbnet_ethtool_ops;
>
> + if (usb_autopm_get_interface(dev->intf) < 0)
> + goto out1;
> +
> // allow device-specific bind/init procedures
> // NOTE net->name still not usable ...
> if (info->bind) {
> status = info->bind (dev, udev);
> if (status < 0)
> - goto out1;
> + goto out2;
>
> // heuristic: "usb%d" for links we know are two-host,
> // else "eth%d" when there's reasonable doubt. userspace
> @@ -1772,6 +1775,8 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
> out3:
> if (info->unbind)
> info->unbind (dev, udev);
> +out2:
> + usb_autopm_put_interface(dev->intf);
> out1:
> /* subdrivers must undo all they did in bind() if they
> * fail it, but we may fail later and a deferred kevent
>

2016-11-07 10:38:14

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

On Fri, 2016-11-04 at 09:26 -0400, Alan Stern wrote:
> On Fri, 4 Nov 2016, Kai-Heng Feng wrote:
>
> > Sometimes cdc_mbim failed to probe if runtime pm is enabled:
> > [ 9.305626] cdc_mbim: probe of 2-2:1.12 failed with error -22
> >
> > This can be solved by increase its pm usage counter.
>
> This should not be needed. The USB core increments the PM usage
> counter of a device before probing its interfaces.

Indeed. Yet we have experimental evidence.

Kai-Heng Feng, could you please enable dynamic debugging
for
drivers/usb/core/driver.c

so that we can see what is going on with the usage counters?

Regards
Oliver


2016-11-07 11:07:55

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

On Fri, 2016-11-04 at 17:57 +0800, Kai-Heng Feng wrote:
> Sometimes cdc_mbim failed to probe if runtime pm is enabled:
> [ 9.305626] cdc_mbim: probe of 2-2:1.12 failed with error -22
>
> This can be solved by increase its pm usage counter.
>
> Signed-off-by: Kai-Heng Feng <[email protected]>

For the record:

NAK. This fixes a symptom. If this patch helps something is broken in
device core. We need to find that.

Regards
Oliver


2016-11-08 07:46:08

by Kai-Heng Feng

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

Hi,

On Mon, Nov 7, 2016 at 7:02 PM, Oliver Neukum <[email protected]> wrote:
> On Fri, 2016-11-04 at 17:57 +0800, Kai-Heng Feng wrote:
>> Sometimes cdc_mbim failed to probe if runtime pm is enabled:
>> [ 9.305626] cdc_mbim: probe of 2-2:1.12 failed with error -22
>>
>> This can be solved by increase its pm usage counter.
>>
>> Signed-off-by: Kai-Heng Feng <[email protected]>
>
> For the record:
>
> NAK. This fixes a symptom. If this patch helps something is broken in
> device core. We need to find that.
>

Please check attached dmesg with usbcore.dyndbg="+p".

Thanks!

> Regards
> Oliver
>
>


Attachments:
dmesg (75.16 kB)

2016-11-08 15:25:31

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

On Tue, 8 Nov 2016, Kai-Heng Feng wrote:

> Hi,
>
> On Mon, Nov 7, 2016 at 7:02 PM, Oliver Neukum <[email protected]> wrote:
> > On Fri, 2016-11-04 at 17:57 +0800, Kai-Heng Feng wrote:
> >> Sometimes cdc_mbim failed to probe if runtime pm is enabled:
> >> [ 9.305626] cdc_mbim: probe of 2-2:1.12 failed with error -22
> >>
> >> This can be solved by increase its pm usage counter.
> >>
> >> Signed-off-by: Kai-Heng Feng <[email protected]>
> >
> > For the record:
> >
> > NAK. This fixes a symptom. If this patch helps something is broken in
> > device core. We need to find that.
> >
>
> Please check attached dmesg with usbcore.dyndbg="+p".

The log shows that the device went into suspend _before_ the cdc_mbim
driver was probed, not during the probe. Then just before the probe
was started, the USB core tried to resume the device and the resume
failed.

The log shows a bunch of other problems with this device:

[ 3.862253] usb 2-2: config 1 has an invalid interface number: 12 but max is 1
[ 3.862254] usb 2-2: config 1 has an invalid interface number: 13 but max is 1
[ 3.862254] usb 2-2: config 1 has an invalid interface number: 13 but max is 1
[ 3.862255] usb 2-2: config 1 has no interface number 0
[ 3.862256] usb 2-2: config 1 has no interface number 1
...
[ 8.295180] usb 2-2: Disable of device-initiated U1 failed.
[ 8.295322] usb 2-2: Disable of device-initiated U2 failed.

I get the impression that the device won't work properly with runtime
PM at all.

Alan Stern

2016-11-08 16:50:48

by Bjørn Mork

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

Alan Stern <[email protected]> writes:

> On Tue, 8 Nov 2016, Kai-Heng Feng wrote:
>
>> Hi,
>>
>> On Mon, Nov 7, 2016 at 7:02 PM, Oliver Neukum <[email protected]> wrote:
>> > On Fri, 2016-11-04 at 17:57 +0800, Kai-Heng Feng wrote:
>> >> Sometimes cdc_mbim failed to probe if runtime pm is enabled:
>> >> [ 9.305626] cdc_mbim: probe of 2-2:1.12 failed with error -22
>> >>
>> >> This can be solved by increase its pm usage counter.
>> >>
>> >> Signed-off-by: Kai-Heng Feng <[email protected]>
>> >
>> > For the record:
>> >
>> > NAK. This fixes a symptom. If this patch helps something is broken in
>> > device core. We need to find that.
>> >
>>
>> Please check attached dmesg with usbcore.dyndbg="+p".
>
> The log shows that the device went into suspend _before_ the cdc_mbim
> driver was probed, not during the probe. Then just before the probe
> was started, the USB core tried to resume the device and the resume
> failed.
>
> The log shows a bunch of other problems with this device:
>
> [ 3.862253] usb 2-2: config 1 has an invalid interface number: 12 but max is 1
> [ 3.862254] usb 2-2: config 1 has an invalid interface number: 13 but max is 1
> [ 3.862254] usb 2-2: config 1 has an invalid interface number: 13 but max is 1
> [ 3.862255] usb 2-2: config 1 has no interface number 0
> [ 3.862256] usb 2-2: config 1 has no interface number 1

These messages are completely harmless and normal for Sierra Wireless
devices. They use the interface number to identify the type of
function, causing this mismatch between the number of interfaces and the
inteface numbers. Boy, that looks weird in writing :)

Ref this discussion we had a few years ago:
http://www.spinics.net/lists/linux-usb/msg77499.html

No, I didn't expect you to remember that :)


> [ 8.295180] usb 2-2: Disable of device-initiated U1 failed.
> [ 8.295322] usb 2-2: Disable of device-initiated U2 failed.
>
> I get the impression that the device won't work properly with runtime
> PM at all.

I suspect the device is an EM7455? If so, then it does work fine with
runtime PM, as long as we're talking USB2. Not sure about USB3 runtime
PM though. Cannot test it. The Lenovo laptop I got with one of these
modems has disabled the USB3 link on the m.2 modem slot for some reason.


Bjørn

2016-11-08 18:44:11

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

On Tue, 8 Nov 2016, Bjørn Mork wrote:

> Alan Stern <[email protected]> writes:
>
> > On Tue, 8 Nov 2016, Kai-Heng Feng wrote:
> >
> >> Hi,
> >>
> >> On Mon, Nov 7, 2016 at 7:02 PM, Oliver Neukum <[email protected]> wrote:
> >> > On Fri, 2016-11-04 at 17:57 +0800, Kai-Heng Feng wrote:
> >> >> Sometimes cdc_mbim failed to probe if runtime pm is enabled:
> >> >> [ 9.305626] cdc_mbim: probe of 2-2:1.12 failed with error -22
> >> >>
> >> >> This can be solved by increase its pm usage counter.
> >> >>
> >> >> Signed-off-by: Kai-Heng Feng <[email protected]>
> >> >
> >> > For the record:
> >> >
> >> > NAK. This fixes a symptom. If this patch helps something is broken in
> >> > device core. We need to find that.
> >> >
> >>
> >> Please check attached dmesg with usbcore.dyndbg="+p".
> >
> > The log shows that the device went into suspend _before_ the cdc_mbim
> > driver was probed, not during the probe. Then just before the probe
> > was started, the USB core tried to resume the device and the resume
> > failed.
> >
> > The log shows a bunch of other problems with this device:
> >
> > [ 3.862253] usb 2-2: config 1 has an invalid interface number: 12 but max is 1
> > [ 3.862254] usb 2-2: config 1 has an invalid interface number: 13 but max is 1
> > [ 3.862254] usb 2-2: config 1 has an invalid interface number: 13 but max is 1
> > [ 3.862255] usb 2-2: config 1 has no interface number 0
> > [ 3.862256] usb 2-2: config 1 has no interface number 1
>
> These messages are completely harmless and normal for Sierra Wireless
> devices. They use the interface number to identify the type of
> function, causing this mismatch between the number of interfaces and the
> inteface numbers. Boy, that looks weird in writing :)
>
> Ref this discussion we had a few years ago:
> http://www.spinics.net/lists/linux-usb/msg77499.html
>
> No, I didn't expect you to remember that :)

You're right; I didn't remember it. But seeing those messages again in
the mailing list archives, they do look a little familiar.

> > [ 8.295180] usb 2-2: Disable of device-initiated U1 failed.
> > [ 8.295322] usb 2-2: Disable of device-initiated U2 failed.
> >
> > I get the impression that the device won't work properly with runtime
> > PM at all.
>
> I suspect the device is an EM7455? If so, then it does work fine with
> runtime PM, as long as we're talking USB2. Not sure about USB3 runtime
> PM though. Cannot test it. The Lenovo laptop I got with one of these
> modems has disabled the USB3 link on the m.2 modem slot for some reason.

These problems could very well be caused by running at SuperSpeed
(USB-3) instead of high speed (USB-2).

Is there any way to test what happens when the device is attached to
the computer by a USB-2 cable? That would prevent it from operating at
SuperSpeed.

The main point, however, is that the proposed patch doesn't seem to
address the true problem, which is that the device gets suspended
between probes. The patch only tries to prevent it from being
suspended during a probe -- which is already prevented by the USB core.

Alan Stern

2016-11-09 12:04:47

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote:

> These problems could very well be caused by running at SuperSpeed
> (USB-3) instead of high speed (USB-2).
>
> Is there any way to test what happens when the device is attached to
> the computer by a USB-2 cable? That would prevent it from operating at
> SuperSpeed.
>
> The main point, however, is that the proposed patch doesn't seem to
> address the true problem, which is that the device gets suspended
> between probes. The patch only tries to prevent it from being
> suspended during a probe -- which is already prevented by the USB core.

But why doesn't it fail during normal operation?

I suspect that its firmware requires the altsetting

/* should we change control altsetting on a NCM/MBIM function? */
if (cdc_ncm_select_altsetting(intf) == CDC_NCM_COMM_ALTSETTING_MBIM) {
data_altsetting = CDC_NCM_DATA_ALTSETTING_MBIM;
ret = cdc_mbim_set_ctrlalt(dev, intf, CDC_NCM_COMM_ALTSETTING_MBIM);

to be set before it accepts a suspension.

Regards
Oliver


2016-11-09 12:32:31

by Bjørn Mork

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

Oliver Neukum <[email protected]> writes:

> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote:
>
>> These problems could very well be caused by running at SuperSpeed
>> (USB-3) instead of high speed (USB-2).
>>
>> Is there any way to test what happens when the device is attached to
>> the computer by a USB-2 cable? That would prevent it from operating at
>> SuperSpeed.
>>
>> The main point, however, is that the proposed patch doesn't seem to
>> address the true problem, which is that the device gets suspended
>> between probes. The patch only tries to prevent it from being
>> suspended during a probe -- which is already prevented by the USB core.
>
> But why doesn't it fail during normal operation?
>
> I suspect that its firmware requires the altsetting
>
> /* should we change control altsetting on a NCM/MBIM function? */
> if (cdc_ncm_select_altsetting(intf) == CDC_NCM_COMM_ALTSETTING_MBIM) {
> data_altsetting = CDC_NCM_DATA_ALTSETTING_MBIM;
> ret = cdc_mbim_set_ctrlalt(dev, intf, CDC_NCM_COMM_ALTSETTING_MBIM);
>
> to be set before it accepts a suspension.

Could be, but I don't think so. The above code is effectively a noop
unless the function is a combined NCM/MBIM function. Something I've
never seen on a Sierra Wireless device (ignoring the infamous EM7345,
which really is an Intel device).

This is a typical example of a Sierra Wireless modem configured for
MBIM:

P: Vendor=1199 ProdID=9079 Rev= 0.06
S: Manufacturer=Sierra Wireless, Incorporated
S: Product=Sierra Wireless EM7455 Qualcomm Snapdragon X7 LTE-A
S: SerialNumber=LF615126xxxxxxx
C:* #Ifs= 2 Cfg#= 1 Atr=a0 MxPwr=500mA
A: FirstIf#=12 IfCount= 2 Cls=02(comm.) Sub=0e Prot=00
I:* If#=12 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=0e Prot=00 Driver=(none)
E: Ad=82(I) Atr=03(Int.) MxPS= 64 Ivl=32ms
I:* If#=13 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=02 Driver=(none)
I: If#=13 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=(none)
E: Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms


The control interface of plain MBIM functions will always have a single
altsetting, like the example above. So cdc_ncm_select_altsetting(intf)
returns "0", while CDC_NCM_COMM_ALTSETTING_MBIM is "1".


Just for reference, using the Intel^H^H^H^H^HEM7345 as example, this is
what a combined NCM/MBIM function looks like:


P: Vendor=1199 ProdID=a001 Rev=17.29
S: Manufacturer=Sierra Wireless Inc.
S: Product=Sierra Wireless EM7345 4G LTE
S: SerialNumber=013937000xxxxxx
C:* #Ifs= 4 Cfg#= 1 Atr=e0 MxPwr=100mA
A: FirstIf#= 0 IfCount= 2 Cls=02(comm.) Sub=0d Prot=00
A: FirstIf#= 2 IfCount= 2 Cls=02(comm.) Sub=02 Prot=01
I: If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=0d Prot=00 Driver=cdc_mbim
E: Ad=81(I) Atr=03(Int.) MxPS= 64 Ivl=1ms
I:* If#= 0 Alt= 1 #EPs= 1 Cls=02(comm.) Sub=0e Prot=00 Driver=cdc_mbim
E: Ad=81(I) Atr=03(Int.) MxPS= 64 Ivl=1ms
I: If#= 1 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_mbim
I: If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_mbim
E: Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#= 1 Alt= 2 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
E: Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#= 2 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=02 Prot=01 Driver=(none)
E: Ad=83(I) Atr=03(Int.) MxPS= 64 Ivl=1ms
I:* If#= 3 Alt= 0 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=(none)
E: Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms


And this is what the code you quote is trying to deal with. Note the
different subclass of altsetting 0 and 1.... This is incredibly ugly.

FWIW, the modem in question cannot be an EM7345. That modem does not
have the static interface numbering oddity. Another sign that it isn't
a true Sierra device.




Bjørn

2016-11-10 04:06:35

by Kai-Heng Feng

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

Hi,

On Wed, Nov 9, 2016 at 8:32 PM, Bjørn Mork <[email protected]> wrote:
> Oliver Neukum <[email protected]> writes:
>
>> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote:
>>
>>> These problems could very well be caused by running at SuperSpeed
>>> (USB-3) instead of high speed (USB-2).

Yes, it's running at SuperSpeed, on a Kabylake laptop.

It does not have this issue on a Broadwell laptop, also running at SuperSpeed.

>>>
>>> Is there any way to test what happens when the device is attached to
>>> the computer by a USB-2 cable? That would prevent it from operating at
>>> SuperSpeed.

I recall old Intel PCH can change the USB host from XHCI to EHCI,
newer PCH does not have this option.

Is there a way to force XHCI run at HighSpeed?

>>>
>>> The main point, however, is that the proposed patch doesn't seem to
>>> address the true problem, which is that the device gets suspended
>>> between probes. The patch only tries to prevent it from being
>>> suspended during a probe -- which is already prevented by the USB core.
>>
>> But why doesn't it fail during normal operation?
>>
>> I suspect that its firmware requires the altsetting
>>
>> /* should we change control altsetting on a NCM/MBIM function? */
>> if (cdc_ncm_select_altsetting(intf) == CDC_NCM_COMM_ALTSETTING_MBIM) {
>> data_altsetting = CDC_NCM_DATA_ALTSETTING_MBIM;
>> ret = cdc_mbim_set_ctrlalt(dev, intf, CDC_NCM_COMM_ALTSETTING_MBIM);
>>
>> to be set before it accepts a suspension.
>
> Could be, but I don't think so. The above code is effectively a noop
> unless the function is a combined NCM/MBIM function. Something I've
> never seen on a Sierra Wireless device (ignoring the infamous EM7345,
> which really is an Intel device).
>
> This is a typical example of a Sierra Wireless modem configured for
> MBIM:
>
> P: Vendor=1199 ProdID=9079 Rev= 0.06
> S: Manufacturer=Sierra Wireless, Incorporated
> S: Product=Sierra Wireless EM7455 Qualcomm Snapdragon X7 LTE-A
> S: SerialNumber=LF615126xxxxxxx
> C:* #Ifs= 2 Cfg#= 1 Atr=a0 MxPwr=500mA
> A: FirstIf#=12 IfCount= 2 Cls=02(comm.) Sub=0e Prot=00
> I:* If#=12 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=0e Prot=00 Driver=(none)
> E: Ad=82(I) Atr=03(Int.) MxPS= 64 Ivl=32ms
> I:* If#=13 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=02 Driver=(none)
> I: If#=13 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=(none)
> E: Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E: Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
>
>
> The control interface of plain MBIM functions will always have a single
> altsetting, like the example above. So cdc_ncm_select_altsetting(intf)
> returns "0", while CDC_NCM_COMM_ALTSETTING_MBIM is "1".
>
>
> Just for reference, using the Intel^H^H^H^H^HEM7345 as example, this is
> what a combined NCM/MBIM function looks like:
>
>
> P: Vendor=1199 ProdID=a001 Rev=17.29
> S: Manufacturer=Sierra Wireless Inc.
> S: Product=Sierra Wireless EM7345 4G LTE
> S: SerialNumber=013937000xxxxxx
> C:* #Ifs= 4 Cfg#= 1 Atr=e0 MxPwr=100mA
> A: FirstIf#= 0 IfCount= 2 Cls=02(comm.) Sub=0d Prot=00
> A: FirstIf#= 2 IfCount= 2 Cls=02(comm.) Sub=02 Prot=01
> I: If#= 0 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=0d Prot=00 Driver=cdc_mbim
> E: Ad=81(I) Atr=03(Int.) MxPS= 64 Ivl=1ms
> I:* If#= 0 Alt= 1 #EPs= 1 Cls=02(comm.) Sub=0e Prot=00 Driver=cdc_mbim
> E: Ad=81(I) Atr=03(Int.) MxPS= 64 Ivl=1ms
> I: If#= 1 Alt= 0 #EPs= 0 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_mbim
> I: If#= 1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=01 Driver=cdc_mbim
> E: Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E: Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> I:* If#= 1 Alt= 2 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
> E: Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E: Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> I:* If#= 2 Alt= 0 #EPs= 1 Cls=02(comm.) Sub=02 Prot=01 Driver=(none)
> E: Ad=83(I) Atr=03(Int.) MxPS= 64 Ivl=1ms
> I:* If#= 3 Alt= 0 #EPs= 2 Cls=0a(data ) Sub=00 Prot=00 Driver=(none)
> E: Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E: Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
>
>
> And this is what the code you quote is trying to deal with. Note the
> different subclass of altsetting 0 and 1.... This is incredibly ugly.
>
> FWIW, the modem in question cannot be an EM7345. That modem does not
> have the static interface numbering oddity. Another sign that it isn't
> a true Sierra device.

Yes, this modem is an EM7445.

>
>
>
>
> Bjørn

2016-11-10 11:11:19

by Bjørn Mork

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

Kai-Heng Feng <[email protected]> writes:
> On Wed, Nov 9, 2016 at 8:32 PM, Bjørn Mork <[email protected]> wrote:
>> Oliver Neukum <[email protected]> writes:
>>
>>> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote:
>>>
>>>> These problems could very well be caused by running at SuperSpeed
>>>> (USB-3) instead of high speed (USB-2).
>
> Yes, it's running at SuperSpeed, on a Kabylake laptop.
>
> It does not have this issue on a Broadwell laptop, also running at SuperSpeed.

Then I must join Oliver, being very surprised by where in the stack you
attempt to fix the issue. What you write above indicates a problem in
pci bridge or usb host controller, doesn't it?


Bjørn

2016-11-10 11:28:29

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

On Thu, 2016-11-10 at 12:09 +0100, Bjørn Mork wrote:
> Kai-Heng Feng <[email protected]> writes:
> > On Wed, Nov 9, 2016 at 8:32 PM, Bjørn Mork <[email protected]> wrote:
> >> Oliver Neukum <[email protected]> writes:
> >>
> >>> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote:
> >>>
> >>>> These problems could very well be caused by running at SuperSpeed
> >>>> (USB-3) instead of high speed (USB-2).
> >
> > Yes, it's running at SuperSpeed, on a Kabylake laptop.
> >
> > It does not have this issue on a Broadwell laptop, also running at SuperSpeed.
>
> Then I must join Oliver, being very surprised by where in the stack you
> attempt to fix the issue. What you write above indicates a problem in
> pci bridge or usb host controller, doesn't it?

Indeed. And this means we need an XHCI specialist.
Mathias, we have a failure specific to one implementation of XHCI.

Regards
Oliver


2016-11-10 15:36:22

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

On Thu, 10 Nov 2016, Kai-Heng Feng wrote:

> Hi,
>
> On Wed, Nov 9, 2016 at 8:32 PM, Bjørn Mork <[email protected]> wrote:
> > Oliver Neukum <[email protected]> writes:
> >
> >> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote:
> >>
> >>> These problems could very well be caused by running at SuperSpeed
> >>> (USB-3) instead of high speed (USB-2).
>
> Yes, it's running at SuperSpeed, on a Kabylake laptop.
>
> It does not have this issue on a Broadwell laptop, also running at SuperSpeed.
>
> >>>
> >>> Is there any way to test what happens when the device is attached to
> >>> the computer by a USB-2 cable? That would prevent it from operating at
> >>> SuperSpeed.
>
> I recall old Intel PCH can change the USB host from XHCI to EHCI,
> newer PCH does not have this option.
>
> Is there a way to force XHCI run at HighSpeed?

Yes, like I said above: Use a USB-2 cable instead of a USB-3 cable.

Alan Stern

2016-11-10 20:39:19

by Bjørn Mork

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

Alan Stern <[email protected]> writes:
> On Thu, 10 Nov 2016, Kai-Heng Feng wrote:
>
>> Is there a way to force XHCI run at HighSpeed?
>
> Yes, like I said above: Use a USB-2 cable instead of a USB-3 cable.

It's an m.2 form factor modem, so there most likely isn't any cable
involved. But the principle is the same: Cover the USB3 diff pair pins
with a piece of tape.



Bjørn

2016-11-11 14:44:20

by Mathias Nyman

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

On 10.11.2016 13:22, Oliver Neukum wrote:
> On Thu, 2016-11-10 at 12:09 +0100, Bjørn Mork wrote:
>> Kai-Heng Feng <[email protected]> writes:
>>> On Wed, Nov 9, 2016 at 8:32 PM, Bjørn Mork <[email protected]> wrote:
>>>> Oliver Neukum <[email protected]> writes:
>>>>
>>>>> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote:
>>>>>
>>>>>> These problems could very well be caused by running at SuperSpeed
>>>>>> (USB-3) instead of high speed (USB-2).
>>>
>>> Yes, it's running at SuperSpeed, on a Kabylake laptop.
>>>
>>> It does not have this issue on a Broadwell laptop, also running at SuperSpeed.
>>
>> Then I must join Oliver, being very surprised by where in the stack you
>> attempt to fix the issue. What you write above indicates a problem in
>> pci bridge or usb host controller, doesn't it?
>
> Indeed. And this means we need an XHCI specialist.
> Mathias, we have a failure specific to one implementation of XHCI.
>


Could be related to resume singnalling time.
Does the xhci fix for it in 4.9-rc3 help?

commit 7d3b016a6f5a0fa610dfd02b05654c08fa4ae514
xhci: use default USB_RESUME_TIMEOUT when resuming ports.

It doesn't directly explain why it would work on Broadwell but not Kabylake,
but it resolved very similar cases.

If not, then adding dynamic debug for xhci could show something.

-Mathias

2016-11-14 07:34:36

by Kai-Heng Feng

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

On Fri, Nov 11, 2016 at 10:44 PM, Mathias Nyman
<[email protected]> wrote:
> On 10.11.2016 13:22, Oliver Neukum wrote:
>>
>> On Thu, 2016-11-10 at 12:09 +0100, Bjørn Mork wrote:
>>>
>>> Kai-Heng Feng <[email protected]> writes:
>>>>
>>>> On Wed, Nov 9, 2016 at 8:32 PM, Bjørn Mork <[email protected]> wrote:
>>>>>
>>>>> Oliver Neukum <[email protected]> writes:
>>>>>
>>>>>> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote:
>>>>>>
>>>>>>> These problems could very well be caused by running at SuperSpeed
>>>>>>> (USB-3) instead of high speed (USB-2).
>>>>
>>>>
>>>> Yes, it's running at SuperSpeed, on a Kabylake laptop.
>>>>
>>>> It does not have this issue on a Broadwell laptop, also running at
>>>> SuperSpeed.
>>>
>>>
>>> Then I must join Oliver, being very surprised by where in the stack you
>>> attempt to fix the issue. What you write above indicates a problem in
>>> pci bridge or usb host controller, doesn't it?

Yes, I was totally wrong about that.

>>
>>
>> Indeed. And this means we need an XHCI specialist.
>> Mathias, we have a failure specific to one implementation of XHCI.
>>
>
>
> Could be related to resume singnalling time.
> Does the xhci fix for it in 4.9-rc3 help?
>
> commit 7d3b016a6f5a0fa610dfd02b05654c08fa4ae514
> xhci: use default USB_RESUME_TIMEOUT when resuming ports.
>
> It doesn't directly explain why it would work on Broadwell but not Kabylake,
> but it resolved very similar cases.
>
> If not, then adding dynamic debug for xhci could show something.

I tried the latest commit, 6005a545cadb2adca64350c7aee17d002563e8c7,
on for-usb-next branch.

Now the cdc_mbim still probe failed at the first time, but somehow it
re-probed again with a success.

I reverted commit 7d3b016a6f5a0fa610dfd02b05654c08fa4ae514 and the
behavior is the same, first time probed failed, second time probed
success.

The attached dmesg is with usbcore and xhci_hcd dynamic debug enabled.

>
> -Mathias
>


Attachments:
dmesg (178.13 kB)

2016-11-16 10:29:35

by Kai-Heng Feng

[permalink] [raw]
Subject: Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function

On Mon, Nov 14, 2016 at 3:34 PM, Kai-Heng Feng
<[email protected]> wrote:
> On Fri, Nov 11, 2016 at 10:44 PM, Mathias Nyman
> <[email protected]> wrote:
>> On 10.11.2016 13:22, Oliver Neukum wrote:
>>>
>>> On Thu, 2016-11-10 at 12:09 +0100, Bjørn Mork wrote:
>>>>
>>>> Kai-Heng Feng <[email protected]> writes:
>>>>>
>>>>> On Wed, Nov 9, 2016 at 8:32 PM, Bjørn Mork <[email protected]> wrote:
>>>>>>
>>>>>> Oliver Neukum <[email protected]> writes:
>>>>>>
>>>>>>> On Tue, 2016-11-08 at 13:44 -0500, Alan Stern wrote:
>>>>>>>
>>>>>>>> These problems could very well be caused by running at SuperSpeed
>>>>>>>> (USB-3) instead of high speed (USB-2).
>>>>>
>>>>>
>>>>> Yes, it's running at SuperSpeed, on a Kabylake laptop.
>>>>>
>>>>> It does not have this issue on a Broadwell laptop, also running at
>>>>> SuperSpeed.
>>>>
>>>>
>>>> Then I must join Oliver, being very surprised by where in the stack you
>>>> attempt to fix the issue. What you write above indicates a problem in
>>>> pci bridge or usb host controller, doesn't it?
>
> Yes, I was totally wrong about that.
>
>>>
>>>
>>> Indeed. And this means we need an XHCI specialist.
>>> Mathias, we have a failure specific to one implementation of XHCI.
>>>
>>
>>
>> Could be related to resume singnalling time.
>> Does the xhci fix for it in 4.9-rc3 help?
>>
>> commit 7d3b016a6f5a0fa610dfd02b05654c08fa4ae514
>> xhci: use default USB_RESUME_TIMEOUT when resuming ports.
>>
>> It doesn't directly explain why it would work on Broadwell but not Kabylake,
>> but it resolved very similar cases.
>>
>> If not, then adding dynamic debug for xhci could show something.
>
> I tried the latest commit, 6005a545cadb2adca64350c7aee17d002563e8c7,
> on for-usb-next branch.
>
> Now the cdc_mbim still probe failed at the first time, but somehow it
> re-probed again with a success.
>
> I reverted commit 7d3b016a6f5a0fa610dfd02b05654c08fa4ae514 and the
> behavior is the same, first time probed failed, second time probed
> success.
>
> The attached dmesg is with usbcore and xhci_hcd dynamic debug enabled.

I filed a bug report on bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=187861

>
>>
>> -Mathias
>>