Re: [PATCH] net: usb: cdc_mbim: adding Microsoft mobile broadband modem
From: Daniele Palmas
Date: Wed Aug 03 2022 - 06:58:27 EST
Hello Seunghun,
Il giorno mer 3 ago 2022 alle ore 11:10 Seunghun Han
<kkamagui@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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
>