mbm driver comments

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello,

I've now looked at this driver in detail and have a few comments:

The driver seems to be nearly identical to cdc_ether.  Even some of the
coding style errors that were commented on in the first version are
coming from cdc_ether.c. And still present there :-)

The main difference between the two drivers, and the only valid reason I
see to split this out, is the fact that the device uses the "Mobile
Direct Line" subclass instead of Ethernet.  This makes it necessary to
test the GUID included in the MDLM descriptor.

Another possibility would be adding the device vid/pid(s) to cdc_ether
and completeley ignore the GUID, as the device otherwise seems to be
completely compliant with CDC Ethernet.  I still wonder why they chose
to call this something else.  It's not like it gives you any direct
access to the radio.  Documented, anyway.

The other differences bewteen cdc_ether.c and mbm.c all seem to be
minor and cosmetic:

 link speed fixup, changing the bogus 10/10 Mbps reported by the device
 to a fixed 7.2/2.0 Mbps.  I fail to see the usefulness of this.  The
 numbers are not used for anything.  Better letting the user know that
 the reported link speed is bogus instead of faking something
 indistinguable from the real thing.

 the addition of mbm_get_drvinfo and an ethtool_ops struct pointing to
 this.  Totally useless as mbm_get_drvinfo is identical to
 usbnet_get_drvinfo, but possibly added as a framwork to be extended
 later.  Well, it can be added later too...

 suspend/resume functions doing nothing but forwarding the calls to
 usbnet.

 a check_connect function reporting the saved link state. This is
 useful, but I fail to see why it's necessary to use precious private
 data to store this state when it's already stored in the net device.
 Why not just read it there?  In fact, I fail to see why
 usbnet_get_link() doesn't fall back to doing this.  Which probably
 means that I am missing something...

 a new the network interface naming scheme.  The usefulness can
 obviously be discussed. But it's certainly only a cosmetic change


Based on these observations, I've taken the liberty to modify mbm.c
heavily, reusing as much as possible of the exported usbnet functions
and stripping away most of the cosmetic changes.  I've left a modified
check_connect function, which reads the __LINK_STATE_NOCARRIER bit from
the net device.  Don't know if this is kosher or not...

To be able to use usbnet_generic_cdc_bind() and still parse the MDLM
descriptors, we end up walking the descriptor list twice on MDLM
interfaces.  Don't know if that's acceptable?  The other options would
be either duplicating the usbnet_generic_cdc_bind() or extending it to
be able to handle verification of MDLM class devices (much the same way
as has been done for RNDIS).

Anyway, the modified driver included below is working for me.   I'm not
submitting this for inclusion as I know the original author disagrees
with at least one of the changes (the network interface naming).  I post
it here only to provoke further discussion.  That's why it lacks all the
proper signed-off-by etc.  It's not signed off.



The 3G device must be configured using AT commands on any of the
assiociated ACM (3) or WDM (2) devices.  This could be automated with a
script running at interface pre-up and post-down.  The hard part would
be matching interface name and correct tty or wdm devices.  You'll
probablly need some udev helper even with the dedicated network
interface name.

But manual configuration is OK for testing.  AFAIK, the minimum
configuration task consists of:


Step 1:  Enter PIN code if necessary

AT+CPIN?
+CPIN: READY


may need to enter a pincode if it answers anything else.  Like:

AT+CPIN?
+CPIN: SIM PIN
OK
AT+CPIN="0000"
OK
AT+CPIN?
+CPIN: READY
OK

Note that the same rules applies as for entering the code on any other
mobile terminal. Better use the correct PIN code..


Step 2: Configure an internet account

There are multiple ways of doing this.  The easiest I know of is just
doing

    AT+CGDCONT=1,"IP","apn"

where "apn" is some operator specific text string.  Note that this will
overwrite any existing account in slot 1.  You can check for existing
accounts with either

  AT+CGDCONT?
  +CGDCONT: 1,"IP","apn","0.0.0.0",0,0
  OK

