netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/5] qmi_wwan fixes for 3.6
@ 2012-06-19 10:41 Bjørn Mork
       [not found] ` <1340102523-23990-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Bjørn Mork @ 2012-06-19 10:41 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: Oliver Neukum, Dan Williams, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	Andrew Bird (Sphere Systems), Bjørn Mork

None of these are critical, so I don't think they are appropriate
for 3.5 at this point. I've therefore based them on net-next.

The two first patches prepare the driver for the new probing
model introduced by patch #3, which is the main change in this
set.  A RFC version of this was posted to linux-usb 29 May 2012 
for discussion, as it also implicitly affects the cdc-wdm driver, 
without any comments so far.

Probing on the data interface was a mistake which was introduced
at a point where I didn't realize that the cdc-wdm subdriver
support was required in any case, because most of the supported
devices only provide a single USB interface. Apart from creating
a confusing and inconsistent driver usage, where some devices would
use cdc-wdm as a subdriver and some would not, a number of real
problems has also shown up.  The most important one at the moment
is the need to make cdc_ether and qmi_wwan cooperate wrt which
devices are supported by which driver.  This is close to impossible
unless the two probes both look at the same control interface. It
is not enough that the probe does this. We need to fine tune the
alias matching, which implies fine tuning the device ID tables.

Completing this change will require removing the now unnecessary
and conflicting device ID entries from cdc-wdm.  This will be
submitted as a separate patch against usb-next to avoid mixing
patches for both trees in one patch set.  There is no build
dependency.

The two last patches are minor obvious editorial fixes.


Bjørn Mork (5):
  net: qmi_wwan: define a structure for driver specific state
  net: qmi_wwan: rearranging to prepare for code sharing
  net: qmi_wwan: bind to both control and data interface
  net: qmi_wwan: shorten driver description
  net: qmi_wwan: use module_usb_driver macro

 drivers/net/usb/qmi_wwan.c |  308 +++++++++++++++++++++++---------------------
 1 file changed, 163 insertions(+), 145 deletions(-)

-- 
1.7.10

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH net-next 1/5] net: qmi_wwan: define a structure for driver specific state
       [not found] ` <1340102523-23990-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
@ 2012-06-19 10:41   ` Bjørn Mork
  2012-06-19 10:42   ` [PATCH net-next 5/5] net: qmi_wwan: use module_usb_driver macro Bjørn Mork
  1 sibling, 0 replies; 8+ messages in thread
From: Bjørn Mork @ 2012-06-19 10:41 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: Oliver Neukum, Dan Williams, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	Andrew Bird (Sphere Systems), Bjørn Mork

usbnet allocates a fixed size array for minidriver specific
state.  Naming the fields and taking advantage of type checking
is a bit more failsafe than casting array elements each time
they are referenced.

Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/qmi_wwan.c |   49 ++++++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 3b20678..c7b9be8 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -54,6 +54,13 @@
  *     corresponding management interface
  */
 
+/* driver specific data */
+struct qmi_wwan_state {
+	struct usb_driver *subdriver;
+	atomic_t pmcount;
+	unsigned long unused[3];
+};
+
 static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
 {
 	int status = -1;
@@ -65,9 +72,11 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
 	struct usb_cdc_ether_desc *cdc_ether = NULL;
 	u32 required = 1 << USB_CDC_HEADER_TYPE | 1 << USB_CDC_UNION_TYPE;
 	u32 found = 0;
-	atomic_t *pmcount = (void *)&dev->data[1];
+	struct qmi_wwan_state *info = (void *)&dev->data;
+
+	BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) < sizeof(struct qmi_wwan_state)));
 
-	atomic_set(pmcount, 0);
+	atomic_set(&info->pmcount, 0);
 
 	/*
 	 * assume a data interface has no additional descriptors and
@@ -177,12 +186,12 @@ err:
 /* using a counter to merge subdriver requests with our own into a combined state */
 static int qmi_wwan_manage_power(struct usbnet *dev, int on)
 {
-	atomic_t *pmcount = (void *)&dev->data[1];
+	struct qmi_wwan_state *info = (void *)&dev->data;
 	int rv = 0;
 
-	dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__, atomic_read(pmcount), on);
+	dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__, atomic_read(&info->pmcount), on);
 
