* [PATCH] : CDC EEM driver patch to be applied to 2.6.30 kernel
@ 2009-04-27 16:12 Omar Laazimani
2009-04-28 9:31 ` David Miller
0 siblings, 1 reply; 12+ messages in thread
From: Omar Laazimani @ 2009-04-27 16:12 UTC (permalink / raw)
To: netdev; +Cc: David Brownell
This is version 3 of the patch that introduces a CDC EEM kernel module
(host side only) to supports USB EEM devices.
Signed-off-by: Omar Laazimani <omar.oberthur <at> gmail.com>
diff -ruN linux-source-2.6.30/drivers/net/usb/cdc_eem.c
linux-source-2.6.30_new/drivers/net/usb/cdc_eem.c
--- linux-source-2.6.30/drivers/net/usb/cdc_eem.c 1970-01-01
01:00:00.000000000 +0100
+++ linux-source-2.6.30_new/drivers/net/usb/cdc_eem.c 2009-04-27
17:55:34.000000000 +0200
@@ -0,0 +1,319 @@
+/*
+ * USB CDC EEM based networking peripherals
+ * Copyright (C) 2009 Oberthur Technologies
+ * by Omar Laazimani, Olivier Condemine
+ *
+ * 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/etherdevice.h>
+#include <linux/ctype.h>
+#include <linux/ethtool.h>
+#include <linux/workqueue.h>
+#include <linux/mii.h>
+#include <linux/usb.h>
+#include <linux/crc32.h>
+#include <linux/usb/cdc.h>
+#include <linux/usb/usbnet.h>
+
+/*
+ * This module encapsulates Ethernet frames for transport
+ * across the USB bus.
+ *
+ * This driver is a first implementation for CDC EEM specification.
+ * For more details, see www.usb.org/developers/devclass_docs/CDC_EEM10.pdf
+ *
+ * This version has been tested with GIGAntIC WuaoW SIM Smart Card on 2.6.24,
+ * 2.6.27 and 2.6.30rc2 kernel.
+ * It has also been validated on Openmoko Om 2008.12 (based on 2.6.24 kernel).
+ * build on 23-April-2009
+ */
+
+
+#define EEM_HEAD 2 /* 2 byte header */
+#define EEM_TAIL 4 /* 4 byte crc tail */
+
+#define EEM_MAX_PACKET (ETH_FRAME_LEN + EEM_HEAD + EEM_TAIL)
+
+
+/*-------------------------------------------------------------------------*/
+
+static void eem_transmit_complete(struct urb *urb)
+{
+ kfree(urb->context);
+ usb_free_urb(urb);
+}
+
+static void eem_transmit(struct usbnet *dev, struct sk_buff *skb)
+{
+ struct urb *urb;
+ struct usb_ctrlrequest *req;
+ int status;
+ struct skb_data *entry;
+ int length;
+
+ urb = usb_alloc_urb(0, GFP_ATOMIC);
+ if (!urb)
+ return;
+
+ length = skb->len;
+
+ req = kmalloc(sizeof *req, GFP_ATOMIC);
+ if (!req) {
+ usb_free_urb(urb);
+ return;
+ }
+
+ entry = (struct skb_data *) skb->cb;
+ entry->urb = urb;
+ entry->dev = dev;
+ entry->length = length;
+
+ usb_fill_bulk_urb(urb, dev->udev, dev->out,
+ skb->data, skb->len, eem_transmit_complete, skb);
+
+ status = usb_submit_urb(urb, GFP_ATOMIC);
+ if (status) {
+ kfree(req);
+ usb_free_urb(urb);
+ return;
+ }
+}
+
+static int eem_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+ int status = 0;
+
+ status = usbnet_get_endpoints(dev, intf);
+ if (status < 0) {
+ usb_set_intfdata(intf, NULL);
+ usb_driver_release_interface(driver_of(intf), intf);
+ return status;
+ }
+
+ return 0;
+}
+
+void eem_unbind(struct usbnet *dev, struct usb_interface *intf)
+{
+ struct cdc_state *info = (void *) &dev->data;
+ struct usb_driver *driver = driver_of(intf);
+
+ if (intf == info->control && info->data) {
+ usb_set_intfdata(info->data, NULL);
+ usb_driver_release_interface(driver, info->data);
+ info->data = NULL;
+ } else if (intf == info->data && info->control) {
+ usb_set_intfdata(info->control, NULL);
+ usb_driver_release_interface(driver, info->control);
+ info->control = NULL;
+ }
+}
+
+
+static struct sk_buff *eem_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
+ gfp_t flags)
+{
+ struct sk_buff *skb2 = NULL;
+ u16 len = skb->len;
+ u32 crc = 0;
+
+ /* EEM packet header format:
+ * b0..13: length of ethernet frame
+ * b14: bmCRC
+ * b15: bmType
+ */
+ if (!skb_cloned(skb)) {
+ int headroom = skb_headroom(skb);
+ int tailroom = skb_tailroom(skb);
+
+ if ((tailroom >= EEM_TAIL) && (headroom >= EEM_HEAD))
+ goto done;
+
+ if ((headroom + tailroom) > (EEM_TAIL + EEM_HEAD)) {
+ skb->data = memmove(skb->head +
+ EEM_HEAD,
+ skb->data,
+ skb->len);
+ skb_set_tail_pointer(skb, len);
+ goto done;
+ }
+ }
+
+ skb2 = skb_copy_expand(skb, EEM_HEAD, EEM_TAIL, flags);
+ if (!skb2)
+ return NULL;
+
+ dev_kfree_skb_any(skb);
+ skb = skb2;
+
+done:
+ crc = crc32_le(~0, skb->data, skb->len);
+ crc = ~crc;
+
+ put_unaligned_le32(crc, skb_put(skb, 4));
+
+ len = skb->len;
+ put_unaligned_le16(BIT(14) | len, skb_push(skb, 2));
+
+ return skb;
+}
+
+static int eem_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
+{
+ struct sk_buff *skb2 = NULL;
+ u32 crc;
+ u16 len, header;
+ u8 bmEEMCmd;
+
+ do {
+ /*
+ * EEM packet header format:
+ * b0..14: EEM type dependant (Data or Command)
+ * b15: bmType
+ */
+ header = get_unaligned_le16(skb->data);
+ skb_pull(skb, EEM_HEAD);
+
+ /*
+ * The bmType bit helps to denote when EEM
+ * packet is data or command :
+ * bmType = 0 : EEM data payload
+ * bmType = 1 : EEM command
+ */
+ if (header & BIT(15)) {
+ /*
+ * EEM command packet:
+ * b0..10: bmEEMCmdParam
+ * b11..13: bmEEMCmd
+ * b14: bmReserved (0 by default)
+ * b15: 1
+ */
+
+ skb2 = skb_clone(skb, GFP_ATOMIC);
+ if (unlikely(!skb2))
+ goto Continue;
+
+ bmEEMCmd = (BIT(11) & header)
+ | (BIT(12) & header)
+ | (BIT(13) & header);
+
+ /*
+ * Only answer to Echo command. Host may
+ * choose to ignore other commands (see CDC EEM spec.)
+ */
+ if (bmEEMCmd == 0) { /* Echo command */
+ len = header & 0x7FF;
+ put_unaligned_le16(BIT(15) | BIT(11) | len
+ , skb_push(skb2, 2));
+ eem_transmit(dev, skb2);
+ } else
+ len = 2; /* length of other commands */
+
+ } else {
+ /*
+ * EEM data packet header :
+ * b0..13: length of ethernet frame
+ * b14: bmCRC
+ * b15: 0
+ */
+ len = header & 0x3FF;
+
+ skb2 = skb_clone(skb, GFP_ATOMIC);
+ if (unlikely(!skb2))
+ goto Continue;
+
+ crc = get_unaligned_le32(skb2->data + len - EEM_TAIL);
+ skb_trim(skb2, len - EEM_TAIL);
+
+ /*
+ * The bmCRC helps to denote when the CRC
+ * field contains a calculated CRC :
+ * bmCRC = 1 : CRC is calculated
+ * bmCRC = 0 : CRC = 0xDEADBEEF
+ */
+ if (header & BIT(14)) {
+ u32 crc2;
+ crc2 = crc32_le(~0, skb2->data, len);
+ crc2 = ~crc2;
+ if (unlikely(crc != crc2)) {
+ dev->stats.rx_errors++;
+ goto Continue;
+ }
+ } else {
+ if (unlikely(crc != 0xdeadbeef)) {
+ dev->stats.rx_errors++;
+ goto Continue;
+ }
+ }
+
+ usbnet_skb_return(dev, skb2);
+ }
+Continue:
+ skb_pull(skb, len);
+ } while (skb->len);
+
+ return 1;
+}
+
+static const struct driver_info eem_info = {
+ .description = "EEM Device",
+ .flags = FLAG_ETHER,
+ .bind = eem_bind,
+ .unbind = eem_unbind,
+ .rx_fixup = eem_rx_fixup,
+ .tx_fixup = eem_tx_fixup,
+};
+
+/*-------------------------------------------------------------------------*/
+
+static const struct usb_device_id products[] = {
+{
+ USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_EEM,
+ USB_CDC_PROTO_EEM),
+ .driver_info = (unsigned long) &eem_info,
+},
+ { },
+};
+MODULE_DEVICE_TABLE(usb, products);
+
+static struct usb_driver eem_driver = {
+ .name = "cdc_eem",
+ .id_table = products,
+ .probe = usbnet_probe,
+ .disconnect = usbnet_disconnect,
+ .suspend = usbnet_suspend,
+ .resume = usbnet_resume,
+};
+
+
+static int __init eem_init(void)
+{
+ return usb_register(&eem_driver);
+}
+module_init(eem_init);
+
+static void __exit eem_exit(void)
+{
+ usb_deregister(&eem_driver);
+}
+module_exit(eem_exit);
+
+MODULE_AUTHOR("Omar Laazimani <omar.oberthur at gmail.com>");
+MODULE_DESCRIPTION("USB CDC EEM");
+MODULE_LICENSE("GPL");
diff -ruN linux-source-2.6.30/drivers/net/usb/Kconfig
linux-source-2.6.30_new/drivers/net/usb/Kconfig
--- linux-source-2.6.30/drivers/net/usb/Kconfig 2009-03-24
10:59:19.000000000 +0100
+++ linux-source-2.6.30_new/drivers/net/usb/Kconfig 2009-04-27
17:51:10.000000000 +0200
@@ -180,6 +180,20 @@
IEEE 802 "local assignment" bit is set in the address, a "usbX"
name is used instead.
+config USB_NET_CDC_EEM
+ tristate "CDC EEM support"
+ depends on USB_USBNET && EXPERIMENTAL
+ help
+ This option supports devices conforming to the Communication Device
+ Class (CDC) Ethernet Emulation Model, a specification that's easy to
+ implement in device firmware. The CDC EEM specifications are available
+ from <http://www.usb.org/>.
+
+ This driver creates an interface named "ethX", where X depends on
+ what other networking devices you have in use. However, if the
+ IEEE 802 "local assignment" bit is set in the address, a "usbX"
+ name is used instead.
+
config USB_NET_DM9601
tristate "Davicom DM9601 based USB 1.1 10/100 ethernet devices"
depends on USB_USBNET
diff -ruN linux-source-2.6.30/drivers/net/usb/Makefile
linux-source-2.6.30_new/drivers/net/usb/Makefile
--- linux-source-2.6.30/drivers/net/usb/Makefile 2009-03-24
10:59:19.000000000 +0100
+++ linux-source-2.6.30_new/drivers/net/usb/Makefile 2009-04-27
17:48:27.000000000 +0200
@@ -9,6 +9,7 @@
obj-$(CONFIG_USB_HSO) += hso.o
obj-$(CONFIG_USB_NET_AX8817X) += asix.o
obj-$(CONFIG_USB_NET_CDCETHER) += cdc_ether.o
+obj-$(CONFIG_USB_NET_CDC_EEM) += cdc_eem.o
obj-$(CONFIG_USB_NET_DM9601) += dm9601.o
obj-$(CONFIG_USB_NET_SMSC95XX) += smsc95xx.o
obj-$(CONFIG_USB_NET_GL620A) += gl620a.o
diff -ruN linux-source-2.6.30/include/linux/usb/cdc.h
linux-source-2.6.30_new/include/linux/usb/cdc.h
--- linux-source-2.6.30/include/linux/usb/cdc.h 2009-04-15
15:17:52.000000000 +0200
+++ linux-source-2.6.30_new/include/linux/usb/cdc.h 2009-04-17
11:11:52.000000000 +0200
@@ -17,6 +17,7 @@
#define USB_CDC_SUBCLASS_DMM 0x09
#define USB_CDC_SUBCLASS_MDLM 0x0a
#define USB_CDC_SUBCLASS_OBEX 0x0b
+#define USB_CDC_SUBCLASS_EEM 0x0c
#define USB_CDC_PROTO_NONE 0
@@ -28,6 +29,8 @@
#define USB_CDC_ACM_PROTO_AT_CDMA 6
#define USB_CDC_ACM_PROTO_VENDOR 0xff
+#define USB_CDC_PROTO_EEM 7
+
/*-------------------------------------------------------------------------*/
/*
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] : CDC EEM driver patch to be applied to 2.6.30 kernel
2009-04-27 16:12 [PATCH] : CDC EEM driver patch to be applied to 2.6.30 kernel Omar Laazimani
@ 2009-04-28 9:31 ` David Miller
2009-04-28 9:55 ` David Brownell
0 siblings, 1 reply; 12+ messages in thread
From: David Miller @ 2009-04-28 9:31 UTC (permalink / raw)
To: omar.oberthur; +Cc: netdev, david-b
From: Omar Laazimani <omar.oberthur@gmail.com>
Date: Mon, 27 Apr 2009 18:12:03 +0200
> This is version 3 of the patch that introduces a CDC EEM kernel module
> (host side only) to supports USB EEM devices.
>
> Signed-off-by: Omar Laazimani <omar.oberthur@gmail.com>
David, does it look good to you? And, assuming it's OK, do you want
to take this or would you like me too?
Either way is fine with me.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] : CDC EEM driver patch to be applied to 2.6.30 kernel
2009-04-28 9:31 ` David Miller
@ 2009-04-28 9:55 ` David Brownell
2009-04-28 11:44 ` David Miller
0 siblings, 1 reply; 12+ messages in thread
From: David Brownell @ 2009-04-28 9:55 UTC (permalink / raw)
To: David Miller; +Cc: omar.oberthur, netdev
On Tuesday 28 April 2009, David Miller wrote:
> From: Omar Laazimani <omar.oberthur@gmail.com>
> Date: Mon, 27 Apr 2009 18:12:03 +0200
>
> > This is version 3 of the patch that introduces a CDC EEM kernel module
> > (host side only) to supports USB EEM devices.
> >
> > Signed-off-by: Omar Laazimani <omar.oberthur@gmail.com>
>
> David, does it look good to you?
He's addressed most of the issues I raised; maybe all,
I've only had a quick glance at this version.
> And, assuming it's OK, do you want
> to take this or would you like me too?
>
> Either way is fine with me.
If it's OK by me, I'll either ack it or send you a
new patch with my s-o-by. Hold off a couple days.
(Since this one goes through the drivers/net queue, I think
it should merge through you ... I just want to sanity check
the USB-ishness, since I'm supposed to maintain the usbnet
parts of drivers/net/usb, and this is a new part of that.)
- dave
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] : CDC EEM driver patch to be applied to 2.6.30 kernel
2009-04-28 9:55 ` David Brownell
@ 2009-04-28 11:44 ` David Miller
2009-04-28 14:47 ` Omar Laazimani
0 siblings, 1 reply; 12+ messages in thread
From: David Miller @ 2009-04-28 11:44 UTC (permalink / raw)
To: david-b; +Cc: omar.oberthur, netdev
From: David Brownell <david-b@pacbell.net>
Date: Tue, 28 Apr 2009 02:55:19 -0700
> If it's OK by me, I'll either ack it or send you a
> new patch with my s-o-by. Hold off a couple days.
Sounds great, thanks!
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] : CDC EEM driver patch to be applied to 2.6.30 kernel
2009-04-28 11:44 ` David Miller
@ 2009-04-28 14:47 ` Omar Laazimani
2009-05-04 0:13 ` David Brownell
0 siblings, 1 reply; 12+ messages in thread
From: Omar Laazimani @ 2009-04-28 14:47 UTC (permalink / raw)
To: netdev; +Cc: david-b, David Miller
This is version 4 of the patch which introduces a CDC EEM kernel module
(host side only) to supports USB EEM devices. This version fixes a
small bug in "USB Packet with
EEM bundle" case.
Signed-off-by: Omar Laazimani <omar.oberthur <at> gmail.com>
diff -ruN linux-source-2.6.30/drivers/net/usb/cdc_eem.c
linux-source-2.6.30_new/drivers/net/usb/cdc_eem.c
--- linux-source-2.6.30/drivers/net/usb/cdc_eem.c 1970-01-01
01:00:00.000000000 +0100
+++ linux-source-2.6.30_new/drivers/net/usb/cdc_eem.c 2009-04-28
16:29:12.000000000 +0200
@@ -0,0 +1,312 @@
+/*
+ * USB CDC EEM based networking peripherals
+ * Copyright (C) 2009 Oberthur Technologies
+ * by Omar Laazimani, Olivier Condemine
+ *
+ * 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/etherdevice.h>
+#include <linux/ctype.h>
+#include <linux/ethtool.h>
+#include <linux/workqueue.h>
+#include <linux/mii.h>
+#include <linux/usb.h>
+#include <linux/crc32.h>
+#include <linux/usb/cdc.h>
+#include <linux/usb/usbnet.h>
+
+/*
+ * This module encapsulates Ethernet frames for transport
+ * across the USB bus.
+ *
+ * This driver is a first implementation for CDC EEM specification.
+ * For more details, see www.usb.org/developers/devclass_docs/CDC_EEM10.pdf
+ *
+ * This version has been tested with GIGAntIC WuaoW SIM Smart Card on 2.6.24,
+ * 2.6.27 and 2.6.30rc2 kernel.
+ * It has also been validated on Openmoko Om 2008.12 (based on 2.6.24 kernel).
+ * build on 23-April-2009
+ */
+
+
+#define EEM_HEAD 2 /* 2 byte header */
+#define EEM_TAIL 4 /* 4 byte crc tail */
+
+#define EEM_MAX_PACKET (ETH_FRAME_LEN + EEM_HEAD + EEM_TAIL)
+
+
+/*-------------------------------------------------------------------------*/
+
+static void eem_transmit_complete(struct urb *urb)
+{
+ kfree(urb->context);
+ usb_free_urb(urb);
+}
+
+static void eem_transmit(struct usbnet *dev, struct sk_buff *skb)
+{
+ struct urb *urb;
+ int status;
+ struct skb_data *entry;
+ int length;
+
+ urb = usb_alloc_urb(0, GFP_ATOMIC);
+ if (!urb)
+ return;
+
+ length = skb->len;
+
+ entry = (struct skb_data *) skb->cb;
+ entry->urb = urb;
+ entry->dev = dev;
+ entry->length = length;
+
+ usb_fill_bulk_urb(urb, dev->udev, dev->out,
+ skb->data, skb->len, eem_transmit_complete, skb);
+
+ status = usb_submit_urb(urb, GFP_ATOMIC);
+ if (status) {
+ usb_free_urb(urb);
+ return;
+ }
+}
+
+static int eem_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+ int status = 0;
+
+ status = usbnet_get_endpoints(dev, intf);
+ if (status < 0) {
+ usb_set_intfdata(intf, NULL);
+ usb_driver_release_interface(driver_of(intf), intf);
+ return status;
+ }
+
+ return 0;
+}
+
+void eem_unbind(struct usbnet *dev, struct usb_interface *intf)
+{
+ struct cdc_state *info = (void *) &dev->data;
+ struct usb_driver *driver = driver_of(intf);
+
+ if (intf == info->control && info->data) {
+ usb_set_intfdata(info->data, NULL);
+ usb_driver_release_interface(driver, info->data);
+ info->data = NULL;
+ } else if (intf == info->data && info->control) {
+ usb_set_intfdata(info->control, NULL);
+ usb_driver_release_interface(driver, info->control);
+ info->control = NULL;
+ }
+}
+
+
+static struct sk_buff *eem_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
+ gfp_t flags)
+{
+ struct sk_buff *skb2 = NULL;
+ u16 len = skb->len;
+ u32 crc = 0;
+
+ /* EEM packet header format:
+ * b0..13: length of ethernet frame
+ * b14: bmCRC
+ * b15: bmType
+ */
+ if (!skb_cloned(skb)) {
+ int headroom = skb_headroom(skb);
+ int tailroom = skb_tailroom(skb);
+
+ if ((tailroom >= EEM_TAIL) && (headroom >= EEM_HEAD))
+ goto done;
+
+ if ((headroom + tailroom) > (EEM_TAIL + EEM_HEAD)) {
+ skb->data = memmove(skb->head +
+ EEM_HEAD,
+ skb->data,
+ skb->len);
+ skb_set_tail_pointer(skb, len);
+ goto done;
+ }
+ }
+
+ skb2 = skb_copy_expand(skb, EEM_HEAD, EEM_TAIL, flags);
+ if (!skb2)
+ return NULL;
+
+ dev_kfree_skb_any(skb);
+ skb = skb2;
+
+done:
+ crc = crc32_le(~0, skb->data, skb->len);
+ crc = ~crc;
+
+ put_unaligned_le32(crc, skb_put(skb, 4));
+
+ len = skb->len;
+ put_unaligned_le16(BIT(14) | len, skb_push(skb, 2));
+
+ return skb;
+}
+
+static int eem_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
+{
+ struct sk_buff *skb2 = NULL;
+ u32 crc;
+ u16 len, header;
+ u16 bmEEMCmd;
+
+ do {
+ /*
+ * EEM packet header format:
+ * b0..14: EEM type dependant (Data or Command)
+ * b15: bmType
+ */
+ header = get_unaligned_le16(skb->data);
+ skb_pull(skb, EEM_HEAD);
+
+ /*
+ * The bmType bit helps to denote when EEM
+ * packet is data or command :
+ * bmType = 0 : EEM data payload
+ * bmType = 1 : EEM command
+ */
+ if (header & BIT(15)) {
+ /*
+ * EEM command packet:
+ * b0..10: bmEEMCmdParam
+ * b11..13: bmEEMCmd
+ * b14: bmReserved (0 by default)
+ * b15: 1
+ */
+
+ skb2 = skb_clone(skb, GFP_ATOMIC);
+ if (unlikely(!skb2))
+ goto Continue;
+
+ bmEEMCmd = (BIT(11) & header)
+ | (BIT(12) & header)
+ | (BIT(13) & header);
+
+ /*
+ * Only answer to Echo command. Host may
+ * choose to ignore other commands (see CDC EEM spec.)
+ */
+ if (bmEEMCmd == 0) { /* Echo command */
+ len = header & 0x7FF;
+ skb_trim(skb2, len);
+ put_unaligned_le16(BIT(15) | BIT(11) | len
+ , skb_push(skb2, 2));
+ eem_transmit(dev, skb2);
+ } else
+ len = 0; /* length of other commands */
+
+ } else {
+ /*
+ * EEM data packet header :
+ * b0..13: length of ethernet frame
+ * b14: bmCRC
+ * b15: 0
+ */
+ len = header & 0x3FFF;
+
+ skb2 = skb_clone(skb, GFP_ATOMIC);
+ if (unlikely(!skb2))
+ goto Continue;
+
+ crc = get_unaligned_le32(skb2->data + len - EEM_TAIL);
+ skb_trim(skb2, len - EEM_TAIL);
+
+ /*
+ * The bmCRC helps to denote when the CRC
+ * field contains a calculated CRC :
+ * bmCRC = 1 : CRC is calculated
+ * bmCRC = 0 : CRC = 0xDEADBEEF
+ */
+ if (header & BIT(14)) {
+ u32 crc2;
+ crc2 = crc32_le(~0, skb2->data, len);
+ crc2 = ~crc2;
+ if (unlikely(crc != crc2)) {
+ dev->stats.rx_errors++;
+ goto Continue;
+ }
+ } else {
+ if (unlikely(crc != 0xdeadbeef)) {
+ dev->stats.rx_errors++;
+ goto Continue;
+ }
+ }
+
+ usbnet_skb_return(dev, skb2);
+ }
+Continue:
+ skb_pull(skb, len);
+ } while (skb->len);
+
+ return 1;
+}
+
+static const struct driver_info eem_info = {
+ .description = "EEM Device",
+ .flags = FLAG_ETHER,
+ .bind = eem_bind,
+ .unbind = eem_unbind,
+ .rx_fixup = eem_rx_fixup,
+ .tx_fixup = eem_tx_fixup,
+};
+
+/*-------------------------------------------------------------------------*/
+
+static const struct usb_device_id products[] = {
+{
+ USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_EEM,
+ USB_CDC_PROTO_EEM),
+ .driver_info = (unsigned long) &eem_info,
+},
+ { },
+};
+MODULE_DEVICE_TABLE(usb, products);
+
+static struct usb_driver eem_driver = {
+ .name = "cdc_eem",
+ .id_table = products,
+ .probe = usbnet_probe,
+ .disconnect = usbnet_disconnect,
+ .suspend = usbnet_suspend,
+ .resume = usbnet_resume,
+};
+
+
+static int __init eem_init(void)
+{
+ return usb_register(&eem_driver);
+}
+module_init(eem_init);
+
+static void __exit eem_exit(void)
+{
+ usb_deregister(&eem_driver);
+}
+module_exit(eem_exit);
+
+MODULE_AUTHOR("Omar Laazimani <omar.oberthur <at> gmail.com>");
+MODULE_DESCRIPTION("USB CDC EEM");
+MODULE_LICENSE("GPL");
diff -ruN linux-source-2.6.30/drivers/net/usb/Kconfig
linux-source-2.6.30_new/drivers/net/usb/Kconfig
--- linux-source-2.6.30/drivers/net/usb/Kconfig 2009-03-24
10:59:19.000000000 +0100
+++ linux-source-2.6.30_new/drivers/net/usb/Kconfig 2009-04-27
17:51:10.000000000 +0200
@@ -180,6 +180,20 @@
IEEE 802 "local assignment" bit is set in the address, a "usbX"
name is used instead.
+config USB_NET_CDC_EEM
+ tristate "CDC EEM support"
+ depends on USB_USBNET && EXPERIMENTAL
+ help
+ This option supports devices conforming to the Communication Device
+ Class (CDC) Ethernet Emulation Model, a specification that's easy to
+ implement in device firmware. The CDC EEM specifications are available
+ from <http://www.usb.org/>.
+
+ This driver creates an interface named "ethX", where X depends on
+ what other networking devices you have in use. However, if the
+ IEEE 802 "local assignment" bit is set in the address, a "usbX"
+ name is used instead.
+
config USB_NET_DM9601
tristate "Davicom DM9601 based USB 1.1 10/100 ethernet devices"
depends on USB_USBNET
diff -ruN linux-source-2.6.30/drivers/net/usb/Makefile
linux-source-2.6.30_new/drivers/net/usb/Makefile
--- linux-source-2.6.30/drivers/net/usb/Makefile 2009-03-24
10:59:19.000000000 +0100
+++ linux-source-2.6.30_new/drivers/net/usb/Makefile 2009-04-27
17:48:27.000000000 +0200
@@ -9,6 +9,7 @@
obj-$(CONFIG_USB_HSO) += hso.o
obj-$(CONFIG_USB_NET_AX8817X) += asix.o
obj-$(CONFIG_USB_NET_CDCETHER) += cdc_ether.o
+obj-$(CONFIG_USB_NET_CDC_EEM) += cdc_eem.o
obj-$(CONFIG_USB_NET_DM9601) += dm9601.o
obj-$(CONFIG_USB_NET_SMSC95XX) += smsc95xx.o
obj-$(CONFIG_USB_NET_GL620A) += gl620a.o
diff -ruN linux-source-2.6.30/include/linux/usb/cdc.h
linux-source-2.6.30_new/include/linux/usb/cdc.h
--- linux-source-2.6.30/include/linux/usb/cdc.h 2009-04-15
15:17:52.000000000 +0200
+++ linux-source-2.6.30_new/include/linux/usb/cdc.h 2009-04-17
11:11:52.000000000 +0200
@@ -17,6 +17,7 @@
#define USB_CDC_SUBCLASS_DMM 0x09
#define USB_CDC_SUBCLASS_MDLM 0x0a
#define USB_CDC_SUBCLASS_OBEX 0x0b
+#define USB_CDC_SUBCLASS_EEM 0x0c
#define USB_CDC_PROTO_NONE 0
@@ -28,6 +29,8 @@
#define USB_CDC_ACM_PROTO_AT_CDMA 6
#define USB_CDC_ACM_PROTO_VENDOR 0xff
+#define USB_CDC_PROTO_EEM 7
+
/*-------------------------------------------------------------------------*/
/*
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] : CDC EEM driver patch to be applied to 2.6.30 kernel
2009-04-28 14:47 ` Omar Laazimani
@ 2009-05-04 0:13 ` David Brownell
2009-05-04 15:51 ` Omar Laazimani
0 siblings, 1 reply; 12+ messages in thread
From: David Brownell @ 2009-05-04 0:13 UTC (permalink / raw)
To: Omar Laazimani; +Cc: netdev, David Miller
I took a look at your v4 patch against the EEM spec,
and it looks much better. However, see the appended
patch ... can you verify that this didn't break things?
If this is OK, I can merge these fixups and submit the
resulting patch.
Also, it would be good if you could add (maybe not right
away) the TX side support for zero length EEM packets.
The current behavior for transfers taking exactly N USB
packets isn't strictly conformant.
Thanks.
- Dave
============== CUT HERE
Various fixes in the proposed EEM driver:
- Framing tweaks:
* Use ETH_FCS_LEN not EEM_TAIL (trailer is the FCS or 0xdeadbeef)
* Update hard_header_len to include EEM_HEAD and ETH_FCS_LEN
- Tighten handling of link layer commands
* Free "echo" SKBs correctly
* Insist the reserved bit is zero (per EEM spec)
* Don't leak SKBs for non-echo commands
* Stub handling for non-echo commands (some trigger warnings)
- Zero length EEM packet support:
* Handle on RX side
* Add FIXME for TX side
- Other bugfixes
* Avoid false increments of rx_error count
* Check for various bogus packet conditions
* Make eem_unbind() static
- Cosmetic
* Rename the link command utilities
* Comments
* Whitespace
---
drivers/net/usb/cdc_eem.c | 236 ++++++++++++++++++++++++++++----------------
1 file changed, 153 insertions(+), 83 deletions(-)
--- a/drivers/net/usb/cdc_eem.c
+++ b/drivers/net/usb/cdc_eem.c
@@ -31,11 +31,13 @@
#include <linux/usb/cdc.h>
#include <linux/usb/usbnet.h>
+
/*
- * This module encapsulates Ethernet frames for transport
- * across the USB bus.
+ * This driver is an implementation of the CDC "Ethernet Emulation
+ * Model" (EEM) specification, which encapsulates Ethernet frames
+ * for transport over USB using a simpler USB device model than the
+ * previous CDC "Ethernet Control Model" (ECM).
*
- * This driver is a first implementation for CDC EEM specification.
* For more details, see www.usb.org/developers/devclass_docs/CDC_EEM10.pdf
*
* This version has been tested with GIGAntIC WuaoW SIM Smart Card on 2.6.24,
@@ -44,45 +46,34 @@
* build on 23-April-2009
*/
-
-#define EEM_HEAD 2 /* 2 byte header */
-#define EEM_TAIL 4 /* 4 byte crc tail */
-
-#define EEM_MAX_PACKET (ETH_FRAME_LEN + EEM_HEAD + EEM_TAIL)
-
+#define EEM_HEAD 2 /* 2 byte header */
/*-------------------------------------------------------------------------*/
-static void eem_transmit_complete(struct urb *urb)
+static void eem_linkcmd_complete(struct urb *urb)
{
- kfree(urb->context);
+ dev_kfree_skb(urb->context);
usb_free_urb(urb);
}
-static void eem_transmit(struct usbnet *dev, struct sk_buff *skb)
+static void eem_linkcmd(struct usbnet *dev, struct sk_buff *skb)
{
struct urb *urb;
int status;
- struct skb_data *entry;
- int length;
urb = usb_alloc_urb(0, GFP_ATOMIC);
if (!urb)
- return;
-
- length = skb->len;
-
- entry = (struct skb_data *) skb->cb;
- entry->urb = urb;
- entry->dev = dev;
- entry->length = length;
+ goto fail;
usb_fill_bulk_urb(urb, dev->udev, dev->out,
- skb->data, skb->len, eem_transmit_complete, skb);
+ skb->data, skb->len, eem_linkcmd_complete, skb);
status = usb_submit_urb(urb, GFP_ATOMIC);
if (status) {
usb_free_urb(urb);
+fail:
+ dev_kfree_skb(skb);
+ devwarn(dev, "link cmd failure\n");
return;
}
}
@@ -98,10 +89,14 @@ static int eem_bind(struct usbnet *dev,
return status;
}
+ /* no jumbogram (16K) support for now */
+
+ dev->net->hard_header_len += EEM_HEAD + ETH_FCS_LEN;
+
return 0;
}
-void eem_unbind(struct usbnet *dev, struct usb_interface *intf)
+static void eem_unbind(struct usbnet *dev, struct usb_interface *intf)
{
struct cdc_state *info = (void *) &dev->data;
struct usb_driver *driver = driver_of(intf);
@@ -117,7 +112,10 @@ void eem_unbind(struct usbnet *dev, stru
}
}
-
+/*
+ * EEM permits packing multiple Ethernet frames into USB transfers
+ * (a "bundle"), but for TX we don't try to do that.
+ */
static struct sk_buff *eem_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
gfp_t flags)
{
@@ -125,19 +123,19 @@ static struct sk_buff *eem_tx_fixup(stru
u16 len = skb->len;
u32 crc = 0;
- /* EEM packet header format:
- * b0..13: length of ethernet frame
- * b14: bmCRC
- * b15: bmType
+ /* FIXME when ((len + EEM_HEAD + ETH_FCS_LEN) % dev->maxpacket)
+ * is zero, stick two bytes of zero length EEM packet on the end
+ * (so the framework won't add invalid single byte padding).
*/
+
if (!skb_cloned(skb)) {
int headroom = skb_headroom(skb);
int tailroom = skb_tailroom(skb);
- if ((tailroom >= EEM_TAIL) && (headroom >= EEM_HEAD))
+ if ((tailroom >= ETH_FCS_LEN) && (headroom >= EEM_HEAD))
goto done;
- if ((headroom + tailroom) > (EEM_TAIL + EEM_HEAD)) {
+ if ((headroom + tailroom) > (ETH_FCS_LEN + EEM_HEAD)) {
skb->data = memmove(skb->head +
EEM_HEAD,
skb->data,
@@ -147,7 +145,7 @@ static struct sk_buff *eem_tx_fixup(stru
}
}
- skb2 = skb_copy_expand(skb, EEM_HEAD, EEM_TAIL, flags);
+ skb2 = skb_copy_expand(skb, EEM_HEAD, ETH_FCS_LEN, flags);
if (!skb2)
return NULL;
@@ -155,11 +153,17 @@ static struct sk_buff *eem_tx_fixup(stru
skb = skb2;
done:
+ /* we don't use the "no Ethernet CRC" option */
crc = crc32_le(~0, skb->data, skb->len);
crc = ~crc;
put_unaligned_le32(crc, skb_put(skb, 4));
+ /* EEM packet header format:
+ * b0..13: length of ethernet frame
+ * b14: bmCRC (1 == valid Ethernet CRC)
+ * b15: bmType (0 == data)
+ */
len = skb->len;
put_unaligned_le16(BIT(14) | len, skb_push(skb, 2));
@@ -168,12 +172,26 @@ done:
static int eem_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
{
- struct sk_buff *skb2 = NULL;
- u32 crc;
- u16 len, header;
- u16 bmEEMCmd;
-
+ /*
+ * Our task here is to strip off framing, leaving skb with one
+ * data frame for the usbnet framework code to process. But we
+ * may have received multiple EEM payloads, or command payloads.
+ * So we must process _everything_ as if it's a header, except
+ * maybe the last data payload
+ *
+ * REVISIT the framework needs updating so that when we consume
+ * all payloads (the last or only message was a command, or a
+ * zero length EEM packet) that is not accounted as an rx_error.
+ */
do {
+ struct sk_buff *skb2 = NULL;
+ u16 header;
+ u16 len = 0;
+
+ /* incomplete EEM header? */
+ if (skb->len < EEM_HEAD)
+ return 0;
+
/*
* EEM packet header format:
* b0..14: EEM type dependant (Data or Command)
@@ -186,78 +204,130 @@ static int eem_rx_fixup(struct usbnet *d
* The bmType bit helps to denote when EEM
* packet is data or command :
* bmType = 0 : EEM data payload
- * bmType = 1 : EEM command
+ * bmType = 1 : EEM (link) command
*/
if (header & BIT(15)) {
+ u16 bmEEMCmd;
+
/*
- * EEM command packet:
+ * EEM (link) command packet:
* b0..10: bmEEMCmdParam
* b11..13: bmEEMCmd
- * b14: bmReserved (0 by default)
- * b15: 1
+ * b14: bmReserved (must be 0)
+ * b15: 1 (EEM command)
*/
+ if (header & BIT(14)) {
+ devdbg(dev, "reserved command %04x\n", header);
+ continue;
+ }
- skb2 = skb_clone(skb, GFP_ATOMIC);
- if (unlikely(!skb2))
- goto Continue;
+ bmEEMCmd = (header >> 11) & 0x7;
+ switch (bmEEMCmd) {
- bmEEMCmd = (BIT(11) & header)
- | (BIT(12) & header)
- | (BIT(13) & header);
+ /* Responding to echo requests is mandatory. */
+ case 0: /* Echo command */
+ len = header & 0x7FF;
+
+ /* bogus command? */
+ if (skb->len < len)
+ return 0;
+
+ skb2 = skb_clone(skb, GFP_ATOMIC);
+ if (unlikely(!skb2))
+ goto next;
+ skb_trim(skb2, len);
+ put_unaligned_le16(BIT(15) | (1 << 11) | len,
+ skb_push(skb2, 2));
+ eem_linkcmd(dev, skb2);
+ break;
/*
- * Only answer to Echo command. Host may
- * choose to ignore other commands (see CDC EEM spec.)
+ * Host may choose to ignore hints.
+ * - suspend: peripheral ready to suspend
+ * - response: suggest N millisec polling
+ * - response complete: suggest N sec polling
*/
- if (bmEEMCmd == 0) { /* Echo command */
- len = header & 0x7FF;
- skb_trim(skb2, len);
- put_unaligned_le16(BIT(15) | BIT(11) | len
- , skb_push(skb2, 2));
- eem_transmit(dev, skb2);
- } else
- len = 0; /* length of other commands */
+ case 2: /* Suspend hint */
+ case 3: /* Response hint */
+ case 4: /* Response complete hint */
+ continue;
+
+ /*
+ * Hosts should never receive host-to-peripheral
+ * or reserved command codes; or responses to
+ * echo commands we didn't sent.
+ */
+ case 1: /* Echo response */
+ case 5: /* Tickle */
+ case 6: /* reserved */
+ case 7: /* reserved */
+ devwarn(dev, "unexpected link command %d\n",
+ bmEEMCmd);
+ continue;
+ }
} else {
+ u32 crc, crc2;
+ int is_last;
+
+ /* zero length EEM packet? */
+ if (header == 0)
+ continue;
+
/*
* EEM data packet header :
* b0..13: length of ethernet frame
* b14: bmCRC
- * b15: 0
+ * b15: 0 (EEM data)
*/
len = header & 0x3FFF;
- skb2 = skb_clone(skb, GFP_ATOMIC);
- if (unlikely(!skb2))
- goto Continue;
+ /* bogus EEM payload? */
+ if (skb->len < len)
+ return 0;
- crc = get_unaligned_le32(skb2->data + len - EEM_TAIL);
- skb_trim(skb2, len - EEM_TAIL);
+ /* bogus ethernet frame? */
+ if (len < (ETH_HLEN + ETH_FCS_LEN))
+ goto next;
/*
- * The bmCRC helps to denote when the CRC
- * field contains a calculated CRC :
+ * Unless this is the last EEM payload, we need
+ * to keep "skb" for further processing instead
+ * of letting the framework handle it.
+ */
+ is_last = (len == skb->len);
+ if (is_last)
+ skb2 = skb;
+ else {
+ skb2 = skb_clone(skb, GFP_ATOMIC);
+ if (unlikely(!skb2))
+ return 0;
+ }
+
+ crc = get_unaligned_le32(skb2->data + len - ETH_FCS_LEN);
+ skb_trim(skb2, len - ETH_FCS_LEN);
+
+ /*
+ * The bmCRC helps to denote when the CRC field in
+ * the Ethernet frame contains a calculated CRC:
* bmCRC = 1 : CRC is calculated
* bmCRC = 0 : CRC = 0xDEADBEEF
*/
- if (header & BIT(14)) {
- u32 crc2;
- crc2 = crc32_le(~0, skb2->data, len);
- crc2 = ~crc2;
- if (unlikely(crc != crc2)) {
- dev->stats.rx_errors++;
- goto Continue;
- }
- } else {
- if (unlikely(crc != 0xdeadbeef)) {
- dev->stats.rx_errors++;
- goto Continue;
- }
- }
+ if (header & BIT(14))
+ crc2 = ~crc32_le(~0, skb2->data, len);
+ else
+ crc2 = 0xdeadbeef;
- usbnet_skb_return(dev, skb2);
+ if (is_last)
+ return crc == crc2;
+
+ if (unlikely(crc != crc2)) {
+ dev->stats.rx_errors++;
+ } else
+ usbnet_skb_return(dev, skb2);
}
-Continue:
+
+next:
skb_pull(skb, len);
} while (skb->len);
@@ -269,7 +339,7 @@ static const struct driver_info eem_info
.flags = FLAG_ETHER,
.bind = eem_bind,
.unbind = eem_unbind,
- .rx_fixup = eem_rx_fixup,
+ .rx_fixup = eem_rx_fixup,
.tx_fixup = eem_tx_fixup,
};
@@ -307,6 +377,6 @@ static void __exit eem_exit(void)
}
module_exit(eem_exit);
-MODULE_AUTHOR("Omar Laazimani <omar.oberthur <at> gmail.com>");
+MODULE_AUTHOR("Omar Laazimani <omar.oberthur@gmail.com>");
MODULE_DESCRIPTION("USB CDC EEM");
MODULE_LICENSE("GPL");
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] : CDC EEM driver patch to be applied to 2.6.30 kernel
2009-05-04 0:13 ` David Brownell
@ 2009-05-04 15:51 ` Omar Laazimani
2009-05-04 16:32 ` David Brownell
2009-05-04 17:27 ` David Brownell
0 siblings, 2 replies; 12+ messages in thread
From: Omar Laazimani @ 2009-05-04 15:51 UTC (permalink / raw)
To: David Brownell; +Cc: David Miller, netdev
Thanks for your quick feedbacks.
I have tested your patch with our device and it's working well.
I have also added the TX side support for ZLP (see patch herein).
Please note that I can't test this issue as our device doesn't support it yet.
By the way, Just for curiosity, I have two questions about your patch
(see bellow) :
Thanks,
Omar
> /*
> - * EEM command packet:
> + * EEM (link) command packet:
> * b0..10: bmEEMCmdParam
> * b11..13: bmEEMCmd
> - * b14: bmReserved (0 by default)
> - * b15: 1
> + * b14: bmReserved (must be 0)
> + * b15: 1 (EEM command)
> */
> + if (header & BIT(14)) {
> + devdbg(dev, "reserved command %04x\n", header);
> + continue;
> + }
>
> - skb2 = skb_clone(skb, GFP_ATOMIC);
> - if (unlikely(!skb2))
> - goto Continue;
> + bmEEMCmd = (header >> 11) & 0x7;
> + switch (bmEEMCmd) {
>
> - bmEEMCmd = (BIT(11) & header)
> - | (BIT(12) & header)
> - | (BIT(13) & header);
> + /* Responding to echo requests is mandatory. */
> + case 0: /* Echo command */
> + len = header & 0x7FF;
> +
> + /* bogus command? */
> + if (skb->len < len)
> + return 0;
> +
> + skb2 = skb_clone(skb, GFP_ATOMIC);
> + if (unlikely(!skb2))
> + goto next;
> + skb_trim(skb2, len);
> + put_unaligned_le16(BIT(15) | (1 << 11) | len,
> + skb_push(skb2, 2));
why did you use 1 << 11 instead of BIT(11) ?
> + eem_linkcmd(dev, skb2);
> + break;
>
> + /*
> + * The bmCRC helps to denote when the CRC field in
> + * the Ethernet frame contains a calculated CRC:
> * bmCRC = 1 : CRC is calculated
> * bmCRC = 0 : CRC = 0xDEADBEEF
> */
> - if (header & BIT(14)) {
> - u32 crc2;
> - crc2 = crc32_le(~0, skb2->data, len);
> - crc2 = ~crc2;
> - if (unlikely(crc != crc2)) {
> - dev->stats.rx_errors++;
> - goto Continue;
> - }
> - } else {
> - if (unlikely(crc != 0xdeadbeef)) {
> - dev->stats.rx_errors++;
> - goto Continue;
> - }
> - }
> + if (header & BIT(14))
> + crc2 = ~crc32_le(~0, skb2->data, len);
> + else
> + crc2 = 0xdeadbeef;
>
> - usbnet_skb_return(dev, skb2);
> + if (is_last)
> + return crc == crc2;
Why do you prefer returning 0 and not incrementing
"dev->stats.rx_errors" instead of returning 1 (in all the cases) and
incrementing "dev->stats.rx_errors" in the error cases?
> +
> + if (unlikely(crc != crc2)) {
> + dev->stats.rx_errors++;
> + } else
> + usbnet_skb_return(dev, skb2);
============== CUT HERE
fixed :
- Zero length EEM packet support:
* Handle on TX side
--- cdc_eem.c 2009-05-04 16:59:43.000000000 +0200
+++ cdc_eem_v5.c 2009-05-04 17:07:20.000000000 +0200
@@ -31,7 +31,6 @@
#include <linux/usb/cdc.h>
#include <linux/usb/usbnet.h>
-
/*
* This driver is an implementation of the CDC "Ethernet Emulation
* Model" (EEM) specification, which encapsulates Ethernet frames
@@ -122,11 +121,14 @@
struct sk_buff *skb2 = NULL;
u16 len = skb->len;
u32 crc = 0;
+ int padlen = 0;
- /* FIXME when ((len + EEM_HEAD + ETH_FCS_LEN) % dev->maxpacket)
+ /* When ((len + EEM_HEAD + ETH_FCS_LEN) % dev->maxpacket)
* is zero, stick two bytes of zero length EEM packet on the end
* (so the framework won't add invalid single byte padding).
*/
+ if (!((len + EEM_HEAD + ETH_FCS_LEN) % dev->maxpacket))
+ padlen += 2;
if (!skb_cloned(skb)) {
int headroom = skb_headroom(skb);
@@ -145,7 +147,7 @@
}
}
- skb2 = skb_copy_expand(skb, EEM_HEAD, ETH_FCS_LEN, flags);
+ skb2 = skb_copy_expand(skb, EEM_HEAD, ETH_FCS_LEN + padlen, flags);
if (!skb2)
return NULL;
@@ -167,6 +169,10 @@
len = skb->len;
put_unaligned_le16(BIT(14) | len, skb_push(skb, 2));
+ /* Add zero length EEM packet if needed */
+ if (padlen)
+ *skb_put(skb, 2) = (u16) 0;
+
return skb;
}
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] : CDC EEM driver patch to be applied to 2.6.30 kernel
2009-05-04 15:51 ` Omar Laazimani
@ 2009-05-04 16:32 ` David Brownell
2009-05-04 17:27 ` David Brownell
1 sibling, 0 replies; 12+ messages in thread
From: David Brownell @ 2009-05-04 16:32 UTC (permalink / raw)
To: Omar Laazimani; +Cc: David Miller, netdev
On Monday 04 May 2009, Omar Laazimani wrote:
> Thanks for your quick feedbacks.
> I have tested your patch with our device and it's working well.
Great, then I'll send something to David Miller and
maybe it can merge before 2.6.30-final.
> I have also added the TX side support for ZLP (see patch herein).
> Please note that I can't test this issue as our device doesn't
> support it yet.
All your device needs to do is ignore them properly. :)
> By the way, Just for curiosity, I have two questions about your patch
> (see bellow) :
>
> > + put_unaligned_le16(BIT(15) | (1 << 11) | len,
> > + skb_push(skb2, 2));
>
> why did you use 1 << 11 instead of BIT(11) ?
To me, BIT(x) is for one-bit fields. That's a three-bit field,
and I'd write "2 << 11" for another opcode not BIT(12), or
even "3 << 11" instead of (BIT(11) | BIT(12)).
Some folk define special macros for "bitfield of length N
at offset O, value V" ... that can be overdone, but in any
case it's not standardized like BIT().
> > - usbnet_skb_return(dev, skb2);
> > + if (is_last)
> > + return crc == crc2;
>
> Why do you prefer returning 0 and not incrementing
> "dev->stats.rx_errors" instead of returning 1 (in all the cases) and
> incrementing "dev->stats.rx_errors" in the error cases?
To follow the standard calling convention as much as possible.
Look at what usbnet.c does:
if (dev->driver_info->rx_fixup
&& !dev->driver_info->rx_fixup (dev, skb))
goto error;
Returning 0 is the error path (for better or worse),
while returning 1 is the success path. So rx_fixup()
routines should not normally touch rx_errors, since
that's handled in the error path.
Plus, the other entry to the error path is returning
with skb->len == 0. You'll notice I changed things
to avoid doing that ... and that in some cases you
were both incrementing rx_error and emptying the SKB,
causing *two* errors to be reported.
> ============== CUT HERE
> fixed :
> - Zero length EEM packet support:
> * Handle on TX side
>
>
> --- cdc_eem.c 2009-05-04 16:59:43.000000000 +0200
> +++ cdc_eem_v5.c 2009-05-04 17:07:20.000000000 +0200
> @@ -31,7 +31,6 @@
> #include <linux/usb/cdc.h>
> #include <linux/usb/usbnet.h>
>
> -
> /*
> * This driver is an implementation of the CDC "Ethernet Emulation
> * Model" (EEM) specification, which encapsulates Ethernet frames
> @@ -122,11 +121,14 @@
> struct sk_buff *skb2 = NULL;
> u16 len = skb->len;
> u32 crc = 0;
> + int padlen = 0;
>
> - /* FIXME when ((len + EEM_HEAD + ETH_FCS_LEN) % dev->maxpacket)
> + /* When ((len + EEM_HEAD + ETH_FCS_LEN) % dev->maxpacket)
> * is zero, stick two bytes of zero length EEM packet on the end
> * (so the framework won't add invalid single byte padding).
> */
> + if (!((len + EEM_HEAD + ETH_FCS_LEN) % dev->maxpacket))
> + padlen += 2;
>
> if (!skb_cloned(skb)) {
> int headroom = skb_headroom(skb);
Close, but you also have to use "padlen + ETH_FCS_LEN"
when verifying there's enough space at the end of the
packet. I'll fix that.
> @@ -145,7 +147,7 @@
> }
> }
>
> - skb2 = skb_copy_expand(skb, EEM_HEAD, ETH_FCS_LEN, flags);
> + skb2 = skb_copy_expand(skb, EEM_HEAD, ETH_FCS_LEN + padlen, flags);
> if (!skb2)
> return NULL;
>
> @@ -167,6 +169,10 @@
> len = skb->len;
> put_unaligned_le16(BIT(14) | len, skb_push(skb, 2));
>
> + /* Add zero length EEM packet if needed */
> + if (padlen)
> + *skb_put(skb, 2) = (u16) 0;
> +
> return skb;
> }
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] : CDC EEM driver patch to be applied to 2.6.30 kernel
2009-05-04 15:51 ` Omar Laazimani
2009-05-04 16:32 ` David Brownell
@ 2009-05-04 17:27 ` David Brownell
2009-05-04 20:58 ` Omar Laazimani
1 sibling, 1 reply; 12+ messages in thread
From: David Brownell @ 2009-05-04 17:27 UTC (permalink / raw)
To: Omar Laazimani; +Cc: David Miller, netdev
On Monday 04 May 2009, Omar Laazimani wrote:
> Thanks for your quick feedbacks.
> I have tested your patch with our device and it's working well.
Hmm, one more issue. The eem_unbind() isn't needed at all,
is it?? It seems leftover from the ACM or ECM code, which
needed a separate control interface. Nothing even sets up
the data used by that routine (other than zero-init).
- Dave
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] : CDC EEM driver patch to be applied to 2.6.30 kernel
2009-05-04 17:27 ` David Brownell
@ 2009-05-04 20:58 ` Omar Laazimani
0 siblings, 0 replies; 12+ messages in thread
From: Omar Laazimani @ 2009-05-04 20:58 UTC (permalink / raw)
To: David Brownell; +Cc: David Miller, netdev
2009/5/4 David Brownell <david-b@pacbell.net>:
> On Monday 04 May 2009, Omar Laazimani wrote:
>> Thanks for your quick feedbacks.
>> I have tested your patch with our device and it's working well.
>
> Hmm, one more issue. The eem_unbind() isn't needed at all,
> is it?? It seems leftover from the ACM or ECM code, which
> needed a separate control interface. Nothing even sets up
> the data used by that routine (other than zero-init).
>
You're right. The eem_unbind() isn't needed and I forgot to remove it.
> - Dave
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] : CDC EEM driver patch to be applied to 2.6.30 kernel
@ 2009-04-24 15:31 Omar Laazimani
2009-04-24 20:41 ` David Brownell
0 siblings, 1 reply; 12+ messages in thread
From: Omar Laazimani @ 2009-04-24 15:31 UTC (permalink / raw)
To: netdev; +Cc: david-b
This patch introduces a CDC EEM kernel module (host side only) to
supports USB EEM devices.
diff -ruN linux-source-2.6.30/drivers/net/usb/cdc_eem.c
linux-source-2.6.30_new/drivers/net/usb/cdc_eem.c
--- linux-source-2.6.30/drivers/net/usb/cdc_eem.c 1970-01-01
01:00:00.000000000 +0100
+++ linux-source-2.6.30_new/drivers/net/usb/cdc_eem.c 2009-04-24
13:27:30.000000000 +0200
@@ -0,0 +1,254 @@
+/*
+ * USB CDC EEM based networking peripherals
+ * Copyright (C) 2009 Oberthur Technologies by Omar Laazimani,
Olivier Condemine
+ *
+ * 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/etherdevice.h>
+#include <linux/ctype.h>
+#include <linux/ethtool.h>
+#include <linux/workqueue.h>
+#include <linux/mii.h>
+#include <linux/usb.h>
+#include <linux/crc32.h>
+#include <linux/usb/cdc.h>
+#include <linux/usb/usbnet.h>
+
+/*
+ * This module encapsulates Ethernet frames for transport
+ * across the USB bus, normally "usb0".
+ *
+ * This driver is a first implementation for CDC EEM specification.
+ * For more details, see www.usb.org/developers/devclass_docs/CDC_EEM10.pdf
+ *
+ * This driver implements only the USB Packet with single EEM payload case.
+ * This version has been tested with GIGAntIC WuaoW SIM Smart Card on 2.6.24,
+ * 2.6.27 and 2.6.30rc2 kernel.
+ * It has also been validated on Openmoko Om 2008.12 (based on 2.6.24 kernel).
+ * v1.0 build on 23-April-2009
+ */
+
+
+#define EEM_HEAD 2 /* 2 byte header */
+#define EEM_TAIL 4 /* 4 byte crc tail */
+
+#define EEM_MAX_PACKET (ETH_FRAME_LEN + EEM_HEAD + EEM_TAIL)
+
+/*-------------------------------------------------------------------------*/
+
+static int eem_bind(struct usbnet *dev, struct usb_interface *intf)
+{
+ int status = 0;
+
+ status = usbnet_get_endpoints(dev, intf);
+ if (status < 0) {
+ usb_set_intfdata(intf, NULL);
+ usb_driver_release_interface(driver_of(intf), intf);
+ return status;
+ }
+
+ return 0;
+}
+
+void eem_unbind(struct usbnet *dev, struct usb_interface *intf)
+{
+ struct cdc_state *info = (void *) &dev->data;
+ struct usb_driver *driver = driver_of(intf);
+
+ if (intf == info->control && info->data) {
+ usb_set_intfdata(info->data, NULL);
+ usb_driver_release_interface(driver, info->data);
+ info->data = NULL;
+ }
+
+ else if (intf == info->data && info->control) {
+ usb_set_intfdata(info->control, NULL);
+ usb_driver_release_interface(driver, info->control);
+ info->control = NULL;
+ }
+}
+
+
+static struct sk_buff *eem_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
+ gfp_t flags)
+{
+ struct sk_buff *skb2 = NULL;
+ int len = skb->len;
+ u32 crc = 0;
+
+ /* EEM packet format:
+ * b0: bmType
+ * b1: bmCRC
+ * b2..b15: length of ethernet frame
+ * b16..n-4: ethernet frame data
+ * bn-3..n: CRC
+ */
+
+ if (!skb_cloned(skb)) {
+ int headroom = skb_headroom(skb);
+ int tailroom = skb_tailroom(skb);
+
+ if ((tailroom >= EEM_TAIL) && (headroom >= EEM_HEAD))
+ goto done;
+
+ if ((headroom + tailroom) > (EEM_TAIL + EEM_HEAD)) {
+ skb->data = memmove(skb->head +
+ EEM_HEAD,
+ skb->data,
+ skb->len);
+ skb_set_tail_pointer(skb, len);
+ goto done;
+ }
+ }
+
+ skb2 = skb_copy_expand(skb, EEM_HEAD, EEM_TAIL, flags);
+ if (!skb2)
+ return NULL;
+
+ dev_kfree_skb_any(skb);
+ skb = skb2;
+
+done:
+ crc = crc32_le(~0, skb->data, skb->len);
+ crc = ~crc;
+
+ *skb_put(skb, 1) = crc & 0xff;
+ *skb_put(skb, 1) = (crc >> 8) & 0xff;
+ *skb_put(skb, 1) = (crc >> 16) & 0xff;
+ *skb_put(skb, 1) = (crc >> 24) & 0xff;
+
+ len = skb->len;
+
+ *skb_push(skb, 1) = (len >> 8) | 0x40;
+ *skb_push(skb, 1) = len;
+
+ return skb;
+}
+
+static int eem_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
+{
+ u16 *header = NULL;
+ u32 *tailer = NULL;
+ bool bmType, bmCRC;
+
+ /*
+ * EEM packet format:
+ * b0: bmType
+ * b1: bmCRC
+ * b2..b15: length of ethernet frame
+ * b16..n-4: ethernet frame data
+ * bn-3..n: CRC
+ */
+ header = (u16 *) skb->data;
+ skb_pull(skb, EEM_HEAD);
+
+ /*
+ * The bmType bit helps to denote when EEM
+ * packet is data or command :
+ * bmType = 0 : EEM data payload
+ * bmType = 1 : EEM command
+ */
+ if (*header >> 15)
+ bmType = true;
+ else
+ bmType = false;
+
+ /*
+ * The bmCRC helps to denote when the CRC
+ * field contains a calculated CRC :
+ * bmCRC = 1 : CRC is calculated
+ * bmCRC = 0 : CRC = "0xDEADBEAF"
+ */
+ if ((*header >> 14) & 0x01)
+ bmCRC = true;
+ else
+ bmCRC = false;
+
+ tailer = (u32 *) (skb->data + skb->len - sizeof(u32));
+ skb_trim(skb, skb->len - EEM_TAIL);
+
+ if (bmCRC) {
+ /* FIXME This version of cdc_eem is a little bit tolerant :
+ * this case is considered like DEADBEAF mode.
+ */
+ } else {
+ if (*tailer != 0xefbeadde) {
+ dbg("bad CRC");
+ return 0;
+ }
+ }
+
+ if (bmType) {
+ /* FIXME the EEM commands are not managed yet. */
+ return 0;
+ }
+
+ return 1;
+}
+
+static const struct driver_info eem_info = {
+ .description = "EEM Device",
+ .flags = FLAG_ETHER,
+ .bind = eem_bind,
+ .unbind = eem_unbind,
+ .rx_fixup = eem_rx_fixup,
+ .tx_fixup = eem_tx_fixup,
+};
+
+/*-------------------------------------------------------------------------*/
+
+
+static const struct usb_device_id products[] = {
+/*
+ * CDC EEM interface.
+ */
+{
+ USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_EEM,
+ USB_CDC_PROTO_EEM),
+ .driver_info = (unsigned long) &eem_info,
+},
+ { },
+};
+MODULE_DEVICE_TABLE(usb, products);
+
+static struct usb_driver eem_driver = {
+ .name = "cdc_eem",
+ .id_table = products,
+ .probe = usbnet_probe,
+ .disconnect = usbnet_disconnect,
+ .suspend = usbnet_suspend,
+ .resume = usbnet_resume,
+};
+
+
+static int __init eem_init(void)
+{
+ return usb_register(&eem_driver);
+}
+module_init(eem_init);
+
+static void __exit eem_exit(void)
+{
+ usb_deregister(&eem_driver);
+}
+module_exit(eem_exit);
+
+MODULE_AUTHOR("Omar Laazimani <omar.oberthur at gmail.com>");
+MODULE_DESCRIPTION("USB CDC EEM v1.0");
+MODULE_LICENSE("GPL");
diff -ruN linux-source-2.6.30/drivers/net/usb/Kconfig
linux-source-2.6.30_new/drivers/net/usb/Kconfig
--- linux-source-2.6.30/drivers/net/usb/Kconfig 2009-03-24
10:59:19.000000000 +0100
+++ linux-source-2.6.30_new/drivers/net/usb/Kconfig 2009-04-23
16:50:08.000000000 +0200
@@ -180,6 +180,21 @@
IEEE 802 "local assignment" bit is set in the address, a "usbX"
name is used instead.
+config USB_NET_CDCEEM
+ tristate "CDC EEM support (smart devices such as usb smart card)"
+ depends on USB_USBNET
+ default m
+ help
+ This option supports devices conforming to the Communication Device
+ Class (CDC) Ethernet Emulation Model, a specification that's easy to
+ implement in device firmware. The CDC EEM specifications are available
+ from <http://www.usb.org/>.
+
+ This driver creates an interface named "ethX", where X depends on
+ what other networking devices you have in use. However, if the
+ IEEE 802 "local assignment" bit is set in the address, a "usbX"
+ name is used instead.
+
config USB_NET_DM9601
tristate "Davicom DM9601 based USB 1.1 10/100 ethernet devices"
depends on USB_USBNET
diff -ruN linux-source-2.6.30/drivers/net/usb/Makefile
linux-source-2.6.30_new/drivers/net/usb/Makefile
--- linux-source-2.6.30/drivers/net/usb/Makefile 2009-03-24
10:59:19.000000000 +0100
+++ linux-source-2.6.30_new/drivers/net/usb/Makefile 2009-04-23
16:49:22.000000000 +0200
@@ -9,6 +9,7 @@
obj-$(CONFIG_USB_HSO) += hso.o
obj-$(CONFIG_USB_NET_AX8817X) += asix.o
obj-$(CONFIG_USB_NET_CDCETHER) += cdc_ether.o
+obj-$(CONFIG_USB_NET_CDCEEM) += cdc_eem.o
obj-$(CONFIG_USB_NET_DM9601) += dm9601.o
obj-$(CONFIG_USB_NET_SMSC95XX) += smsc95xx.o
obj-$(CONFIG_USB_NET_GL620A) += gl620a.o
diff -ruN linux-source-2.6.30/include/linux/usb/cdc.h
linux-source-2.6.30_new/include/linux/usb/cdc.h
--- linux-source-2.6.30/include/linux/usb/cdc.h 2009-04-15
15:17:52.000000000 +0200
+++ linux-source-2.6.30_new/include/linux/usb/cdc.h 2009-04-17
11:11:52.000000000 +0200
@@ -17,6 +17,7 @@
#define USB_CDC_SUBCLASS_DMM 0x09
#define USB_CDC_SUBCLASS_MDLM 0x0a
#define USB_CDC_SUBCLASS_OBEX 0x0b
+#define USB_CDC_SUBCLASS_EEM 0x0c
#define USB_CDC_PROTO_NONE 0
@@ -28,6 +29,8 @@
#define USB_CDC_ACM_PROTO_AT_CDMA 6
#define USB_CDC_ACM_PROTO_VENDOR 0xff
+#define USB_CDC_PROTO_EEM 7
+
/*-------------------------------------------------------------------------*/
/*
Signed-off-by: Omar Laazimani <omar.oberthur@gmail.com>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] : CDC EEM driver patch to be applied to 2.6.30 kernel
2009-04-24 15:31 Omar Laazimani
@ 2009-04-24 20:41 ` David Brownell
0 siblings, 0 replies; 12+ messages in thread
From: David Brownell @ 2009-04-24 20:41 UTC (permalink / raw)
To: Omar Laazimani; +Cc: netdev
On Friday 24 April 2009, Omar Laazimani wrote:
> This patch introduces a CDC EEM kernel module (host side only) to
> supports USB EEM devices.
Great ... v2 of the patch, delivered in-line! And with a few
cleanups! :)
Still needs a signed-off-by line; see Documentation/SubmittingPatches
But it's not yet ready for merge, IMO. There are some
clear x86 dependencies here, and some bit-ordering code
that look buggy regardless.
>
>
> diff -ruN linux-source-2.6.30/drivers/net/usb/cdc_eem.c
> linux-source-2.6.30_new/drivers/net/usb/cdc_eem.c
> --- linux-source-2.6.30/drivers/net/usb/cdc_eem.c 1970-01-01
> 01:00:00.000000000 +0100
> +++ linux-source-2.6.30_new/drivers/net/usb/cdc_eem.c 2009-04-24
> 13:27:30.000000000 +0200
> @@ -0,0 +1,254 @@
> +/*
> + * USB CDC EEM based networking peripherals
> + * Copyright (C) 2009 Oberthur Technologies by Omar Laazimani,
> Olivier Condemine
Looks like your mailer is wrapping lines ... and that one is
kind of long anyway, I'd wrap by hand before "by".
Maybe Documentation/email-clients.txt would help.
> + *
> + * 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/etherdevice.h>
> +#include <linux/ctype.h>
> +#include <linux/ethtool.h>
> +#include <linux/workqueue.h>
> +#include <linux/mii.h>
> +#include <linux/usb.h>
> +#include <linux/crc32.h>
> +#include <linux/usb/cdc.h>
> +#include <linux/usb/usbnet.h>
> +
> +/*
> + * This module encapsulates Ethernet frames for transport
> + * across the USB bus, normally "usb0".
The bus isn't going to be "usb0" ... they're numbered
starting at 1, like ports. Maybe you mean the network
interface will normally be "usb0"? But that's only
going to be true if the devices you talk to aren't
using managed Ethernet addresses; you set FLAG_ETHER.
> + *
> + * This driver is a first implementation for CDC EEM specification.
> + * For more details, see www.usb.org/developers/devclass_docs/CDC_EEM10.pdf
> + *
> + * This driver implements only the USB Packet with single EEM payload case.
> + * This version has been tested with GIGAntIC WuaoW SIM Smart Card on 2.6.24,
> + * 2.6.27 and 2.6.30rc2 kernel.
> + * It has also been validated on Openmoko Om 2008.12 (based on 2.6.24 kernel).
> + * v1.0 build on 23-April-2009
> + */
> +
> +
> +#define EEM_HEAD 2 /* 2 byte header */
> +#define EEM_TAIL 4 /* 4 byte crc tail */
> +
> +#define EEM_MAX_PACKET (ETH_FRAME_LEN + EEM_HEAD + EEM_TAIL)
> +
> +/*-------------------------------------------------------------------------*/
> +
> +static int eem_bind(struct usbnet *dev, struct usb_interface *intf)
> +{
> + int status = 0;
> +
> + status = usbnet_get_endpoints(dev, intf);
> + if (status < 0) {
> + usb_set_intfdata(intf, NULL);
> + usb_driver_release_interface(driver_of(intf), intf);
> + return status;
> + }
> +
> + return 0;
> +}
> +
> +void eem_unbind(struct usbnet *dev, struct usb_interface *intf)
> +{
> + struct cdc_state *info = (void *) &dev->data;
> + struct usb_driver *driver = driver_of(intf);
> +
> + if (intf == info->control && info->data) {
> + usb_set_intfdata(info->data, NULL);
> + usb_driver_release_interface(driver, info->data);
> + info->data = NULL;
> + }
> +
> + else if (intf == info->data && info->control) {
Style: "} else if ... {"
I'd think scripts/checkpatch.pl would remind you of that.
> + usb_set_intfdata(info->control, NULL);
> + usb_driver_release_interface(driver, info->control);
> + info->control = NULL;
> + }
> +}
> +
> +
> +static struct sk_buff *eem_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
> + gfp_t flags)
> +{
> + struct sk_buff *skb2 = NULL;
> + int len = skb->len;
> + u32 crc = 0;
> +
> + /* EEM packet format:
> + * b0: bmType
> + * b1: bmCRC
> + * b2..b15: length of ethernet frame
> + * b16..n-4: ethernet frame data
> + * bn-3..n: CRC
> + */
> +
Nit: strike the extra blank line there.
> + if (!skb_cloned(skb)) {
> + int headroom = skb_headroom(skb);
> + int tailroom = skb_tailroom(skb);
> +
> + if ((tailroom >= EEM_TAIL) && (headroom >= EEM_HEAD))
> + goto done;
> +
> + if ((headroom + tailroom) > (EEM_TAIL + EEM_HEAD)) {
> + skb->data = memmove(skb->head +
> + EEM_HEAD,
> + skb->data,
> + skb->len);
> + skb_set_tail_pointer(skb, len);
> + goto done;
> + }
> + }
> +
> + skb2 = skb_copy_expand(skb, EEM_HEAD, EEM_TAIL, flags);
> + if (!skb2)
> + return NULL;
> +
> + dev_kfree_skb_any(skb);
> + skb = skb2;
> +
> +done:
> + crc = crc32_le(~0, skb->data, skb->len);
> + crc = ~crc;
> +
> + *skb_put(skb, 1) = crc & 0xff;
> + *skb_put(skb, 1) = (crc >> 8) & 0xff;
> + *skb_put(skb, 1) = (crc >> 16) & 0xff;
> + *skb_put(skb, 1) = (crc >> 24) & 0xff;
Do you really need the masks? I'd think just casting to u8
ought to suffice. But
put_unaligned_le32(crc, *skb_put(skb, 4))
would likely be more efficient.
> +
> + len = skb->len;
> +
> + *skb_push(skb, 1) = (len >> 8) | 0x40;
> + *skb_push(skb, 1) = len;
Similarly, put_unaligned_le16(BIT(0) | len)
... or what ever the right thing is. Note that your
bit numbering above doesn't match the code; you're
setting BIT(6) but documenting BIT(1) as holding the
"crc is included" flag. I'm fairly sure there are
some problems there, which would be resolved by having
your TX and RX sides consistently write 16 or 32 bit
data values instead of unrolling that logic by hand.
> +
> + return skb;
> +}
> +
> +static int eem_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> +{
> + u16 *header = NULL;
> + u32 *tailer = NULL;
That would normally be "trailer", but some variety is nice. :)
> + bool bmType, bmCRC;
> +
> + /*
> + * EEM packet format:
> + * b0: bmType
> + * b1: bmCRC
> + * b2..b15: length of ethernet frame
> + * b16..n-4: ethernet frame data
> + * bn-3..n: CRC
> + */
> + header = (u16 *) skb->data;
Hmm, this might better by "u16 header;" paired
with "header = le16_to_cpup(skb->data);" ... or
more conservatively, use get_unaligned_le16(...).
You mustn't assume the host byte order is the
same as the wire byte order.
It may be OK to assume 16-bit aligned data though;
I forget. On x86 it won't matter, but on other
CPUs it may.
> + skb_pull(skb, EEM_HEAD);
> +
> + /*
> + * The bmType bit helps to denote when EEM
> + * packet is data or command :
> + * bmType = 0 : EEM data payload
> + * bmType = 1 : EEM command
> + */
> + if (*header >> 15)
It'd be more clear if you just mask BIT(0), matching
the docs, instead of testing BIT(15) ... is this a
symptom of the invalid byte-order assumptions noted
above?
> + bmType = true;
> + else
> + bmType = false;
> +
> + /*
> + * The bmCRC helps to denote when the CRC
> + * field contains a calculated CRC :
> + * bmCRC = 1 : CRC is calculated
> + * bmCRC = 0 : CRC = "0xDEADBEAF"
> + */
> + if ((*header >> 14) & 0x01)
Likewise: test against BIT(1) or whatever ... and
make sure that your comments fix the problem where
the bit ordering in code and comments don't match.
> + bmCRC = true;
> + else
> + bmCRC = false;
> +
> + tailer = (u32 *) (skb->data + skb->len - sizeof(u32));
Better would be "u32 crc", and "crc = get_unaligned_le32(...)"
using the relevant skb ops to get that pointer.
> + skb_trim(skb, skb->len - EEM_TAIL);
It's also odd that you use EEM_TAIL here but "sizeof(u32)" above..
> +
> + if (bmCRC) {
> + /* FIXME This version of cdc_eem is a little bit tolerant :
> + * this case is considered like DEADBEAF mode.
> + */
Fix that soonish -- before submitting for merge!
You'll need to do the byteorder conversion correctly, and
handle alignment right. Again, get_unaligned_le32() would
be safest.
> + } else {
> + if (*tailer != 0xefbeadde) {
The fact that you can't test against 0xdeadbeef is strong
evidence of byteswap bugs.
> + dbg("bad CRC");
> + return 0;
> + }
> + }
> +
> + if (bmType) {
> + /* FIXME the EEM commands are not managed yet. */
Right, something to fix before merge.
I seem to recall that EEM didn't require USB packet
boundaries to match EEM message boundaries. Is that
true? If so, you'll have to cope with additional
cases here, like:
- the RX packet includes two Ethernet frames
- or one EEM command and an Ethernet frame
- or two EEM commands
- ... etc
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +static const struct driver_info eem_info = {
> + .description = "EEM Device",
> + .flags = FLAG_ETHER,
> + .bind = eem_bind,
> + .unbind = eem_unbind,
> + .rx_fixup = eem_rx_fixup,
> + .tx_fixup = eem_tx_fixup,
> +};
> +
> +/*-------------------------------------------------------------------------*/
> +
> +
> +static const struct usb_device_id products[] = {
> +/*
> + * CDC EEM interface.
> + */
Needless comment. That's what the USB_INTERFACE_INFO() says. :)
> +{
> + USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_EEM,
> + USB_CDC_PROTO_EEM),
> + .driver_info = (unsigned long) &eem_info,
> +},
> + { },
> +};
> +MODULE_DEVICE_TABLE(usb, products);
> +
> +static struct usb_driver eem_driver = {
> + .name = "cdc_eem",
> + .id_table = products,
> + .probe = usbnet_probe,
> + .disconnect = usbnet_disconnect,
> + .suspend = usbnet_suspend,
> + .resume = usbnet_resume,
> +};
> +
> +
> +static int __init eem_init(void)
> +{
> + return usb_register(&eem_driver);
> +}
> +module_init(eem_init);
> +
> +static void __exit eem_exit(void)
> +{
> + usb_deregister(&eem_driver);
> +}
> +module_exit(eem_exit);
> +
> +MODULE_AUTHOR("Omar Laazimani <omar.oberthur at gmail.com>");
> +MODULE_DESCRIPTION("USB CDC EEM v1.0");
> +MODULE_LICENSE("GPL");
> diff -ruN linux-source-2.6.30/drivers/net/usb/Kconfig
> linux-source-2.6.30_new/drivers/net/usb/Kconfig
> --- linux-source-2.6.30/drivers/net/usb/Kconfig 2009-03-24
> 10:59:19.000000000 +0100
> +++ linux-source-2.6.30_new/drivers/net/usb/Kconfig 2009-04-23
> 16:50:08.000000000 +0200
> @@ -180,6 +180,21 @@
> IEEE 802 "local assignment" bit is set in the address, a "usbX"
> name is used instead.
>
> +config USB_NET_CDCEEM
> + tristate "CDC EEM support (smart devices such as usb smart card)"
> + depends on USB_USBNET
> + default m
> + help
> + This option supports devices conforming to the Communication Device
> + Class (CDC) Ethernet Emulation Model, a specification that's easy to
> + implement in device firmware. The CDC EEM specifications are available
> + from <http://www.usb.org/>.
> +
> + This driver creates an interface named "ethX", where X depends on
> + what other networking devices you have in use. However, if the
> + IEEE 802 "local assignment" bit is set in the address, a "usbX"
> + name is used instead.
> +
> config USB_NET_DM9601
> tristate "Davicom DM9601 based USB 1.1 10/100 ethernet devices"
> depends on USB_USBNET
> diff -ruN linux-source-2.6.30/drivers/net/usb/Makefile
> linux-source-2.6.30_new/drivers/net/usb/Makefile
> --- linux-source-2.6.30/drivers/net/usb/Makefile 2009-03-24
> 10:59:19.000000000 +0100
> +++ linux-source-2.6.30_new/drivers/net/usb/Makefile 2009-04-23
> 16:49:22.000000000 +0200
> @@ -9,6 +9,7 @@
> obj-$(CONFIG_USB_HSO) += hso.o
> obj-$(CONFIG_USB_NET_AX8817X) += asix.o
> obj-$(CONFIG_USB_NET_CDCETHER) += cdc_ether.o
> +obj-$(CONFIG_USB_NET_CDCEEM) += cdc_eem.o
> obj-$(CONFIG_USB_NET_DM9601) += dm9601.o
> obj-$(CONFIG_USB_NET_SMSC95XX) += smsc95xx.o
> obj-$(CONFIG_USB_NET_GL620A) += gl620a.o
> diff -ruN linux-source-2.6.30/include/linux/usb/cdc.h
> linux-source-2.6.30_new/include/linux/usb/cdc.h
> --- linux-source-2.6.30/include/linux/usb/cdc.h 2009-04-15
> 15:17:52.000000000 +0200
> +++ linux-source-2.6.30_new/include/linux/usb/cdc.h 2009-04-17
> 11:11:52.000000000 +0200
> @@ -17,6 +17,7 @@
> #define USB_CDC_SUBCLASS_DMM 0x09
> #define USB_CDC_SUBCLASS_MDLM 0x0a
> #define USB_CDC_SUBCLASS_OBEX 0x0b
> +#define USB_CDC_SUBCLASS_EEM 0x0c
>
> #define USB_CDC_PROTO_NONE 0
>
> @@ -28,6 +29,8 @@
> #define USB_CDC_ACM_PROTO_AT_CDMA 6
> #define USB_CDC_ACM_PROTO_VENDOR 0xff
>
> +#define USB_CDC_PROTO_EEM 7
> +
> /*-------------------------------------------------------------------------*/
>
> /*
> Signed-off-by: Omar Laazimani <omar.oberthur@gmail.com>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2009-05-04 20:58 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-04-27 16:12 [PATCH] : CDC EEM driver patch to be applied to 2.6.30 kernel Omar Laazimani
2009-04-28 9:31 ` David Miller
2009-04-28 9:55 ` David Brownell
2009-04-28 11:44 ` David Miller
2009-04-28 14:47 ` Omar Laazimani
2009-05-04 0:13 ` David Brownell
2009-05-04 15:51 ` Omar Laazimani
2009-05-04 16:32 ` David Brownell
2009-05-04 17:27 ` David Brownell
2009-05-04 20:58 ` Omar Laazimani
-- strict thread matches above, loose matches on Subject: below --
2009-04-24 15:31 Omar Laazimani
2009-04-24 20:41 ` David Brownell
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).