and/or

  AT*EIAR=0
  *EIAR: 1,1,"PS Account 1"
  OK
  AT*EIAPSR=0
  *EIAPSR: 1,1,"apn",4,0,0
  OK


Step 3: Power on the radio

  AT+CFUN=1
  OK


Step 4: Initiate connection

 AT*ENAP=1,1
 OK

The second parameter is the number of your internet account.  Adjust as
necessary if you're not connecting to the first PS account.


Step 5: Configure IP interface

You're now connected and ready to use the network interface, run dhclient
on it (or just watch network manager do it as it gets the "link up"
message) etc.

nemi:/tmp# ethtool usb0
Settings for usb0:
        Current message level: 0x00000007 (7)
        Link detected: yes
nemi:/tmp# ifconfig usb0
usb0      Link encap:Ethernet  HWaddr 02:80:37:ec:02:00  
          inet addr:77.17.140.111  Bcast:77.17.140.127  Mask:255.255.255.224
          inet6 addr: fe80::80:37ff:feec:200/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:52 errors:0 dropped:0 overruns:0 frame:0
          TX packets:73 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000 
          RX bytes:5674 (5.5 KiB)  TX bytes:7789 (7.6 KiB)


Step 6: Disconnect and possibly turn off the radio 

 AT*ENAP=0
 OK
 AT+CFUN=4
 OK

If your using network manager, it will notice that the link goes down
and deconfigure the IP interface for you:

nemi:/tmp# ethtool usb0
Settings for usb0:
        Current message level: 0x00000007 (7)
        Link detected: no
nemi:/tmp# ifconfig usb0
usb0      Link encap:Ethernet  HWaddr 02:80:37:ec:02:00  
          UP BROADCAST MULTICAST  MTU:1500  Metric:1
          RX packets:56 errors:0 dropped:0 overruns:0 frame:0
          TX packets:80 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:1000 
          RX bytes:6131 (5.9 KiB)  TX bytes:8515 (8.3 KiB)






Bjørn



diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 8ee2103..9eae380 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -345,4 +345,21 @@ config USB_HSO
 	  To compile this driver as a module, choose M here: the
 	  module will be called hso.
 
+config USB_NET_MBM
+	tristate "Ericsson Mobile Broadband Module"
+	depends on USB_USBNET
+	select USB_NET_CDCETHER
+	default y
+	help
+	  Choose this option to support Mobile Broadband devices from
+	  Ericsson MBM, Mobile Broadband Module.
+	  This driver should work with at least the following devices:
+	    * Ericsson Mobile Broadband Minicard
+	    * Ericsson F3507g Wireless Module
+	    * Ericsson F3607gw Broadband Module
+	    * Dell Wireless 5530 HSPA
+	    * Toshiba F3507g
+	    * Sony Ericsson EC400
+	    * Sony Ericsson MD400
+
 endmenu
diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index 88a87ee..368043b 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_USB_NET_PLUSB)	+= plusb.o
 obj-$(CONFIG_USB_NET_RNDIS_HOST)	+= rndis_host.o
 obj-$(CONFIG_USB_NET_CDC_SUBSET)	+= cdc_subset.o
 obj-$(CONFIG_USB_NET_ZAURUS)	+= zaurus.o
+obj-$(CONFIG_USB_NET_MBM)	+= mbm.o
 obj-$(CONFIG_USB_NET_MCS7830)	+= mcs7830.o
 obj-$(CONFIG_USB_USBNET)	+= usbnet.o
 
