Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1518680

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

From Kai-Heng Feng <kai.heng.feng@canonical.com>
Newsgroups linux.kernel
Subject Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function
Date 2016-11-10 05:10 +0100
Message-ID <sBOQx-om-1@gated-at.bofh.it> (permalink)
References <sBjD3-4ES-7@gated-at.bofh.it> <sBzRw-77T-7@gated-at.bofh.it> <sBAkx-7jO-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Hi,

On Wed, Nov 9, 2016 at 8:32 PM, Bjørn Mork <bjorn@mork.no> wrote:
> Oliver Neukum <oneukum@suse.com> 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

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe  function Alan Stern <stern@rowland.harvard.edu> - 2016-11-08 16:30 +0100
  Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function Bjørn Mork <bjorn@mork.no> - 2016-11-08 18:00 +0100
    Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe  function Alan Stern <stern@rowland.harvard.edu> - 2016-11-08 19:50 +0100
      Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe  function Oliver Neukum <oneukum@suse.com> - 2016-11-09 13:10 +0100
        Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function Bjørn Mork <bjorn@mork.no> - 2016-11-09 13:40 +0100
          Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function Kai-Heng Feng <kai.heng.feng@canonical.com> - 2016-11-10 05:10 +0100
            Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function Bjørn Mork <bjorn@mork.no> - 2016-11-10 12:20 +0100
              Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe  function Oliver Neukum <oneukum@suse.com> - 2016-11-10 12:30 +0100
                Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe  function Mathias Nyman <mathias.nyman@linux.intel.com> - 2016-11-11 15:50 +0100
            Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe  function Alan Stern <stern@rowland.harvard.edu> - 2016-11-10 16:40 +0100
              Re: [PATCH] usbnet: prevent device rpm suspend in usbnet_probe function Bjørn Mork <bjorn@mork.no> - 2016-11-10 21:40 +0100

csiph-web