net: qmi_wwan: set correct altsetting for Gobi 1K devices (b701f16d) · Commits · 戴 / test

commit ("net: qmi_wwan: use a single bind function for all device types") made Gobi 1K devices fail probing. Using the number of endpoints in the default altsetting to decide whether the function use one or two interfaces is wrong. Other altsettings may provide more endpoints. With Gobi 1K devices, USB interface #3's altsetting is 0 by default, but altsetting 0 only provides one interrupt endpoint and is not sufficent for QMI. Altsetting 1 provides all 3 endpoints required for qmi_wwan and works with QMI. Gobi 1K layout for intf#3 is: Interface Descriptor: 255/255/255 bInterfaceNumber 3 bAlternateSetting 0 Endpoint Descriptor: Interrupt IN Interface Descriptor: 255/255/255 bInterfaceNumber 3 bAlternateSetting 1 Endpoint Descriptor: Interrupt IN Endpoint Descriptor: Bulk IN Endpoint Descriptor: Bulk OUT Prior to commit , we would call usbnet_get_endpoints before giving up finding enough endpoints. Removing the early endpoint number test and the strict functional descriptor requirement allow qmi_wwan_bind to continue until usbnet_get_endpoints has made the final attempt to collect endpoints. This restores the behaviour from before commit without losing the added benefit of using a single bind function. The driver has always required a CDC Union functional descriptor for two-interface functions. Using the existence of this descriptor to detect two-interface functions is the logically correct method. Reported-by:

Dan Williams <dcbw@redhat.com> Signed-off-by:

Bjørn Mork <bjorn@mork.no> Tested-by:

Dan Williams <dcbw@redhat.com> Signed-off-by:

David S. Miller <davem@davemloft.net>

drivers/net/usb/qmi_wwan.c

+16 −33

Original line number	Diff line number	Diff line
		@@ -139,16 +139,9 @@ static int qmi_wwan_bind(struct usbnet dev, struct usb_interface intf)

		BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) < sizeof(struct qmi_wwan_state)));

		/* control and data is shared? */
		if (intf->cur_altsetting->desc.bNumEndpoints == 3) {
		/* set up initial state */
		info->control = intf;
		info->data = intf;
		goto shared;
		}

		/* else require a single interrupt status endpoint on control intf */
		if (intf->cur_altsetting->desc.bNumEndpoints != 1)
		goto err;

		/* and a number of CDC descriptors */
		while (len > 3) {
		@@ -207,26 +200,15 @@ next_desc:
		buf += h->bLength;
		}

		/* did we find all the required ones? */
		if (!(found & (1 << USB_CDC_HEADER_TYPE)) \|\|
		!(found & (1 << USB_CDC_UNION_TYPE))) {
		dev_err(&intf->dev, "CDC functional descriptors missing\n");
		goto err;
		}

		/* verify CDC Union */
		if (desc->bInterfaceNumber != cdc_union->bMasterInterface0) {
		dev_err(&intf->dev, "bogus CDC Union: master=%u\n", cdc_union->bMasterInterface0);
		goto err;
		}

		/* need to save these for unbind */
		info->control = intf;
		/* Use separate control and data interfaces if we found a CDC Union */
		if (cdc_union) {
		info->data = usb_ifnum_to_if(dev->udev, cdc_union->bSlaveInterface0);
		if (!info->data) {
		dev_err(&intf->dev, "bogus CDC Union: slave=%u\n", cdc_union->bSlaveInterface0);
		if (desc->bInterfaceNumber != cdc_union->bMasterInterface0 \|\| !info->data) {
		dev_err(&intf->dev, "bogus CDC Union: master=%u, slave=%u\n",
		cdc_union->bMasterInterface0, cdc_union->bSlaveInterface0);
		goto err;
		}
		}

		/* errors aren't fatal - we can live with the dynamic address */
		if (cdc_ether) {
		@@ -235,11 +217,12 @@ next_desc:
		}

		/* claim data interface and set it up */
		if (info->control != info->data) {
		status = usb_driver_claim_interface(driver, info->data, dev);
		if (status < 0)
		goto err;
		}

		shared:
		status = qmi_wwan_register_subdriver(dev);
		if (status < 0 && info->control != info->data) {
		usb_set_intfdata(info->data, NULL);

Admin message