-	if ((on && atomic_add_return(1, pmcount) == 1) || (!on && atomic_dec_and_test(pmcount))) {
+	if ((on && atomic_add_return(1, &info->pmcount) == 1) || (!on && atomic_dec_and_test(&info->pmcount))) {
 		/* need autopm_get/put here to ensure the usbcore sees the new value */
 		rv = usb_autopm_get_interface(dev->intf);
 		if (rv < 0)
@@ -212,7 +221,7 @@ static int qmi_wwan_bind_shared(struct usbnet *dev, struct usb_interface *intf)
 {
 	int rv;
 	struct usb_driver *subdriver = NULL;
-	atomic_t *pmcount = (void *)&dev->data[1];
+	struct qmi_wwan_state *info = (void *)&dev->data;
 
 	/* ZTE makes devices where the interface descriptors and endpoint
 	 * configurations of two or more interfaces are identical, even
@@ -228,7 +237,7 @@ static int qmi_wwan_bind_shared(struct usbnet *dev, struct usb_interface *intf)
 		goto err;
 	}
 
-	atomic_set(pmcount, 0);
+	atomic_set(&info->pmcount, 0);
 
 	/* collect all three endpoints */
 	rv = usbnet_get_endpoints(dev, intf);
@@ -251,7 +260,7 @@ static int qmi_wwan_bind_shared(struct usbnet *dev, struct usb_interface *intf)
 	dev->status = NULL;
 
 	/* save subdriver struct for suspend/resume wrappers */
-	dev->data[0] = (unsigned long)subdriver;
+	info->subdriver = subdriver;
 
 err:
 	return rv;
@@ -282,12 +291,12 @@ err:
 
 static void qmi_wwan_unbind_shared(struct usbnet *dev, struct usb_interface *intf)
 {
-	struct usb_driver *subdriver = (void *)dev->data[0];
+	struct qmi_wwan_state *info = (void *)&dev->data;
 
-	if (subdriver && subdriver->disconnect)
-		subdriver->disconnect(intf);
+	if (info->subdriver && info->subdriver->disconnect)
+		info->subdriver->disconnect(intf);
 
-	dev->data[0] = (unsigned long)NULL;
+	info->subdriver = NULL;
 }
 
 /* suspend/resume wrappers calling both usbnet and the cdc-wdm
@@ -299,15 +308,15 @@ static void qmi_wwan_unbind_shared(struct usbnet *dev, struct usb_interface *int
 static int qmi_wwan_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct usbnet *dev = usb_get_intfdata(intf);
-	struct usb_driver *subdriver = (void *)dev->data[0];
+	struct qmi_wwan_state *info = (void *)&dev->data;
 	int ret;
 
 	ret = usbnet_suspend(intf, message);
 	if (ret < 0)
 		goto err;
 
-	if (subdriver && subdriver->suspend)
-		ret = subdriver->suspend(intf, message);
+	if (info->subdriver && info->subdriver->suspend)
+		ret = info->subdriver->suspend(intf, message);
 	if (ret < 0)
 		usbnet_resume(intf);
 err:
@@ -317,16 +326,16 @@ err:
 static int qmi_wwan_resume(struct usb_interface *intf)
 {
 	struct usbnet *dev = usb_get_intfdata(intf);
-	struct usb_driver *subdriver = (void *)dev->data[0];
+	struct qmi_wwan_state *info = (void *)&dev->data;
 	int ret = 0;
 
-	if (subdriver && subdriver->resume)
-		ret = subdriver->resume(intf);
+	if (info->subdriver && info->subdriver->resume)
+		ret = info->subdriver->resume(intf);
 	if (ret < 0)
 		goto err;
 	ret = usbnet_resume(intf);
-	if (ret < 0 && subdriver && subdriver->resume && subdriver->suspend)
-		subdriver->suspend(intf, PMSG_SUSPEND);
+	if (ret < 0 && info->subdriver && info->subdriver->resume && info->subdriver->suspend)
+		info->subdriver->suspend(intf, PMSG_SUSPEND);
 err:
 	return ret;
 }
-- 
1.7.10

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH net-next 2/5] net: qmi_wwan: rearranging to prepare for code sharing
  2012-06-19 10:41 [PATCH net-next 0/5] qmi_wwan fixes for 3.6 Bjørn Mork
       [not found] ` <1340102523-23990-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
@ 2012-06-19 10:42 ` Bjørn Mork
  2012-06-19 10:42 ` [PATCH net-next 3/5] net: qmi_wwan: bind to both control and data interface Bjørn Mork
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Bjørn Mork @ 2012-06-19 10:42 UTC (permalink / raw)
  To: netdev
  Cc: Oliver Neukum, Dan Williams, linux-usb,
	Andrew Bird (Sphere Systems), Bjørn Mork

Most of the subdriver registration code can be reused for devices
with separate control and data interfaces.  Move the code a bit
around to prepare for such reuse.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/qmi_wwan.c |  128 ++++++++++++++++++++++++++------------------
 1 file changed, 76 insertions(+), 52 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index c7b9be8..6fcf54d 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -58,9 +58,80 @@
 struct qmi_wwan_state {
 	struct usb_driver *subdriver;
 	atomic_t pmcount;
-	unsigned long unused[3];
+	unsigned long unused;
+	struct usb_interface *control;
+	struct usb_interface *data;
 };
 
+/* using a counter to merge subdriver requests with our own into a combined state */
+static int qmi_wwan_manage_power(struct usbnet *dev, int on)
+{
+	struct qmi_wwan_state *info = (void *)&dev->data;
+	int rv = 0;
+
+	dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__, atomic_read(&info->pmcount), on);
+
+	if ((on && atomic_add_return(1, &info->pmcount) == 1) || (!on && atomic_dec_and_test(&info->pmcount))) {
+		/* need autopm_get/put here to ensure the usbcore sees the new value */
+		rv = usb_autopm_get_interface(dev->intf);
+		if (rv < 0)
+			goto err;
+		dev->intf->needs_remote_wakeup = on;
+		usb_autopm_put_interface(dev->intf);
+	}
+err:
+	return rv;
+}
+
+static int qmi_wwan_cdc_wdm_manage_power(struct usb_interface *intf, int on)
+{
+	struct usbnet *dev = usb_get_intfdata(intf);
+	return qmi_wwan_manage_power(dev, on);
+}
+
+/* collect all three endpoints and register subdriver */
+static int qmi_wwan_register_subdriver(struct usbnet *dev)
+{
+	int rv;
+	struct usb_driver *subdriver = NULL;
+	struct qmi_wwan_state *info = (void *)&dev->data;
+
+	/* collect bulk endpoints */
+	rv = usbnet_get_endpoints(dev, info->data);
+	if (rv < 0)
+		goto err;
+
+	/* update status endpoint if separate control interface */
+	if (info->control != info->data)
+		dev->status = &info->control->cur_altsetting->endpoint[0];
+
+	/* require interrupt endpoint for subdriver */
+	if (!dev->status) {
+		rv = -EINVAL;
+		goto err;
+	}
+
+	/* for subdriver power management */
+	atomic_set(&info->pmcount, 0);
+
+	/* register subdriver */
+	subdriver = usb_cdc_wdm_register(info->control, &dev->status->desc, 512, &qmi_wwan_cdc_wdm_manage_power);
+	if (IS_ERR(subdriver)) {
+		dev_err(&info->control->dev, "subdriver registration failed\n");
+		rv = PTR_ERR(subdriver);
+		goto err;
+	}
+
+	/* prevent usbnet from using status endpoint */
+	dev->status = NULL;
+
+	/* save subdriver struct for suspend/resume wrappers */
+	info->subdriver = subdriver;
+
+err:
+	return rv;
+}
+
 static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
 {
 	int status = -1;
@@ -183,32 +254,6 @@ err:
 	return status;
 }
 
-/* using a counter to merge subdriver requests with our own into a combined state */
-static int qmi_wwan_manage_power(struct usbnet *dev, int on)
-{
-	struct qmi_wwan_state *info = (void *)&dev->data;
-	int rv = 0;
-
-	dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__, atomic_read(&info->pmcount), on);
-
-	if ((on && atomic_add_return(1, &info->pmcount) == 1) || (!on && atomic_dec_and_test(&info->pmcount))) {
-		/* need autopm_get/put here to ensure the usbcore sees the new value */
-		rv = usb_autopm_get_interface(dev->intf);
-		if (rv < 0)
-			goto err;
-		dev->intf->needs_remote_wakeup = on;
-		usb_autopm_put_interface(dev->intf);
-	}
-err:
-	return rv;
-}
-
-static int qmi_wwan_cdc_wdm_manage_power(struct usb_interface *intf, int on)
-{
-	struct usbnet *dev = usb_get_intfdata(intf);
-	return qmi_wwan_manage_power(dev, on);
-}
-
 /* Some devices combine the "control" and "data" functions into a
  * single interface with all three endpoints: interrupt + bulk in and
  * out
@@ -220,7 +265,6 @@ static int qmi_wwan_cdc_wdm_manage_power(struct usb_interface *intf, int on)
 static int qmi_wwan_bind_shared(struct usbnet *dev, struct usb_interface *intf)
 {
 	int rv;
-	struct usb_driver *subdriver = NULL;
 	struct qmi_wwan_state *info = (void *)&dev->data;
 
 	/* ZTE makes devices where the interface descriptors and endpoint
@@ -237,30 +281,10 @@ static int qmi_wwan_bind_shared(struct usbnet *dev, struct usb_interface *intf)
 		goto err;
 	}
 
-	atomic_set(&info->pmcount, 0);
-
-	/* collect all three endpoints */
-	rv = usbnet_get_endpoints(dev, intf);
-	if (rv < 0)
-		goto err;
-
-	/* require interrupt endpoint for subdriver */
-	if (!dev->status) {
-		rv = -EINVAL;
-		goto err;
-	}
-
-	subdriver = usb_cdc_wdm_register(intf, &dev->status->desc, 512, &qmi_wwan_cdc_wdm_manage_power);
-	if (IS_ERR(subdriver)) {
-		rv = PTR_ERR(subdriver);
-		goto err;
-	}
-
-	/* can't let usbnet use the interrupt endpoint */
-	dev->status = NULL;
-
-	/* save subdriver struct for suspend/resume wrappers */
-	info->subdriver = subdriver;
+	/*  control and data is shared */
+	info->control = intf;
+	info->data = intf;
+	rv = qmi_wwan_register_subdriver(dev);
 
 err:
 	return rv;
-- 
1.7.10

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

* [PATCH net-next 3/5] net: qmi_wwan: bind to both control and data interface
  2012-06-19 10:41 [PATCH net-next 0/5] qmi_wwan fixes for 3.6 Bjørn Mork
       [not found] ` <1340102523-23990-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
  2012-06-19 10:42 ` [PATCH net-next 2/5] net: qmi_wwan: rearranging to prepare for code sharing Bjørn Mork
@ 2012-06-19 10:42 ` Bjørn Mork
  2012-06-19 10:42 ` [PATCH net-next 4/5] net: qmi_wwan: shorten driver description Bjørn Mork
  2012-06-19 10:53 ` [PATCH net-next 0/5] qmi_wwan fixes for 3.6 Oliver Neukum
  4 siblings, 0 replies; 8+ messages in thread
From: Bjørn Mork @ 2012-06-19 10:42 UTC (permalink / raw)
  To: netdev
  Cc: Oliver Neukum, Dan Williams, linux-usb,
	Andrew Bird (Sphere Systems), Bjørn Mork,
	Thomas Schäfer

Always bind to control interface regardless of whether
it is a shared interface or not.

A QMI/wwan function is required to provide both a control
interface (QMI) and a data interface (wwan).  All devices
supported by this driver do so.  But the vendors may
choose to use different USB descriptor layouts, and some
vendors even allow the same device to present different
layouts.

Most of these devices use a USB descriptor layout with a
single USB interface for both control and data.  But some
split control and data into two interfaces, bound together
by a CDC Union descriptor on the control interface. Before
the cdc-wdm subdriver support was added, this split was
used to let cdc-wdm drive the QMI control interface and
qmi_wwan drive the wwna data interface.

This split driver model has a number of issues:
 - qmi_wwan must match on the data interface descriptor,
   which often are indistiguishable from data interfaces
   belonging to other CDC (like) functions like ACM
 - supporting a single QMI/wwan function requires adding
   the device to two drivers
 - syncronizing the probes among a number of drivers, to
   ensure selecting the correct driver, is difficult unless
   all drivers match on the same interface

This patch resolves these problems by using the same
probing mechanism as cdc-ether for devices with a two-
interface USB descriptor layout.  This makes the driver
behave consistently, supporting both the control and data
part of the QMI/wwan function, regardless of the USB
descriptors.

Cc: Thomas Schäfer <tschaefer@t-online.de>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/qmi_wwan.c |  131 +++++++++++++++++++++-----------------------
 1 file changed, 63 insertions(+), 68 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 6fcf54d..05571fc 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1,6 +1,10 @@
 /*
  * Copyright (c) 2012  Bjørn Mork <bjorn@mork.no>
  *
+ * The probing code is heavily inspired by cdc_ether, which is:
+ * Copyright (C) 2003-2005 by David Brownell
+ * Copyright (C) 2006 by Ole Andre Vadla Ravnas (ActiveSync)
+ *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
  * version 2 as published by the Free Software Foundation.
@@ -15,11 +19,7 @@
 #include <linux/usb/usbnet.h>
 #include <linux/usb/cdc-wdm.h>
 
-/* The name of the CDC Device Management driver */
-#define DM_DRIVER "cdc_wdm"
-
-/*
- * This driver supports wwan (3G/LTE/?) devices using a vendor
+/* This driver supports wwan (3G/LTE/?) devices using a vendor
  * specific management protocol called Qualcomm MSM Interface (QMI) -
  * in addition to the more common AT commands over serial interface
  * management
@@ -31,27 +31,16 @@
  * management protocol is used in place of the standard CDC
  * notifications NOTIFY_NETWORK_CONNECTION and NOTIFY_SPEED_CHANGE
  *
+ * Alternatively, control and data functions can be combined in a
+ * single USB interface.
+ *
  * Handling a protocol like QMI is out of the scope for any driver.
- * It can be exported as a character device using the cdc-wdm driver,
- * which will enable userspace applications ("modem managers") to
- * handle it.  This may be required to use the network interface
- * provided by the driver.
+ * It is exported as a character device using the cdc-wdm driver as
+ * a subdriver, enabling userspace applications ("modem managers") to
+ * handle it.
  *
  * These devices may alternatively/additionally be configured using AT
- * commands on any of the serial interfaces driven by the option driver
- *
- * This driver binds only to the data ("slave") interface to enable
- * the cdc-wdm driver to bind to the control interface.  It still
- * parses the CDC functional descriptors on the control interface to
- *  a) verify that this is indeed a handled interface (CDC Union
- *     header lists it as slave)
- *  b) get MAC address and other ethernet config from the CDC Ethernet
- *     header
- *  c) enable user bind requests against the control interface, which
- *     is the common way to bind to CDC Ethernet Control Model type
- *     interfaces
- *  d) provide a hint to the user about which interface is the
- *     corresponding management interface
+ * commands on a serial interface
  */
 
 /* driver specific data */
@@ -135,7 +124,6 @@ err:
 static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
 {
 	int status = -1;
-	struct usb_interface *control = NULL;
 	u8 *buf = intf->cur_altsetting->extra;
 	int len = intf->cur_altsetting->extralen;
 	struct usb_interface_descriptor *desc = &intf->cur_altsetting->desc;
@@ -143,27 +131,14 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
 	struct usb_cdc_ether_desc *cdc_ether = NULL;
 	u32 required = 1 << USB_CDC_HEADER_TYPE | 1 << USB_CDC_UNION_TYPE;
 	u32 found = 0;
+	struct usb_driver *driver = driver_of(intf);
 	struct qmi_wwan_state *info = (void *)&dev->data;
 
 	BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) < sizeof(struct qmi_wwan_state)));
 
-	atomic_set(&info->pmcount, 0);
-
-	/*
-	 * assume a data interface has no additional descriptors and
-	 * that the control and data interface are numbered
-	 * consecutively - this holds for the Huawei device at least
-	 */
-	if (len == 0 && desc->bInterfaceNumber > 0) {
-		control = usb_ifnum_to_if(dev->udev, desc->bInterfaceNumber - 1);
-		if (!control)
-			goto err;
-
-		buf = control->cur_altsetting->extra;
-		len = control->cur_altsetting->extralen;
-		dev_dbg(&intf->dev, "guessing \"control\" => %s, \"data\" => this\n",
-			dev_name(&control->dev));
-	}
+	/* require a single interrupt status endpoint for subdriver */
+	if (intf->cur_altsetting->desc.bNumEndpoints != 1)
+		goto err;
 
 	while (len > 3) {
 		struct usb_descriptor_header *h = (void *)buf;
@@ -227,10 +202,17 @@ next_desc:
 		goto err;
 	}
 
-	/* give the user a helpful hint if trying to bind to the wrong interface */
-	if (cdc_union && desc->bInterfaceNumber == cdc_union->bMasterInterface0) {
-		dev_err(&intf->dev, "leaving \"control\" interface for " DM_DRIVER " - try binding to %s instead!\n",
-			dev_name(&usb_ifnum_to_if(dev->udev, cdc_union->bSlaveInterface0)->dev));
+	/* 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;
+	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);
 		goto err;
 	}
 
@@ -240,15 +222,16 @@ next_desc:
 		usbnet_get_ethernet_addr(dev, cdc_ether->iMACAddress);
 	}
 
-	/* success! point the user to the management interface */
-	if (control)
-		dev_info(&intf->dev, "Use \"" DM_DRIVER "\" for QMI interface %s\n",
-			dev_name(&control->dev));
-
-	/* XXX: add a sysfs symlink somewhere to help management applications find it? */
+	/* claim data interface and set it up */
+	status = usb_driver_claim_interface(driver, info->data, dev);
+	if (status < 0)
+		goto err;
 
-	/* collect bulk endpoints now that we know intf == "data" interface */
-	status = usbnet_get_endpoints(dev, intf);
+	status = qmi_wwan_register_subdriver(dev);
+	if (status < 0) {
+		usb_set_intfdata(info->data, NULL);
+		usb_driver_release_interface(driver, info->data);
+	}
 
 err:
 	return status;
@@ -257,11 +240,7 @@ err:
 /* Some devices combine the "control" and "data" functions into a
  * single interface with all three endpoints: interrupt + bulk in and
  * out
- *
- * Setting up cdc-wdm as a subdriver owning the interrupt endpoint
- * will let it provide userspace access to the encapsulated QMI
- * protocol without interfering with the usbnet operations.
-  */
+ */
 static int qmi_wwan_bind_shared(struct usbnet *dev, struct usb_interface *intf)
 {
 	int rv;
@@ -313,14 +292,30 @@ err:
 	return rv;
 }
 
-static void qmi_wwan_unbind_shared(struct usbnet *dev, struct usb_interface *intf)
+static void qmi_wwan_unbind(struct usbnet *dev, struct usb_interface *intf)
 {
 	struct qmi_wwan_state *info = (void *)&dev->data;
+	struct usb_driver *driver = driver_of(intf);
+	struct usb_interface *other;
 
 	if (info->subdriver && info->subdriver->disconnect)
-		info->subdriver->disconnect(intf);
+		info->subdriver->disconnect(info->control);
+
+	/* allow user to unbind using either control or data */
+	if (intf == info->control)
+		other = info->data;
+	else
+		other = info->control;
+
+	/* only if not shared */
+	if (other && intf != other) {
+		usb_set_intfdata(other, NULL);
+		usb_driver_release_interface(driver, other);
+	}
 
 	info->subdriver = NULL;
+	info->data = NULL;
+	info->control = NULL;
 }
 
 /* suspend/resume wrappers calling both usbnet and the cdc-wdm
@@ -364,11 +359,11 @@ err:
 	return ret;
 }
 
-
 static const struct driver_info	qmi_wwan_info = {
 	.description	= "QMI speaking wwan device",
 	.flags		= FLAG_WWAN,
 	.bind		= qmi_wwan_bind,
+	.unbind		= qmi_wwan_unbind,
 	.manage_power	= qmi_wwan_manage_power,
 };
 
@@ -376,7 +371,7 @@ static const struct driver_info	qmi_wwan_shared = {
 	.description	= "QMI speaking wwan device with combined interface",
 	.flags		= FLAG_WWAN,
 	.bind		= qmi_wwan_bind_shared,
-	.unbind		= qmi_wwan_unbind_shared,
+	.unbind		= qmi_wwan_unbind,
 	.manage_power	= qmi_wwan_manage_power,
 };
 
@@ -384,7 +379,7 @@ static const struct driver_info	qmi_wwan_gobi = {
 	.description	= "Qualcomm Gobi wwan/QMI device",
 	.flags		= FLAG_WWAN,
 	.bind		= qmi_wwan_bind_gobi,
-	.unbind		= qmi_wwan_unbind_shared,
+	.unbind		= qmi_wwan_unbind,
 	.manage_power	= qmi_wwan_manage_power,
 };
 
@@ -393,7 +388,7 @@ static const struct driver_info	qmi_wwan_force_int1 = {
 	.description	= "Qualcomm WWAN/QMI device",
 	.flags		= FLAG_WWAN,
 	.bind		= qmi_wwan_bind_shared,
-	.unbind		= qmi_wwan_unbind_shared,
+	.unbind		= qmi_wwan_unbind,
 	.manage_power	= qmi_wwan_manage_power,
 	.data		= BIT(1), /* interface whitelist bitmap */
 };
@@ -402,7 +397,7 @@ static const struct driver_info	qmi_wwan_force_int4 = {
 	.description	= "Qualcomm WWAN/QMI device",
 	.flags		= FLAG_WWAN,
 	.bind		= qmi_wwan_bind_shared,
-	.unbind		= qmi_wwan_unbind_shared,
+	.unbind		= qmi_wwan_unbind,
 	.manage_power	= qmi_wwan_manage_power,
 	.data		= BIT(4), /* interface whitelist bitmap */
 };
@@ -424,7 +419,7 @@ static const struct driver_info	qmi_wwan_sierra = {
 	.description	= "Sierra Wireless wwan/QMI device",
 	.flags		= FLAG_WWAN,
 	.bind		= qmi_wwan_bind_gobi,
-	.unbind		= qmi_wwan_unbind_shared,
+	.unbind		= qmi_wwan_unbind,
 	.manage_power	= qmi_wwan_manage_power,
 	.data		= BIT(8) | BIT(19), /* interface whitelist bitmap */
 };
@@ -440,7 +435,7 @@ static const struct usb_device_id products[] = {
 		.idVendor           = HUAWEI_VENDOR_ID,
 		.bInterfaceClass    = USB_CLASS_VENDOR_SPEC,
 		.bInterfaceSubClass = 1,
-		.bInterfaceProtocol = 8, /* NOTE: This is the *slave* interface of the CDC Union! */
+		.bInterfaceProtocol = 9, /* CDC Ethernet *control* interface */
 		.driver_info        = (unsigned long)&qmi_wwan_info,
 	},
 	{	/* Vodafone/Huawei K5005 (12d1:14c8) and similar modems */
@@ -448,7 +443,7 @@ static const struct usb_device_id products[] = {
 		.idVendor           = HUAWEI_VENDOR_ID,
 		.bInterfaceClass    = USB_CLASS_VENDOR_SPEC,
 		.bInterfaceSubClass = 1,
-		.bInterfaceProtocol = 56, /* NOTE: This is the *slave* interface of the CDC Union! */
+		.bInterfaceProtocol = 57, /* CDC Ethernet *control* interface */
 		.driver_info        = (unsigned long)&qmi_wwan_info,
 	},
 	{	/* Huawei E392, E398 and possibly others in "Windows mode"
-- 
1.7.10

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

* [PATCH net-next 4/5] net: qmi_wwan: shorten driver description
  2012-06-19 10:41 [PATCH net-next 0/5] qmi_wwan fixes for 3.6 Bjørn Mork
                   ` (2 preceding siblings ...)
  2012-06-19 10:42 ` [PATCH net-next 3/5] net: qmi_wwan: bind to both control and data interface Bjørn Mork
@ 2012-06-19 10:42 ` Bjørn Mork
  2012-06-19 10:53 ` [PATCH net-next 0/5] qmi_wwan fixes for 3.6 Oliver Neukum
  4 siblings, 0 replies; 8+ messages in thread
From: Bjørn Mork @ 2012-06-19 10:42 UTC (permalink / raw)
  To: netdev
  Cc: Oliver Neukum, Dan Williams, linux-usb,
	Andrew Bird (Sphere Systems), Bjørn Mork

The description is used in ethtool fixed length fields.  Make
it shorter to avoid truncation.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/qmi_wwan.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 05571fc..f12ba3c 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -360,7 +360,7 @@ err:
 }
 
 static const struct driver_info	qmi_wwan_info = {
-	.description	= "QMI speaking wwan device",
+	.description	= "WWAN/QMI device",
 	.flags		= FLAG_WWAN,
 	.bind		= qmi_wwan_bind,
 	.unbind		= qmi_wwan_unbind,
@@ -368,7 +368,7 @@ static const struct driver_info	qmi_wwan_info = {
 };
 
 static const struct driver_info	qmi_wwan_shared = {
-	.description	= "QMI speaking wwan device with combined interface",
+	.description	= "WWAN/QMI device",
 	.flags		= FLAG_WWAN,
 	.bind		= qmi_wwan_bind_shared,
 	.unbind		= qmi_wwan_unbind,
-- 
1.7.10

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

* [PATCH net-next 5/5] net: qmi_wwan: use module_usb_driver macro
       [not found] ` <1340102523-23990-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
  2012-06-19 10:41   ` [PATCH net-next 1/5] net: qmi_wwan: define a structure for driver specific state Bjørn Mork
@ 2012-06-19 10:42   ` Bjørn Mork
  1 sibling, 0 replies; 8+ messages in thread
From: Bjørn Mork @ 2012-06-19 10:42 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: Oliver Neukum, Dan Williams, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	Andrew Bird (Sphere Systems), Bjørn Mork

Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/qmi_wwan.c |   12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index f12ba3c..f1e7791 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -593,17 +593,7 @@ static struct usb_driver qmi_wwan_driver = {
 	.disable_hub_initiated_lpm = 1,
 };
 
-static int __init qmi_wwan_init(void)
-{
-	return usb_register(&qmi_wwan_driver);
-}
-module_init(qmi_wwan_init);

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

* Re: [PATCH net-next 0/5] qmi_wwan fixes for 3.6
  2012-06-19 10:41 [PATCH net-next 0/5] qmi_wwan fixes for 3.6 Bjørn Mork
                   ` (3 preceding siblings ...)
  2012-06-19 10:42 ` [PATCH net-next 4/5] net: qmi_wwan: shorten driver description Bjørn Mork
@ 2012-06-19 10:53 ` Oliver Neukum
       [not found]   ` <201206191253.17413.oneukum-l3A5Bk7waGM@public.gmane.org>
  4 siblings, 1 reply; 8+ messages in thread
From: Oliver Neukum @ 2012-06-19 10:53 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: netdev, Dan Williams, linux-usb, Andrew Bird (Sphere Systems)

Am Dienstag, 19. Juni 2012, 12:41:58 schrieb Bjørn Mork:
> The two first patches prepare the driver for the new probing
> model introduced by patch #3, which is the main change in this
> set.  A RFC version of this was posted to linux-usb 29 May 2012 
> for discussion, as it also implicitly affects the cdc-wdm driver, 
> without any comments so far.

Looking good.

	Regards
		Oliver

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

* Re: [PATCH net-next 0/5] qmi_wwan fixes for 3.6
       [not found]   ` <201206191253.17413.oneukum-l3A5Bk7waGM@public.gmane.org>
@ 2012-06-19 22:04     ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2012-06-19 22:04 UTC (permalink / raw)
  To: oneukum-l3A5Bk7waGM
  Cc: bjorn-yOkvZcmFvRU, netdev-u79uwXL29TY76Z2rM5mHXA,
	dcbw-H+wXaHxf7aLQT0dZR+AlfA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	ajb-5+cxppFmGx6/3pe1ocb+s/XRex20P6io

From: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>
Date: Tue, 19 Jun 2012 12:53:17 +0200

> Am Dienstag, 19. Juni 2012, 12:41:58 schrieb Bjørn Mork:
>> The two first patches prepare the driver for the new probing
>> model introduced by patch #3, which is the main change in this
>> set.  A RFC version of this was posted to linux-usb 29 May 2012 
>> for discussion, as it also implicitly affects the cdc-wdm driver, 
>> without any comments so far.
> 
> Looking good.

All applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2012-06-19 22:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-19 10:41 [PATCH net-next 0/5] qmi_wwan fixes for 3.6 Bjørn Mork
     [not found] ` <1340102523-23990-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
2012-06-19 10:41   ` [PATCH net-next 1/5] net: qmi_wwan: define a structure for driver specific state Bjørn Mork
2012-06-19 10:42   ` [PATCH net-next 5/5] net: qmi_wwan: use module_usb_driver macro Bjørn Mork
2012-06-19 10:42 ` [PATCH net-next 2/5] net: qmi_wwan: rearranging to prepare for code sharing Bjørn Mork
2012-06-19 10:42 ` [PATCH net-next 3/5] net: qmi_wwan: bind to both control and data interface Bjørn Mork
2012-06-19 10:42 ` [PATCH net-next 4/5] net: qmi_wwan: shorten driver description Bjørn Mork
2012-06-19 10:53 ` [PATCH net-next 0/5] qmi_wwan fixes for 3.6 Oliver Neukum
     [not found]   ` <201206191253.17413.oneukum-l3A5Bk7waGM@public.gmane.org>
2012-06-19 22:04     ` David Miller

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).