* [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