Re: [PATCH V3] Set NTB format again after altsetting switch for Huawei devices

[David Miller]

From: Enrico Mioso 
Date: Tue, 11 Jul 2017 17:21:52 +0200

> Some firmwares in Huawei E3372H devices have been observed to switch back
> to NTB 32-bit format after altsetting switch.
> This patch implements a driver flag to check for the device settings and
> set NTB format to 16-bit again if needed.
> The flag has been activated for devices controlled by the huawei_cdc_ncm.c
> driver.
> 
> V1->V2:
> - fixed broken error checks
> - some corrections to the commit message
> V2->V3:
> - variable name changes, to clarify what's happening
> - check (and possibly set) the NTB format later in the common bind code path
> 
> Signed-off-by: Enrico Mioso 
> Reported-and-tested-by: Christian Panton 
> Reviewed-by: Bjørn Mork 

Applied, thanks.

Re: [PATCH V3] Set NTB format again after altsetting switch for Huawei devices

[Bjørn Mork]

Oliver Neukum  writes:
> Am Dienstag, den 11.07.2017, 17:21 +0200 schrieb Enrico Mioso:
>> Some firmwares in Huawei E3372H devices have been observed to switch back
>> to NTB 32-bit format after altsetting switch.
>> This patch implements a driver flag to check for the device settings and
>> set NTB format to 16-bit again if needed.
>> The flag has been activated for devices controlled by the huawei_cdc_ncm.c
>> driver.
>
> Hi,
>
> does this cover reset_resume()?

AFAIK reset_resume doesn't really work for 3G/LTE modems.  Too much
state to restore both in firmware and network, with no well defined
protocol to restore it.

The only reason for having reset_resume in the modem drivers is that
quite a few modems have an SD reader. We need to support persist for the
usb-storage function, even if the modem functions fail.  There is no
reason to mess up the file system too.



Bjørn

Re: [PATCH V3] Set NTB format again after altsetting switch for Huawei devices

[Oliver Neukum]

Am Dienstag, den 11.07.2017, 17:21 +0200 schrieb Enrico Mioso:
> 
> Some firmwares in Huawei E3372H devices have been observed to switch back
> to NTB 32-bit format after altsetting switch.
> This patch implements a driver flag to check for the device settings and
> set NTB format to 16-bit again if needed.
> The flag has been activated for devices controlled by the huawei_cdc_ncm.c
> driver.

Hi,

does this cover reset_resume()?

Regards
Oliver

[PATCH V3] Set NTB format again after altsetting switch for Huawei devices

[Enrico Mioso]

Some firmwares in Huawei E3372H devices have been observed to switch back
to NTB 32-bit format after altsetting switch.
This patch implements a driver flag to check for the device settings and
set NTB format to 16-bit again if needed.
The flag has been activated for devices controlled by the huawei_cdc_ncm.c
driver.

V1->V2:
- fixed broken error checks
- some corrections to the commit message
V2->V3:
- variable name changes, to clarify what's happening
- check (and possibly set) the NTB format later in the common bind code path

Signed-off-by: Enrico Mioso 
Reported-and-tested-by: Christian Panton 
Reviewed-by: Bjørn Mork 
CC: Bjørn Mork 
CC: Christian Panton 
CC: linux-...@vger.kernel.org
CC: netdev@vger.kernel.org
CC: Oliver Neukum 
---
 drivers/net/usb/cdc_ncm.c| 28 
 drivers/net/usb/huawei_cdc_ncm.c |  6 ++
 include/linux/usb/cdc_ncm.h  |  1 +
 3 files changed, 35 insertions(+)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index d103a1d4fb36..8f572b9f3625 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -768,8 +768,10 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
u8 *buf;
int len;
int temp;
+   int err;
u8 iface_no;
struct usb_cdc_parsed_header hdr;
+   u16 curr_ntb_format;
 
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
if (!ctx)
@@ -874,6 +876,32 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct 
usb_interface *intf, u8 data_
goto error2;
}
 
+   /*
+* Some Huawei devices have been observed to come out of reset in NDP32 
mode.
+* Let's check if this is the case, and set the device to NDP16 mode 
again if
+* needed.
+   */
+   if (ctx->drvflags & CDC_NCM_FLAG_RESET_NTB16) {
+   err = usbnet_read_cmd(dev, USB_CDC_GET_NTB_FORMAT,
+ USB_TYPE_CLASS | USB_DIR_IN | 
USB_RECIP_INTERFACE,
+ 0, iface_no, _ntb_format, 2);
+   if (err < 0) {
+   goto error2;
+   }
+
+   if (curr_ntb_format == USB_CDC_NCM_NTB32_FORMAT) {
+   dev_info(>dev, "resetting NTB format to 16-bit");
+   err = usbnet_write_cmd(dev, USB_CDC_SET_NTB_FORMAT,
+  USB_TYPE_CLASS | USB_DIR_OUT
+  | USB_RECIP_INTERFACE,
+  USB_CDC_NCM_NTB16_FORMAT,
+  iface_no, NULL, 0);
+
+   if (err < 0)
+   goto error2;
+   }
+   }
+
cdc_ncm_find_endpoints(dev, ctx->data);
cdc_ncm_find_endpoints(dev, ctx->control);
if (!dev->in || !dev->out || !dev->status) {
diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c
index 2680a65cd5e4..63f28908afda 100644
--- a/drivers/net/usb/huawei_cdc_ncm.c
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -80,6 +80,12 @@ static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev,
 * be at the end of the frame.
 */
drvflags |= CDC_NCM_FLAG_NDP_TO_END;
+
+   /* Additionally, it has been reported that some Huawei E3372H devices, 
with
+* firmware version 21.318.01.00.541, come out of reset in NTB32 format 
mode, hence
+* needing to be set to the NTB16 one again.
+*/
+   drvflags |= CDC_NCM_FLAG_RESET_NTB16;
ret = cdc_ncm_bind_common(usbnet_dev, intf, 1, drvflags);
if (ret)
goto err;
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 021f7a88f52c..1a59699cf82a 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -83,6 +83,7 @@
 /* Driver flags */
 #define CDC_NCM_FLAG_NDP_TO_END0x02/* NDP is 
placed at end of frame */
 #define CDC_MBIM_FLAG_AVOID_ALTSETTING_TOGGLE  0x04/* Avoid altsetting 
toggle during init */
+#define CDC_NCM_FLAG_RESET_NTB16 0x08  /* set NDP16 one more time after 
altsetting switch */
 
 #define cdc_ncm_comm_intf_is_mbim(x)  ((x)->desc.bInterfaceSubClass == 
USB_CDC_SUBCLASS_MBIM && \
   (x)->desc.bInterfaceProtocol == 
USB_CDC_PROTO_NONE)
-- 
2.13.2