linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: usb: cdc_mbim: adding Microsoft mobile broadband modem
@ 2022-08-03  9:02 Seunghun Han
  2022-08-03 10:59 ` Daniele Palmas
  2022-10-12  4:28 ` Seunghun Han
  0 siblings, 2 replies; 6+ messages in thread
From: Seunghun Han @ 2022-08-03  9:02 UTC (permalink / raw)
  To: oliver; +Cc: davem, linux-usb, linux-kernel, kkamagui

Microsoft branded mobile broadband modems typically used in the Surface series
don't work with the Linux kernel. When I query firmware information to the
modem using the mbimcli tool, it always returns the Failure (0x02) value like
below.

=== Start of the firmware id query ===
$> mbimcli -d /dev/cdc-wdm4 --ms-query-firmware-id -v
[26  Jul 2022, 05:24:07] [Debug] opening device...
[26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Queried max control message size: 4096
[26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Sent message...
<<<<<< RAW:
<<<<<<   length = 16
<<<<<<   data   = 01:00:00:00:10:00:00:00:01:00:00:00:00:10:00:00

[26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Sent message (translated)...
<<<<<< Header:
<<<<<<   length      = 16
<<<<<<   type        = open (0x00000001)
<<<<<<   transaction = 1
<<<<<< Contents:
<<<<<<   max control transfer = 4096

[26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Received message...
>>>>>> RAW:
>>>>>>   length = 16
>>>>>>   data   = 01:00:00:80:10:00:00:00:01:00:00:00:02:00:00:00

[26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Received message...Message Type 80000001
[26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 864
[26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 897
[26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 923
[26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 930
[26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 935
[26  Jul 2022, 05:24:07] [Debug] getting open done result failed: closed
error: couldn't open the MbimDevice: Failure
=== End of the firmware id query ===

After kernel debugging, I found that the modem reported that the dwNtbInMaxSize
value of ncm_parm was 16384 during the initialization sequence.
So the cdc_ncm_update_rxtx_max() in cdc_ncm_bind_common() didn't send
USB_CDC_SET_NTB_INPUT_SIZE command because the default input size was the same,
16384.

It's good and proper behavior. However, Microsoft branded MBMs (including the
latest one in Surface Go 3) fail until the kernel explicitly sets the input
size.

This patch adds a new table and code changes that explicitly send
the USB_CDC_SET_NTB_INPUT_SIZE command to support Microsoft branded MBMs.

Signed-off-by: Seunghun Han <kkamagui@gmail.com>
---
 drivers/net/usb/cdc_mbim.c  | 24 ++++++++++++++++++++++++
 drivers/net/usb/cdc_ncm.c   |  2 +-
 include/linux/usb/cdc_ncm.h |  1 +
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index c89639381eca..c0c23cfc02a7 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -618,6 +618,22 @@ static const struct driver_info cdc_mbim_info_avoid_altsetting_toggle = {
 	.data = CDC_MBIM_FLAG_AVOID_ALTSETTING_TOGGLE,
 };
 
+/* Microsoft branded modems do not work properly without setting the input size
+ * explicitly in cdc_ncm_bind_common.
+ * CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY flag is used to set the input size
+ * during initialization.
+ */
+static const struct driver_info cdc_mbim_info_set_input_size_explicitly = {
+	.description = "CDC MBIM",
+	.flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN | FLAG_SEND_ZLP,
+	.bind = cdc_mbim_bind,
+	.unbind = cdc_mbim_unbind,
+	.manage_power = cdc_mbim_manage_power,
+	.rx_fixup = cdc_mbim_rx_fixup,
+	.tx_fixup = cdc_mbim_tx_fixup,
+	.data = CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY,
+};
+
 static const struct usb_device_id mbim_devs[] = {
 	/* This duplicate NCM entry is intentional. MBIM devices can
 	 * be disguised as NCM by default, and this is necessary to
@@ -665,6 +681,14 @@ static const struct usb_device_id mbim_devs[] = {
 	  .driver_info = (unsigned long)&cdc_mbim_info_avoid_altsetting_toggle,
 	},
 
+	/* Some Microsoft branded mobile broadband modems used in the Surface
+	 * seires are known to fail unless the input size is set explicitly.
+	 * Applying it to all Microsoft branded MBMs.
+	 */
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x045e, USB_CLASS_COMM, USB_CDC_SUBCLASS_MBIM, USB_CDC_PROTO_NONE),
+	  .driver_info = (unsigned long)&cdc_mbim_info_set_input_size_explicitly,
+	},
+
 	/* default entry */
 	{ USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_MBIM, USB_CDC_PROTO_NONE),
 	  .driver_info = (unsigned long)&cdc_mbim_info_zlp,
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 8d5cbda33f66..915e29c987cb 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -407,7 +407,7 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
 	val = cdc_ncm_check_rx_max(dev, new_rx);
 
 	/* inform device about NTB input size changes */
-	if (val != ctx->rx_max) {
+	if (val != ctx->rx_max || ctx->drvflags & CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY) {
 		__le32 dwNtbInMaxSize = cpu_to_le32(val);
 
 		dev_info(&dev->intf->dev, "setting rx_max = %u\n", val);
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 2d207cb4837d..a24f84b31a54 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -88,6 +88,7 @@
 #define CDC_NCM_FLAG_NDP_TO_END			0x02	/* NDP is placed at end of frame */
 #define CDC_MBIM_FLAG_AVOID_ALTSETTING_TOGGLE	0x04	/* Avoid altsetting toggle during init */
 #define CDC_NCM_FLAG_PREFER_NTB32 0x08	/* prefer NDP32 over NDP16 */
+#define CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY	0x10	/* Set input size explicitly during init */
 
 #define cdc_ncm_comm_intf_is_mbim(x)  ((x)->desc.bInterfaceSubClass == USB_CDC_SUBCLASS_MBIM && \
 				       (x)->desc.bInterfaceProtocol == USB_CDC_PROTO_NONE)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: usb: cdc_mbim: adding Microsoft mobile broadband modem
  2022-08-03  9:02 [PATCH] net: usb: cdc_mbim: adding Microsoft mobile broadband modem Seunghun Han
@ 2022-08-03 10:59 ` Daniele Palmas
  2022-08-03 12:55   ` Seunghun Han
  2022-10-12  4:28 ` Seunghun Han
  1 sibling, 1 reply; 6+ messages in thread
From: Daniele Palmas @ 2022-08-03 10:59 UTC (permalink / raw)
  To: Seunghun Han; +Cc: Oliver Neukum, David Miller, linux-usb, linux-kernel

Hello Seunghun,

Il giorno mer 3 ago 2022 alle ore 11:10 Seunghun Han
<kkamagui@gmail.com> ha scritto:
>
> Microsoft branded mobile broadband modems typically used in the Surface series
> don't work with the Linux kernel. When I query firmware information to the
> modem using the mbimcli tool, it always returns the Failure (0x02) value like
> below.
>
> === Start of the firmware id query ===
> $> mbimcli -d /dev/cdc-wdm4 --ms-query-firmware-id -v
> [26  Jul 2022, 05:24:07] [Debug] opening device...
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Queried max control message size: 4096
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Sent message...
> <<<<<< RAW:
> <<<<<<   length = 16
> <<<<<<   data   = 01:00:00:00:10:00:00:00:01:00:00:00:00:10:00:00
>
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Sent message (translated)...
> <<<<<< Header:
> <<<<<<   length      = 16
> <<<<<<   type        = open (0x00000001)
> <<<<<<   transaction = 1
> <<<<<< Contents:
> <<<<<<   max control transfer = 4096
>
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Received message...
> >>>>>> RAW:
> >>>>>>   length = 16
> >>>>>>   data   = 01:00:00:80:10:00:00:00:01:00:00:00:02:00:00:00
>
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Received message...Message Type 80000001
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 864
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 897
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 923
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 930
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 935
> [26  Jul 2022, 05:24:07] [Debug] getting open done result failed: closed
> error: couldn't open the MbimDevice: Failure
> === End of the firmware id query ===
>
> After kernel debugging, I found that the modem reported that the dwNtbInMaxSize
> value of ncm_parm was 16384 during the initialization sequence.
> So the cdc_ncm_update_rxtx_max() in cdc_ncm_bind_common() didn't send
> USB_CDC_SET_NTB_INPUT_SIZE command because the default input size was the same,
> 16384.
>
> It's good and proper behavior. However, Microsoft branded MBMs (including the
> latest one in Surface Go 3) fail until the kernel explicitly sets the input
> size.
>
> This patch adds a new table and code changes that explicitly send
> the USB_CDC_SET_NTB_INPUT_SIZE command to support Microsoft branded MBMs.
>

Just for reference, are you allowed to disclose which chipsets these
modems are based on?

Thanks,
Daniele

> Signed-off-by: Seunghun Han <kkamagui@gmail.com>
> ---
>  drivers/net/usb/cdc_mbim.c  | 24 ++++++++++++++++++++++++
>  drivers/net/usb/cdc_ncm.c   |  2 +-
>  include/linux/usb/cdc_ncm.h |  1 +
>  3 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
> index c89639381eca..c0c23cfc02a7 100644
> --- a/drivers/net/usb/cdc_mbim.c
> +++ b/drivers/net/usb/cdc_mbim.c
> @@ -618,6 +618,22 @@ static const struct driver_info cdc_mbim_info_avoid_altsetting_toggle = {
>         .data = CDC_MBIM_FLAG_AVOID_ALTSETTING_TOGGLE,
>  };
>
> +/* Microsoft branded modems do not work properly without setting the input size
> + * explicitly in cdc_ncm_bind_common.
> + * CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY flag is used to set the input size
> + * during initialization.
> + */
> +static const struct driver_info cdc_mbim_info_set_input_size_explicitly = {
> +       .description = "CDC MBIM",
> +       .flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN | FLAG_SEND_ZLP,
> +       .bind = cdc_mbim_bind,
> +       .unbind = cdc_mbim_unbind,
> +       .manage_power = cdc_mbim_manage_power,
> +       .rx_fixup = cdc_mbim_rx_fixup,
> +       .tx_fixup = cdc_mbim_tx_fixup,
> +       .data = CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY,
> +};
> +
>  static const struct usb_device_id mbim_devs[] = {
>         /* This duplicate NCM entry is intentional. MBIM devices can
>          * be disguised as NCM by default, and this is necessary to
> @@ -665,6 +681,14 @@ static const struct usb_device_id mbim_devs[] = {
>           .driver_info = (unsigned long)&cdc_mbim_info_avoid_altsetting_toggle,
>         },
>
> +       /* Some Microsoft branded mobile broadband modems used in the Surface
> +        * seires are known to fail unless the input size is set explicitly.
> +        * Applying it to all Microsoft branded MBMs.
> +        */
> +       { USB_VENDOR_AND_INTERFACE_INFO(0x045e, USB_CLASS_COMM, USB_CDC_SUBCLASS_MBIM, USB_CDC_PROTO_NONE),
> +         .driver_info = (unsigned long)&cdc_mbim_info_set_input_size_explicitly,
> +       },
> +
>         /* default entry */
>         { USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_MBIM, USB_CDC_PROTO_NONE),
>           .driver_info = (unsigned long)&cdc_mbim_info_zlp,
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 8d5cbda33f66..915e29c987cb 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -407,7 +407,7 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
>         val = cdc_ncm_check_rx_max(dev, new_rx);
>
>         /* inform device about NTB input size changes */
> -       if (val != ctx->rx_max) {
> +       if (val != ctx->rx_max || ctx->drvflags & CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY) {
>                 __le32 dwNtbInMaxSize = cpu_to_le32(val);
>
>                 dev_info(&dev->intf->dev, "setting rx_max = %u\n", val);
> diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
> index 2d207cb4837d..a24f84b31a54 100644
> --- a/include/linux/usb/cdc_ncm.h
> +++ b/include/linux/usb/cdc_ncm.h
> @@ -88,6 +88,7 @@
>  #define CDC_NCM_FLAG_NDP_TO_END                        0x02    /* NDP is placed at end of frame */
>  #define CDC_MBIM_FLAG_AVOID_ALTSETTING_TOGGLE  0x04    /* Avoid altsetting toggle during init */
>  #define CDC_NCM_FLAG_PREFER_NTB32 0x08 /* prefer NDP32 over NDP16 */
> +#define CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY        0x10    /* Set input size explicitly during init */
>
>  #define cdc_ncm_comm_intf_is_mbim(x)  ((x)->desc.bInterfaceSubClass == USB_CDC_SUBCLASS_MBIM && \
>                                        (x)->desc.bInterfaceProtocol == USB_CDC_PROTO_NONE)
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: usb: cdc_mbim: adding Microsoft mobile broadband modem
  2022-08-03 10:59 ` Daniele Palmas
@ 2022-08-03 12:55   ` Seunghun Han
  2022-08-04  7:08     ` Daniele Palmas
  0 siblings, 1 reply; 6+ messages in thread
From: Seunghun Han @ 2022-08-03 12:55 UTC (permalink / raw)
  To: Daniele Palmas
  Cc: Oliver Neukum, David Miller, linux-usb, Linux Kernel Mailing List

Hello Daniele,

On Wed, Aug 3, 2022 at 7:58 PM Daniele Palmas <dnlplm@gmail.com> wrote:
>
> Just for reference, are you allowed to disclose which chipsets these
> modems are based on?
>
> Thanks,
> Daniele

I'm not sure which chipsets are used for them. In the Windows
environment, the information that I could find was Microsoft Surface
Mobile Broadband Modem. Some people guess they are based on Qualcomm
Snapdragon X16 and successors, but I could find no evidence about
them.

If you know the way, would you tell me how I can find or confirm it?

Best regards,
Seunghun

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: usb: cdc_mbim: adding Microsoft mobile broadband modem
  2022-08-03 12:55   ` Seunghun Han
@ 2022-08-04  7:08     ` Daniele Palmas
  2022-08-04 11:04       ` Seunghun Han
  0 siblings, 1 reply; 6+ messages in thread
From: Daniele Palmas @ 2022-08-04  7:08 UTC (permalink / raw)
  To: Seunghun Han
  Cc: Oliver Neukum, David Miller, linux-usb, Linux Kernel Mailing List

Hello Seunghun,

Il giorno mer 3 ago 2022 alle ore 14:56 Seunghun Han
<kkamagui@gmail.com> ha scritto:
>
> Hello Daniele,
>
> On Wed, Aug 3, 2022 at 7:58 PM Daniele Palmas <dnlplm@gmail.com> wrote:
> >
> > Just for reference, are you allowed to disclose which chipsets these
> > modems are based on?
> >
> > Thanks,
> > Daniele
>
> I'm not sure which chipsets are used for them. In the Windows
> environment, the information that I could find was Microsoft Surface
> Mobile Broadband Modem. Some people guess they are based on Qualcomm
> Snapdragon X16 and successors, but I could find no evidence about
> them.
>
> If you know the way, would you tell me how I can find or confirm it?
>

Unfortunately I'm not aware of any specific place for that: I wrongly
thought that you were involved in the development of the modem and you
knew the chipset.

However, sometimes the name of the chipset is left in the firmware
revision: you can try looking if there's any hint in the output of the
device caps request. If it's a Qualcomm modem it should probably also
support QMI-over-MBIM service, so maybe you can also try a few DMS
requests (--dms-get-software-version, --dms-get-revision...).

Sometimes also the USB descriptors can be useful.

Regards,
Daniele

> Best regards,
> Seunghun

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: usb: cdc_mbim: adding Microsoft mobile broadband modem
  2022-08-04  7:08     ` Daniele Palmas
@ 2022-08-04 11:04       ` Seunghun Han
  0 siblings, 0 replies; 6+ messages in thread
From: Seunghun Han @ 2022-08-04 11:04 UTC (permalink / raw)
  To: Daniele Palmas
  Cc: Oliver Neukum, David Miller, linux-usb, Linux Kernel Mailing List

Hello Daniele,

On Thu, Aug 4, 2022 at 4:07 PM Daniele Palmas <dnlplm@gmail.com> wrote:
>
> Unfortunately I'm not aware of any specific place for that: I wrongly
> thought that you were involved in the development of the modem and you
> knew the chipset.

Thank you for thinking like that. However, I'm just a Linux lover and
a Microsoft Surface user.

> However, sometimes the name of the chipset is left in the firmware
> revision: you can try looking if there's any hint in the output of the
> device caps request. If it's a Qualcomm modem it should probably also
> support QMI-over-MBIM service, so maybe you can also try a few DMS
> requests (--dms-get-software-version, --dms-get-revision...).
>
> Sometimes also the USB descriptors can be useful.

According to your guides, I did get some useful information from my
Surface Go 3 modem. Finally, it was based on a Qualcomm chip, and
other Microsoft mobile broadband modems should be based on the
Qualcomm chip. The test results on my Surface Go 3 are below.

==== Start of test results ====
 $> lsusb
Bus 002 Device 004: ID 045e:09a5 Microsoft Corp. Surface

$> lsbusb -t
/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/6p, 5000M
    |__ Port 1: Dev 5, If 0, Class=Hub, Driver=hub/4p, 5000M
        |__ Port 4: Dev 6, If 0, Class=Vendor Specific Class,
Driver=r8152, 5000M
    |__ Port 3: Dev 4, If 0, Class=Communications, Driver=cdc_mbim, 5000M
    |__ Port 3: Dev 4, If 1, Class=CDC Data, Driver=cdc_mbim, 5000M
    |__ Port 3: Dev 4, If 2, Class=Vendor Specific Class, Driver=, 5000M

$> qmicli --device=/dev/cdc-wdm3 --dms-get-manufacturer
[/dev/cdc-wdm3] Device manufacturer retrieved:
Manufacturer: 'QUALCOMM INCORPORATED'

$> qmicli --device=/dev/cdc-wdm3 --dms-get-revision
[/dev/cdc-wdm3] Device revision retrieved:
Revision: '2140-e88e4b-c8-00004.36  1  [May 17 2021 01:00:00]'

$> qmicli --device=/dev/cdc-wdm3 --dms-get-software-version
[/dev/cdc-wdm3] Software version: 655_GEN_PACK-1.362573.1.400078.2

$> qmicli --device=/dev/cdc-wdm3 --dms-get-band-capabilities
[/dev/cdc-wdm3] Device band capabilities retrieved:
Bands: 'wcdma-2100, wcdma-pcs-1900, wcdma-850-us, wcdma-800, wcdma-900'
LTE bands: '1, 2, 3, 4, 5, 7, 8, 12, 13, 14, 19, 20, 25, 26, 28, 29,
30, 38, 39, 40, 41'
LTE bands (extended): '1, 2, 3, 4, 5, 7, 8, 12, 13, 14, 19, 20, 25,
26, 28, 29, 30, 38, 39, 40, 41, 66'

$> qmicli --device=/dev/cdc-wdm3 --dms-get-capabilities
[/dev/cdc-wdm3] Device capabilities retrieved:
Max TX channel rate: '50000000'
Max RX channel rate: '100000000'
       Data Service: 'non-simultaneous-cs-ps'
                SIM: 'supported'
           Networks: 'umts, lte'
==== End of test results ====

Thank you for your help.

Best regards,
Seunghun

> Regards,
> Daniele
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: usb: cdc_mbim: adding Microsoft mobile broadband modem
  2022-08-03  9:02 [PATCH] net: usb: cdc_mbim: adding Microsoft mobile broadband modem Seunghun Han
  2022-08-03 10:59 ` Daniele Palmas
@ 2022-10-12  4:28 ` Seunghun Han
  1 sibling, 0 replies; 6+ messages in thread
From: Seunghun Han @ 2022-10-12  4:28 UTC (permalink / raw)
  To: linux-kernel; +Cc: davem, linux-usb, oliver, kuba, edumazet, pabeni

Does anyone have comments on my patch? If there are some points to
improve, please let me know. I'm waiting for them.

On Wed, Aug 3, 2022 at 6:02 PM Seunghun Han <kkamagui@gmail.com> wrote:
>
> Microsoft branded mobile broadband modems typically used in the Surface series
> don't work with the Linux kernel. When I query firmware information to the
> modem using the mbimcli tool, it always returns the Failure (0x02) value like
> below.
>
> === Start of the firmware id query ===
> $> mbimcli -d /dev/cdc-wdm4 --ms-query-firmware-id -v
> [26  Jul 2022, 05:24:07] [Debug] opening device...
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Queried max control message size: 4096
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Sent message...
> <<<<<< RAW:
> <<<<<<   length = 16
> <<<<<<   data   = 01:00:00:00:10:00:00:00:01:00:00:00:00:10:00:00
>
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Sent message (translated)...
> <<<<<< Header:
> <<<<<<   length      = 16
> <<<<<<   type        = open (0x00000001)
> <<<<<<   transaction = 1
> <<<<<< Contents:
> <<<<<<   max control transfer = 4096
>
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Received message...
> >>>>>> RAW:
> >>>>>>   length = 16
> >>>>>>   data   = 01:00:00:80:10:00:00:00:01:00:00:00:02:00:00:00
>
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Received message...Message Type 80000001
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 864
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 897
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 923
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 930
> [26  Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 935
> [26  Jul 2022, 05:24:07] [Debug] getting open done result failed: closed
> error: couldn't open the MbimDevice: Failure
> === End of the firmware id query ===
>
> After kernel debugging, I found that the modem reported that the dwNtbInMaxSize
> value of ncm_parm was 16384 during the initialization sequence.
> So the cdc_ncm_update_rxtx_max() in cdc_ncm_bind_common() didn't send
> USB_CDC_SET_NTB_INPUT_SIZE command because the default input size was the same,
> 16384.
>
> It's good and proper behavior. However, Microsoft branded MBMs (including the
> latest one in Surface Go 3) fail until the kernel explicitly sets the input
> size.
>
> This patch adds a new table and code changes that explicitly send
> the USB_CDC_SET_NTB_INPUT_SIZE command to support Microsoft branded MBMs.
>
> Signed-off-by: Seunghun Han <kkamagui@gmail.com>
> ---
>  drivers/net/usb/cdc_mbim.c  | 24 ++++++++++++++++++++++++
>  drivers/net/usb/cdc_ncm.c   |  2 +-
>  include/linux/usb/cdc_ncm.h |  1 +
>  3 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
> index c89639381eca..c0c23cfc02a7 100644
> --- a/drivers/net/usb/cdc_mbim.c
> +++ b/drivers/net/usb/cdc_mbim.c
> @@ -618,6 +618,22 @@ static const struct driver_info cdc_mbim_info_avoid_altsetting_toggle = {
>         .data = CDC_MBIM_FLAG_AVOID_ALTSETTING_TOGGLE,
>  };
>
> +/* Microsoft branded modems do not work properly without setting the input size
> + * explicitly in cdc_ncm_bind_common.
> + * CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY flag is used to set the input size
> + * during initialization.
> + */
> +static const struct driver_info cdc_mbim_info_set_input_size_explicitly = {
> +       .description = "CDC MBIM",
> +       .flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN | FLAG_SEND_ZLP,
> +       .bind = cdc_mbim_bind,
> +       .unbind = cdc_mbim_unbind,
> +       .manage_power = cdc_mbim_manage_power,
> +       .rx_fixup = cdc_mbim_rx_fixup,
> +       .tx_fixup = cdc_mbim_tx_fixup,
> +       .data = CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY,
> +};
> +
>  static const struct usb_device_id mbim_devs[] = {
>         /* This duplicate NCM entry is intentional. MBIM devices can
>          * be disguised as NCM by default, and this is necessary to
> @@ -665,6 +681,14 @@ static const struct usb_device_id mbim_devs[] = {
>           .driver_info = (unsigned long)&cdc_mbim_info_avoid_altsetting_toggle,
>         },
>
> +       /* Some Microsoft branded mobile broadband modems used in the Surface
> +        * seires are known to fail unless the input size is set explicitly.
> +        * Applying it to all Microsoft branded MBMs.
> +        */
> +       { USB_VENDOR_AND_INTERFACE_INFO(0x045e, USB_CLASS_COMM, USB_CDC_SUBCLASS_MBIM, USB_CDC_PROTO_NONE),
> +         .driver_info = (unsigned long)&cdc_mbim_info_set_input_size_explicitly,
> +       },
> +
>         /* default entry */
>         { USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_MBIM, USB_CDC_PROTO_NONE),
>           .driver_info = (unsigned long)&cdc_mbim_info_zlp,
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 8d5cbda33f66..915e29c987cb 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -407,7 +407,7 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
>         val = cdc_ncm_check_rx_max(dev, new_rx);
>
>         /* inform device about NTB input size changes */
> -       if (val != ctx->rx_max) {
> +       if (val != ctx->rx_max || ctx->drvflags & CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY) {
>                 __le32 dwNtbInMaxSize = cpu_to_le32(val);
>
>                 dev_info(&dev->intf->dev, "setting rx_max = %u\n", val);
> diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
> index 2d207cb4837d..a24f84b31a54 100644
> --- a/include/linux/usb/cdc_ncm.h
> +++ b/include/linux/usb/cdc_ncm.h
> @@ -88,6 +88,7 @@
>  #define CDC_NCM_FLAG_NDP_TO_END                        0x02    /* NDP is placed at end of frame */
>  #define CDC_MBIM_FLAG_AVOID_ALTSETTING_TOGGLE  0x04    /* Avoid altsetting toggle during init */
>  #define CDC_NCM_FLAG_PREFER_NTB32 0x08 /* prefer NDP32 over NDP16 */
> +#define CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY        0x10    /* Set input size explicitly during init */
>
>  #define cdc_ncm_comm_intf_is_mbim(x)  ((x)->desc.bInterfaceSubClass == USB_CDC_SUBCLASS_MBIM && \
>                                        (x)->desc.bInterfaceProtocol == USB_CDC_PROTO_NONE)
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-10-12  4:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-03  9:02 [PATCH] net: usb: cdc_mbim: adding Microsoft mobile broadband modem Seunghun Han
2022-08-03 10:59 ` Daniele Palmas
2022-08-03 12:55   ` Seunghun Han
2022-08-04  7:08     ` Daniele Palmas
2022-08-04 11:04       ` Seunghun Han
2022-10-12  4:28 ` Seunghun Han

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).