diff --git a/drivers/net/usb/mbm.c b/drivers/net/usb/mbm.c
new file mode 100644
index 0000000..e0861b8
--- /dev/null
+++ b/drivers/net/usb/mbm.c
@@ -0,0 +1,253 @@
+/*
+ * Copyright (C) 2008 Carl Nordbeck <Carl.Nordbeck@xxxxxxxxxxxx>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/netdevice.h>
+#include <linux/ctype.h>
+#include <linux/ethtool.h>
+#include <linux/mii.h>
+#include <linux/usb.h>
+#include <linux/usb/cdc.h>
+#include <linux/usb/usbnet.h>
+
+#define DRIVER_VERSION "0.04"
+
+static const u8 mbm_guid[16] = {
+	0xa3, 0x17, 0xa8, 0x8b, 0x04, 0x5e, 0x4f, 0x01,
+	0xa6, 0x07, 0xc0, 0xff, 0xcb, 0x7e, 0x39, 0x2a,
+};
+
+static void dumpspeed(struct usbnet *dev, __le32 *speeds)
+{
+	/* some devices return 10 Mbps regardless of actual speed */
+	if (__le32_to_cpu(speeds[0]) == 10000000 && 
+	    __le32_to_cpu(speeds[1]) == 10000000) {
+		devinfo(dev, "ignoring bogus link speed");
+		return;
+	}
+	if (netif_msg_timer(dev))
+		devinfo(dev, "link speeds: %u kbps up, %u kbps down",
+			__le32_to_cpu(speeds[0]) / 1000,
+			__le32_to_cpu(speeds[1]) / 1000);
+}
+
+static void mbm_status(struct usbnet *dev, struct urb *urb)
+{
+	struct usb_cdc_notification	*event;
+
+	if (urb->actual_length < sizeof *event)
+		return;
+
+	/* SPEED_CHANGE can get split into two 8-byte packets */
+	if (test_and_clear_bit(EVENT_STS_SPLIT, &dev->flags)) {
+		dumpspeed(dev, (__le32 *) urb->transfer_buffer);
+		return;
+	}
+
+	event = urb->transfer_buffer;
+	switch (event->bNotificationType) {
+	case USB_CDC_NOTIFY_NETWORK_CONNECTION:
+		if (netif_msg_timer(dev))
+			devdbg(dev, "CDC: carrier %s",
+					event->wValue ? "on" : "off");
+		if (event->wValue)
+			netif_carrier_on(dev->net);
+		else
+			netif_carrier_off(dev->net);
+		break;
+	case USB_CDC_NOTIFY_SPEED_CHANGE:	/* tx/rx rates */
+		if (netif_msg_timer(dev))
+			devdbg(dev, "CDC: speed change (len %d)",
+					urb->actual_length);
+		if (urb->actual_length != (sizeof *event + 8))
+			set_bit(EVENT_STS_SPLIT, &dev->flags);
+		else
+			dumpspeed(dev, (__le32 *) &event[1]);
+		break;
+	default:
+		deverr(dev, "CDC: unexpected notification 0x%02x",
+				 event->bNotificationType);
+		break;
+	}
+}
+
+static u8 nibble(unsigned char c)
+{
+	if (likely(isdigit(c)))
+		return c - '0';
+	c = toupper(c);
+	if (likely(isxdigit(c)))
+		return 10 + c - 'A';
+	return 0;
+}
+
+static inline int
+get_ethernet_addr(struct usbnet *dev, struct usb_cdc_ether_desc *e)
+{
+	int tmp, i;
+	unsigned char buf[13];
+
+	tmp = usb_string(dev->udev, e->iMACAddress, buf, sizeof(buf));
+	if (tmp != 12) {
+		dev_dbg(&dev->udev->dev,
+			"bad MAC string %d fetch, %d\n", e->iMACAddress, tmp);
+		if (tmp >= 0)
+			tmp = -EINVAL;
+		return tmp;
+	}
+	for (i = tmp = 0; i < 6; i++, tmp += 2)
+		dev->net->dev_addr[i] =
+		    (nibble(buf[tmp]) << 4) + nibble(buf[tmp + 1]);
+	return 0;
+}
+
+static int mbm_check_connect(struct usbnet *dev)
+{
+	if (dev && dev->net)
+		return test_bit(__LINK_STATE_NOCARRIER, &dev->net->state);
+	return 0;
+}
+
+static int mbm_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+	struct cdc_state *info = (void *)&dev->data;
+	struct usb_cdc_mdlm_desc *desc = NULL;
+	struct usb_cdc_mdlm_detail_desc *detail = NULL;
+
+	u8 *buf = intf->cur_altsetting->extra;
+	int len = intf->cur_altsetting->extralen;
+	int status;
+
+	status = usbnet_generic_cdc_bind(dev, intf);
+	if (status < 0)
+		return status;
+
+	status = -ENODEV;
+
+	while (len > 3) {
+		if (buf[1] != USB_DT_CS_INTERFACE)
+			goto next_desc;
+
+		switch (buf[2]) {
+		case USB_CDC_MDLM_TYPE:
+			if (desc) {
+				dev_dbg(&intf->dev, "extra MDLM descriptor\n");
+				goto bad_desc;
+			}
+
+			desc = (void *)buf;
+
+			if (desc->bLength != sizeof(*desc))
+				goto bad_desc;
+
+			if (memcmp(&desc->bGUID, mbm_guid, 16))
+				goto bad_desc;
+			break;
+		case USB_CDC_MDLM_DETAIL_TYPE:
+			/* look like these devices only support one MDLM detail 
+			   descriptor, which must have type = 0 and contain exactly
+			   one byte of data - so let's verify that strictly for now */
+
+			if (detail) {
+				dev_dbg(&intf->dev, "extra MDLM detail descriptor\n");
+				goto bad_desc;
+			}
+
+			detail = (void *)buf;
+
+			if (detail->bGuidDescriptorType == 0) {
+				if (detail->bLength < (sizeof(*detail) + 1))
+					goto bad_desc;
+			} else
+				goto bad_desc;
+
+			break;
+		}
+next_desc:
+		len -= buf[0];	/* bLength */
+		buf += buf[0];
+	}
+
+	if (!desc || !detail) {
+		dev_dbg(&intf->dev, "missing cdc mdlm %s%sdescriptor\n",
+			desc ? "" : "func ", detail ? "" : "detail ");
+		goto bad_desc;
+	}
+
+	status = get_ethernet_addr(dev, info->ether);
+	if (status < 0)
+		goto bad_desc;
+
+	return 0;
+
+bad_desc:
+	usb_set_intfdata(info->data, NULL);
+	usb_driver_release_interface(driver_of(intf), info->data);
+	dev_info(&dev->udev->dev, "unsupported MDLM descriptors\n");
+	return status;
+}
+
+static const struct driver_info mbm_info = {
+	.description = "Mobile Broadband Network Device",
+	.check_connect = mbm_check_connect,
+	.bind = mbm_bind,
+	.unbind = usbnet_cdc_unbind,
+	.status = mbm_status,
+};
+
+static const struct usb_device_id products[] = {
+	{
+	 USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_MDLM,
+			    USB_CDC_PROTO_NONE),
+	 .driver_info = (unsigned long)&mbm_info,
+	 },
+
+	{}, /* END */
+};
+
+MODULE_DEVICE_TABLE(usb, products);
+
+
+static struct usb_driver usbmbm_driver = {
+	.name = "mbm",
+	.id_table = products,
+	.probe = usbnet_probe,
+	.disconnect = usbnet_disconnect,
+	.suspend = usbnet_suspend,
+	.resume = usbnet_resume,
+	.supports_autosuspend = 1,
+};
+
+static int __init usbmbm_init(void)
+{
+	return usb_register(&usbmbm_driver);
+}
+
+module_init(usbmbm_init);
+
+static void __exit usbmbm_exit(void)
+{
+	usb_deregister(&usbmbm_driver);
+}
+
+module_exit(usbmbm_exit);
+
+MODULE_AUTHOR("Carl Nordbeck");
+MODULE_DESCRIPTION("Ericsson Mobile Broadband");
+MODULE_LICENSE("GPL");

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

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux