[PATCH] USB host CDC Phonet network interface driver

[PATCH] USB host CDC Phonet network interface driver

[Rémi Denis-Courmont]

From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

Many Nokia handsets support a Phonet interface to the cellular modem
via a vendor-specific USB interface. CDC Phonet follows the
Communications Device Class model, with one control interface, and
and a pair of inactive and active data alternative interface. The later
has two bulk endpoint, one per direction.

This was tested against Nokia E61, Nokia N95, and the existing Phonet
gadget function for the Linux composite USB gadget framework.

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 drivers/net/usb/Kconfig      |    8 +
 drivers/net/usb/Makefile     |    1 +
 drivers/net/usb/cdc-phonet.c |  461 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 470 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/usb/cdc-phonet.c

diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index a906d39..c47237c 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -369,4 +369,12 @@ config USB_NET_INT51X1
 	  (Powerline Communications) solution with an Intellon
 	  INT51x1/INT5200 chip, like the "devolo dLan duo".
 
+config USB_CDC_PHONET
+	tristate "CDC Phonet support"
+	depends on PHONET
+	help
+	  Choose this option to support the Phonet interface to a Nokia
+	  cellular modem, as found on most Nokia handsets with the
+	  "PC suite" USB profile.
+
 endmenu
diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index b870b0b..e17afb7 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -21,4 +21,5 @@ obj-$(CONFIG_USB_NET_ZAURUS)	+= zaurus.o
 obj-$(CONFIG_USB_NET_MCS7830)	+= mcs7830.o
 obj-$(CONFIG_USB_USBNET)	+= usbnet.o
 obj-$(CONFIG_USB_NET_INT51X1)	+= int51x1.o
+obj-$(CONFIG_USB_CDC_PHONET)	+= cdc-phonet.o
 
diff --git a/drivers/net/usb/cdc-phonet.c b/drivers/net/usb/cdc-phonet.c
new file mode 100644
index 0000000..792af72
--- /dev/null
+++ b/drivers/net/usb/cdc-phonet.c
@@ -0,0 +1,461 @@
+/*
+ * phonet.c -- USB CDC Phonet host driver
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation. All rights reserved.
+ *
+ * Author: Rémi Denis-Courmont
+ *
+ * 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.
+ *
+ * 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., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/usb.h>
+#include <linux/usb/cdc.h>
+#include <linux/netdevice.h>
+#include <linux/if_arp.h>
+#include <linux/if_phonet.h>
+
+#define PN_MEDIA_USB	0x1B
+
+static const unsigned rxq_size = 17;
+
+struct usbpn_dev {
+	struct net_device	*dev;
+
+	struct usb_interface	*intf, *data_intf;
+	struct usb_device	*usb;
+	unsigned int		tx_pipe, rx_pipe;
+	u8 active_setting;
+	u8 disconnected;
+
+	unsigned		tx_queue;
+	spinlock_t		tx_lock;
+
+	spinlock_t		rx_lock;
+	struct sk_buff		*rx_skb;
+	struct urb		*urbs[0];
+};
+
+static void tx_complete(struct urb *req);
+static void rx_complete(struct urb *req);
+
+/*
+ * Network device callbacks
+ */
+static int usbpn_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	struct urb *req = NULL;
+	unsigned long flags;
+	int err;
+
+	if (skb->protocol != htons(ETH_P_PHONET))
+		goto drop;
+
+	req = usb_alloc_urb(0, GFP_ATOMIC);
+	if (!req)
+		goto drop;
+	usb_fill_bulk_urb(req, pnd->usb, pnd->tx_pipe, skb->data, skb->len,
+				tx_complete, skb);
+	req->transfer_flags = URB_ZERO_PACKET;
+	err = usb_submit_urb(req, GFP_ATOMIC);
+	if (err) {
+		usb_free_urb(req);
+		goto drop;
+	}
+
+	spin_lock_irqsave(&pnd->tx_lock, flags);
+	pnd->tx_queue++;
+	if (pnd->tx_queue >= dev->tx_queue_len)
+		netif_stop_queue(dev);
+	spin_unlock_irqrestore(&pnd->tx_lock, flags);
+	return 0;
+
+drop:
+	dev_kfree_skb(skb);
+	dev->stats.tx_dropped++;
+	return 0;
+}
+
+static void tx_complete(struct urb *req)
+{
+	struct sk_buff *skb = req->context;
+	struct net_device *dev = skb->dev;
+	struct usbpn_dev *pnd = netdev_priv(dev);
+
+	switch (req->status) {
+	case 0:
+		dev->stats.tx_bytes += skb->len;
+		break;
+
+	case -ENOENT:
+	case -ECONNRESET:
+	case -ESHUTDOWN:
+		dev->stats.tx_aborted_errors++;
+	default:
+		dev->stats.tx_errors++;
+		dev_dbg(&dev->dev, "TX error (%d)\n", req->status);
+	}
+	dev->stats.tx_packets++;
+
+	spin_lock(&pnd->tx_lock);
+	pnd->tx_queue--;
+	netif_wake_queue(dev);
+	spin_unlock(&pnd->tx_lock);
+
+	dev_kfree_skb_any(skb);
+	usb_free_urb(req);
+}
+
+static int rx_submit(struct usbpn_dev *pnd, struct urb *req, gfp_t gfp_flags)
+{
+	struct net_device *dev = pnd->dev;
+	struct page *page;
+	int err;
+
+	page = __netdev_alloc_page(dev, gfp_flags);
+	if (!page)
+		return -ENOMEM;
+
+	usb_fill_bulk_urb(req, pnd->usb, pnd->rx_pipe, page_address(page),
+				PAGE_SIZE, rx_complete, dev);
+	req->transfer_flags = 0;
+	err = usb_submit_urb(req, gfp_flags);
+	if (unlikely(err)) {
+		dev_dbg(&dev->dev, "RX submit error (%d)\n", err);
+		netdev_free_page(dev, page);
+	}
+	return err;
+}
+
+static void rx_complete(struct urb *req)
+{
+	struct net_device *dev = req->context;
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	struct page *page = virt_to_page(req->transfer_buffer);
+	struct sk_buff *skb;
+	unsigned long flags;
+
+	switch (req->status) {
+	case 0:
+		spin_lock_irqsave(&pnd->rx_lock, flags);
+		skb = pnd->rx_skb;
+		if (!skb) {
+			skb = pnd->rx_skb = netdev_alloc_skb(dev, 12);
+			if (likely(skb)) {
+				/* Can't use pskb_pull() on page in IRQ */
+				memcpy(skb_put(skb, 1), page_address(page), 1);
+				skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
+						page, 1, req->actual_length);
+				page = NULL;
+			}
+		} else {
+			skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
+					page, 0, req->actual_length);
+			page = NULL;
+		}
+		if (req->actual_length < PAGE_SIZE)
+			pnd->rx_skb = NULL; /* Last fragment */
+		else
+			skb = NULL;
+		spin_unlock_irqrestore(&pnd->rx_lock, flags);
+		if (skb) {
+			skb->protocol = htons(ETH_P_PHONET);
+			skb_reset_mac_header(skb);
+			__skb_pull(skb, 1);
+			skb->dev = dev;
+			dev->stats.rx_packets++;
+			dev->stats.rx_bytes += skb->len;
+
+			netif_rx(skb);
+		}
+		goto resubmit;
+
+	case -ENOENT:
+	case -ECONNRESET:
+	case -ESHUTDOWN:
+		req = NULL;
+		break;
+
+	case -EOVERFLOW:
+		dev->stats.rx_over_errors++;
+		dev_dbg(&dev->dev, "RX overflow\n");
+		break;
+
+	case -EILSEQ:
+		dev->stats.rx_crc_errors++;
+		break;
+	}
+
+	dev->stats.rx_errors++;
+resubmit:
+	if (page)
+		netdev_free_page(dev, page);
+	if (req)
+		rx_submit(pnd, req, GFP_ATOMIC);
+}
+
+static int usbpn_close(struct net_device *dev);
+
+static int usbpn_open(struct net_device *dev)
+{
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	int err;
+	unsigned i;
+	unsigned num = pnd->data_intf->cur_altsetting->desc.bInterfaceNumber;
+
+	err = usb_set_interface(pnd->usb, num, pnd->active_setting);
+	if (err)
+		return err;
+
+	for (i = 0; i < rxq_size; i++) {
+		struct urb *req = usb_alloc_urb(0, GFP_KERNEL);
+
+		if (!req || rx_submit(pnd, req, GFP_KERNEL)) {
+			usbpn_close(dev);
+			return -ENOMEM;
+		}
+		pnd->urbs[i] = req;
+	}
+
+	netif_wake_queue(dev);
+	return 0;
+}
+
+static int usbpn_close(struct net_device *dev)
+{
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	unsigned i;
+	unsigned num = pnd->data_intf->cur_altsetting->desc.bInterfaceNumber;
+
+	netif_stop_queue(dev);
+
+	for (i = 0; i < rxq_size; i++) {
+		struct urb *req = pnd->urbs[i];
+
+		if (!req)
+			continue;
+		usb_kill_urb(req);
+		usb_free_urb(req);
+		pnd->urbs[i] = NULL;
+	}
+
+	return usb_set_interface(pnd->usb, num, !pnd->active_setting);
+}
+
+static int usbpn_set_mtu(struct net_device *dev, int new_mtu)
+{
+	if ((new_mtu < PHONET_MIN_MTU) || (new_mtu > PHONET_MAX_MTU))
+		return -EINVAL;
+
+	dev->mtu = new_mtu;
+	return 0;
+}
+
+static const struct net_device_ops usbpn_ops = {
+	.ndo_open	= usbpn_open,
+	.ndo_stop	= usbpn_close,
+	.ndo_start_xmit = usbpn_xmit,
+	.ndo_change_mtu = usbpn_set_mtu,
+};
+
+static void usbpn_setup(struct net_device *dev)
+{
+	dev->features		= 0;
+	dev->netdev_ops		= &usbpn_ops,
+	dev->header_ops		= &phonet_header_ops;
+	dev->type		= ARPHRD_PHONET;
+	dev->flags		= IFF_POINTOPOINT | IFF_NOARP;
+	dev->mtu		= PHONET_MAX_MTU;
+	dev->hard_header_len	= 1;
+	dev->dev_addr[0]	= PN_MEDIA_USB;
+	dev->addr_len		= 1;
+	dev->tx_queue_len	= 3;
+
+	dev->destructor		= free_netdev;
+}
+
+/*
+ * USB driver callbacks
+ */
+static struct usb_device_id usbpn_ids[] = {
+	{
+		.match_flags = USB_DEVICE_ID_MATCH_VENDOR
+			| USB_DEVICE_ID_MATCH_INT_CLASS
+			| USB_DEVICE_ID_MATCH_INT_SUBCLASS,
+		.idVendor = 0x0421, /* Nokia */
+		.bInterfaceClass = USB_CLASS_COMM,
+		.bInterfaceSubClass = 0xFE,
+	},
+	{ },
+};
+
+MODULE_DEVICE_TABLE(usb, usbpn_ids);
+
+static struct usb_driver usbpn_driver;
+
+int usbpn_probe(struct usb_interface *intf, const struct usb_device_id *id)
+{
+	static const char ifname[] = "usbpn%d";
+	const struct usb_cdc_union_desc *union_header = NULL;
+	const struct usb_cdc_header_desc *phonet_header = NULL;
+	const struct usb_host_interface *data_desc;
+	struct usb_interface *data_intf;
+	struct usb_device *usbdev = interface_to_usbdev(intf);
+	struct net_device *dev;
+	struct usbpn_dev *pnd;
+	u8 *data;
+	int len, err;
+
+	data = intf->altsetting->extra;
+	len = intf->altsetting->extralen;
+	while (len >= 3) {
+		u8 dlen = data[0];
+		if (dlen < 3)
+			return -EINVAL;
+
+		/* bDescriptorType */
+		if (data[1] == USB_DT_CS_INTERFACE) {
+			/* bDescriptorSubType */
+			switch (data[2]) {
+			case USB_CDC_UNION_TYPE:
+				if (union_header || dlen < 5)
+					break;
+				union_header =
+					(struct usb_cdc_union_desc *)data;
+				break;
+			case 0xAB:
+				if (phonet_header || dlen < 5)
+					break;
+				phonet_header =
+					(struct usb_cdc_header_desc *)data;
+				break;
+			}
+		}
+		data += dlen;
+		len -= dlen;
+	}
+
+	if (!union_header || !phonet_header)
+		return -EINVAL;
+
+	data_intf = usb_ifnum_to_if(usbdev, union_header->bSlaveInterface0);
+	if (data_intf == NULL)
+		return -ENODEV;
+	/* Data interface has one inactive and one active setting */
+	if (data_intf->num_altsetting != 2)
+		return -EINVAL;
+	if (data_intf->altsetting[0].desc.bNumEndpoints == 0
+	 && data_intf->altsetting[1].desc.bNumEndpoints == 2)
+		data_desc = data_intf->altsetting + 1;
+	else
+	if (data_intf->altsetting[0].desc.bNumEndpoints == 2
+	 && data_intf->altsetting[1].desc.bNumEndpoints == 0)
+		data_desc = data_intf->altsetting;
+	else
+		return -EINVAL;
+
+	dev = alloc_netdev(sizeof(*pnd) + sizeof(pnd->urbs[0]) * rxq_size,
+				ifname, usbpn_setup);
+	if (!dev)
+		return -ENOMEM;
+
+	pnd = netdev_priv(dev);
+	SET_NETDEV_DEV(dev, &intf->dev);
+	netif_stop_queue(dev);
+
+	pnd->dev = dev;
+	pnd->usb = usb_get_dev(usbdev);
+	pnd->intf = intf;
+	pnd->data_intf = data_intf;
+	spin_lock_init(&pnd->tx_lock);
+	spin_lock_init(&pnd->rx_lock);
+	/* Endpoints */
+	if (usb_pipein(data_desc->endpoint[0].desc.bEndpointAddress)) {
+		pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
+			data_desc->endpoint[0].desc.bEndpointAddress);
+		pnd->tx_pipe = usb_sndbulkpipe(usbdev,
+			data_desc->endpoint[1].desc.bEndpointAddress);
+	} else {
+		pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
+			data_desc->endpoint[1].desc.bEndpointAddress);
+		pnd->tx_pipe = usb_sndbulkpipe(usbdev,
+			data_desc->endpoint[0].desc.bEndpointAddress);
+	}
+	pnd->active_setting = data_desc - data_intf->altsetting;
+
+	err = usb_driver_claim_interface(&usbpn_driver, data_intf, pnd);
+	if (err)
+		goto out;
+
+	/* Force inactive mode until the network device is brought UP */
+	usb_set_interface(usbdev, union_header->bSlaveInterface0,
+				!pnd->active_setting);
+	usb_set_intfdata(intf, pnd);
+
+	err = register_netdev(dev);
+	if (err) {
+		usb_driver_release_interface(&usbpn_driver, data_intf);
+		goto out;
+	}
+
+	dev_dbg(&dev->dev, "USB CDC Phonet device found\n");
+	return 0;
+
+out:
+	usb_set_intfdata(intf, NULL);
+	free_netdev(dev);
+	return err;
+}
+
+static void usbpn_disconnect(struct usb_interface *intf)
+{
+	struct usbpn_dev *pnd = usb_get_intfdata(intf);
+	struct usb_device *usb = pnd->usb;
+
+	if (pnd->disconnected)
+		return;
+
+	pnd->disconnected = 1;
+	usb_driver_release_interface(&usbpn_driver,
+			(pnd->intf == intf) ? pnd->data_intf : pnd->intf);
+	unregister_netdev(pnd->dev);
+	usb_put_dev(usb);
+}
+
+static struct usb_driver usbpn_driver = {
+	.name =		"cdc_phonet",
+	.probe =	usbpn_probe,
+	.disconnect =	usbpn_disconnect,
+	.id_table =	usbpn_ids,
+};
+
+static int __init usbpn_init(void)
+{
+	return usb_register(&usbpn_driver);
+}
+
+static void __exit usbpn_exit(void)
+{
+	usb_deregister(&usbpn_driver);
+}
+
+module_init(usbpn_init);
+module_exit(usbpn_exit);
+
+MODULE_AUTHOR("Remi Denis-Courmont");
+MODULE_DESCRIPTION("USB CDC Phonet host interface");
+MODULE_LICENSE("GPL");
-- 
1.6.0.4

Re: [PATCH] USB host CDC Phonet network interface driver

[Dan Williams]

On Tue, 2009-07-21 at 14:58 +0300, Rémi Denis-Courmont wrote:
> From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> 
> Many Nokia handsets support a Phonet interface to the cellular modem
> via a vendor-specific USB interface. CDC Phonet follows the
> Communications Device Class model, with one control interface, and
> and a pair of inactive and active data alternative interface. The later
> has two bulk endpoint, one per direction.
> 
> This was tested against Nokia E61, Nokia N95, and the existing Phonet
> gadget function for the Linux composite USB gadget framework.

Is there an example somewhere of how to use Phonet to get a mobile
broadband connection in place of usb-serial and PPP?  I've read the
Phonet protocol description and other random docs I can find, but can't
figure out how that would work.  Or does "PC Suite" mode not support
that?

Thanks!
Dan


> Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> ---
>  drivers/net/usb/Kconfig      |    8 +
>  drivers/net/usb/Makefile     |    1 +
>  drivers/net/usb/cdc-phonet.c |  461 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 470 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/net/usb/cdc-phonet.c
> 
> diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
> index a906d39..c47237c 100644
> --- a/drivers/net/usb/Kconfig
> +++ b/drivers/net/usb/Kconfig
> @@ -369,4 +369,12 @@ config USB_NET_INT51X1
>  	  (Powerline Communications) solution with an Intellon
>  	  INT51x1/INT5200 chip, like the "devolo dLan duo".
>  
> +config USB_CDC_PHONET
> +	tristate "CDC Phonet support"
> +	depends on PHONET
> +	help
> +	  Choose this option to support the Phonet interface to a Nokia
> +	  cellular modem, as found on most Nokia handsets with the
> +	  "PC suite" USB profile.
> +
>  endmenu
> diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
> index b870b0b..e17afb7 100644
> --- a/drivers/net/usb/Makefile
> +++ b/drivers/net/usb/Makefile
> @@ -21,4 +21,5 @@ obj-$(CONFIG_USB_NET_ZAURUS)	+= zaurus.o
>  obj-$(CONFIG_USB_NET_MCS7830)	+= mcs7830.o
>  obj-$(CONFIG_USB_USBNET)	+= usbnet.o
>  obj-$(CONFIG_USB_NET_INT51X1)	+= int51x1.o
> +obj-$(CONFIG_USB_CDC_PHONET)	+= cdc-phonet.o
>  
> diff --git a/drivers/net/usb/cdc-phonet.c b/drivers/net/usb/cdc-phonet.c
> new file mode 100644
> index 0000000..792af72
> --- /dev/null
> +++ b/drivers/net/usb/cdc-phonet.c
> @@ -0,0 +1,461 @@
> +/*
> + * phonet.c -- USB CDC Phonet host driver
> + *
> + * Copyright (C) 2008-2009 Nokia Corporation. All rights reserved.
> + *
> + * Author: Rémi Denis-Courmont
> + *
> + * 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.
> + *
> + * 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., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110-1301 USA
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/usb.h>
> +#include <linux/usb/cdc.h>
> +#include <linux/netdevice.h>
> +#include <linux/if_arp.h>
> +#include <linux/if_phonet.h>
> +
> +#define PN_MEDIA_USB	0x1B
> +
> +static const unsigned rxq_size = 17;
> +
> +struct usbpn_dev {
> +	struct net_device	*dev;
> +
> +	struct usb_interface	*intf, *data_intf;
> +	struct usb_device	*usb;
> +	unsigned int		tx_pipe, rx_pipe;
> +	u8 active_setting;
> +	u8 disconnected;
> +
> +	unsigned		tx_queue;
> +	spinlock_t		tx_lock;
> +
> +	spinlock_t		rx_lock;
> +	struct sk_buff		*rx_skb;
> +	struct urb		*urbs[0];
> +};
> +
> +static void tx_complete(struct urb *req);
> +static void rx_complete(struct urb *req);
> +
> +/*
> + * Network device callbacks
> + */
> +static int usbpn_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct usbpn_dev *pnd = netdev_priv(dev);
> +	struct urb *req = NULL;
> +	unsigned long flags;
> +	int err;
> +
> +	if (skb->protocol != htons(ETH_P_PHONET))
> +		goto drop;
> +
> +	req = usb_alloc_urb(0, GFP_ATOMIC);
> +	if (!req)
> +		goto drop;
> +	usb_fill_bulk_urb(req, pnd->usb, pnd->tx_pipe, skb->data, skb->len,
> +				tx_complete, skb);
> +	req->transfer_flags = URB_ZERO_PACKET;
> +	err = usb_submit_urb(req, GFP_ATOMIC);
> +	if (err) {
> +		usb_free_urb(req);
> +		goto drop;
> +	}
> +
> +	spin_lock_irqsave(&pnd->tx_lock, flags);
> +	pnd->tx_queue++;
> +	if (pnd->tx_queue >= dev->tx_queue_len)
> +		netif_stop_queue(dev);
> +	spin_unlock_irqrestore(&pnd->tx_lock, flags);
> +	return 0;
> +
> +drop:
> +	dev_kfree_skb(skb);
> +	dev->stats.tx_dropped++;
> +	return 0;
> +}
> +
> +static void tx_complete(struct urb *req)
> +{
> +	struct sk_buff *skb = req->context;
> +	struct net_device *dev = skb->dev;
> +	struct usbpn_dev *pnd = netdev_priv(dev);
> +
> +	switch (req->status) {
> +	case 0:
> +		dev->stats.tx_bytes += skb->len;
> +		break;
> +
> +	case -ENOENT:
> +	case -ECONNRESET:
> +	case -ESHUTDOWN:
> +		dev->stats.tx_aborted_errors++;
> +	default:
> +		dev->stats.tx_errors++;
> +		dev_dbg(&dev->dev, "TX error (%d)\n", req->status);
> +	}
> +	dev->stats.tx_packets++;
> +
> +	spin_lock(&pnd->tx_lock);
> +	pnd->tx_queue--;
> +	netif_wake_queue(dev);
> +	spin_unlock(&pnd->tx_lock);
> +
> +	dev_kfree_skb_any(skb);
> +	usb_free_urb(req);
> +}
> +
> +static int rx_submit(struct usbpn_dev *pnd, struct urb *req, gfp_t gfp_flags)
> +{
> +	struct net_device *dev = pnd->dev;
> +	struct page *page;
> +	int err;
> +
> +	page = __netdev_alloc_page(dev, gfp_flags);
> +	if (!page)
> +		return -ENOMEM;
> +
> +	usb_fill_bulk_urb(req, pnd->usb, pnd->rx_pipe, page_address(page),
> +				PAGE_SIZE, rx_complete, dev);
> +	req->transfer_flags = 0;
> +	err = usb_submit_urb(req, gfp_flags);
> +	if (unlikely(err)) {
> +		dev_dbg(&dev->dev, "RX submit error (%d)\n", err);
> +		netdev_free_page(dev, page);
> +	}
> +	return err;
> +}
> +
> +static void rx_complete(struct urb *req)
> +{
> +	struct net_device *dev = req->context;
> +	struct usbpn_dev *pnd = netdev_priv(dev);
> +	struct page *page = virt_to_page(req->transfer_buffer);
> +	struct sk_buff *skb;
> +	unsigned long flags;
> +
> +	switch (req->status) {
> +	case 0:
> +		spin_lock_irqsave(&pnd->rx_lock, flags);
> +		skb = pnd->rx_skb;
> +		if (!skb) {
> +			skb = pnd->rx_skb = netdev_alloc_skb(dev, 12);
> +			if (likely(skb)) {
> +				/* Can't use pskb_pull() on page in IRQ */
> +				memcpy(skb_put(skb, 1), page_address(page), 1);
> +				skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
> +						page, 1, req->actual_length);
> +				page = NULL;
> +			}
> +		} else {
> +			skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
> +					page, 0, req->actual_length);
> +			page = NULL;
> +		}
> +		if (req->actual_length < PAGE_SIZE)
> +			pnd->rx_skb = NULL; /* Last fragment */
> +		else
> +			skb = NULL;
> +		spin_unlock_irqrestore(&pnd->rx_lock, flags);
> +		if (skb) {
> +			skb->protocol = htons(ETH_P_PHONET);
> +			skb_reset_mac_header(skb);
> +			__skb_pull(skb, 1);
> +			skb->dev = dev;
> +			dev->stats.rx_packets++;
> +			dev->stats.rx_bytes += skb->len;
> +
> +			netif_rx(skb);
> +		}
> +		goto resubmit;
> +
> +	case -ENOENT:
> +	case -ECONNRESET:
> +	case -ESHUTDOWN:
> +		req = NULL;
> +		break;
> +
> +	case -EOVERFLOW:
> +		dev->stats.rx_over_errors++;
> +		dev_dbg(&dev->dev, "RX overflow\n");
> +		break;
> +
> +	case -EILSEQ:
> +		dev->stats.rx_crc_errors++;
> +		break;
> +	}
> +
> +	dev->stats.rx_errors++;
> +resubmit:
> +	if (page)
> +		netdev_free_page(dev, page);
> +	if (req)
> +		rx_submit(pnd, req, GFP_ATOMIC);
> +}
> +
> +static int usbpn_close(struct net_device *dev);
> +
> +static int usbpn_open(struct net_device *dev)
> +{
> +	struct usbpn_dev *pnd = netdev_priv(dev);
> +	int err;
> +	unsigned i;
> +	unsigned num = pnd->data_intf->cur_altsetting->desc.bInterfaceNumber;
> +
> +	err = usb_set_interface(pnd->usb, num, pnd->active_setting);
> +	if (err)
> +		return err;
> +
> +	for (i = 0; i < rxq_size; i++) {
> +		struct urb *req = usb_alloc_urb(0, GFP_KERNEL);
> +
> +		if (!req || rx_submit(pnd, req, GFP_KERNEL)) {
> +			usbpn_close(dev);
> +			return -ENOMEM;
> +		}
> +		pnd->urbs[i] = req;
> +	}
> +
> +	netif_wake_queue(dev);
> +	return 0;
> +}
> +
> +static int usbpn_close(struct net_device *dev)
> +{
> +	struct usbpn_dev *pnd = netdev_priv(dev);
> +	unsigned i;
> +	unsigned num = pnd->data_intf->cur_altsetting->desc.bInterfaceNumber;
> +
> +	netif_stop_queue(dev);
> +
> +	for (i = 0; i < rxq_size; i++) {
> +		struct urb *req = pnd->urbs[i];
> +
> +		if (!req)
> +			continue;
> +		usb_kill_urb(req);
> +		usb_free_urb(req);
> +		pnd->urbs[i] = NULL;
> +	}
> +
> +	return usb_set_interface(pnd->usb, num, !pnd->active_setting);
> +}
> +
> +static int usbpn_set_mtu(struct net_device *dev, int new_mtu)
> +{
> +	if ((new_mtu < PHONET_MIN_MTU) || (new_mtu > PHONET_MAX_MTU))
> +		return -EINVAL;
> +
> +	dev->mtu = new_mtu;
> +	return 0;
> +}
> +
> +static const struct net_device_ops usbpn_ops = {
> +	.ndo_open	= usbpn_open,
> +	.ndo_stop	= usbpn_close,
> +	.ndo_start_xmit = usbpn_xmit,
> +	.ndo_change_mtu = usbpn_set_mtu,
> +};
> +
> +static void usbpn_setup(struct net_device *dev)
> +{
> +	dev->features		= 0;
> +	dev->netdev_ops		= &usbpn_ops,
> +	dev->header_ops		= &phonet_header_ops;
> +	dev->type		= ARPHRD_PHONET;
> +	dev->flags		= IFF_POINTOPOINT | IFF_NOARP;
> +	dev->mtu		= PHONET_MAX_MTU;
> +	dev->hard_header_len	= 1;
> +	dev->dev_addr[0]	= PN_MEDIA_USB;
> +	dev->addr_len		= 1;
> +	dev->tx_queue_len	= 3;
> +
> +	dev->destructor		= free_netdev;
> +}
> +
> +/*
> + * USB driver callbacks
> + */
> +static struct usb_device_id usbpn_ids[] = {
> +	{
> +		.match_flags = USB_DEVICE_ID_MATCH_VENDOR
> +			| USB_DEVICE_ID_MATCH_INT_CLASS
> +			| USB_DEVICE_ID_MATCH_INT_SUBCLASS,
> +		.idVendor = 0x0421, /* Nokia */
> +		.bInterfaceClass = USB_CLASS_COMM,
> +		.bInterfaceSubClass = 0xFE,
> +	},
> +	{ },
> +};
> +
> +MODULE_DEVICE_TABLE(usb, usbpn_ids);
> +
> +static struct usb_driver usbpn_driver;
> +
> +int usbpn_probe(struct usb_interface *intf, const struct usb_device_id *id)
> +{
> +	static const char ifname[] = "usbpn%d";
> +	const struct usb_cdc_union_desc *union_header = NULL;
> +	const struct usb_cdc_header_desc *phonet_header = NULL;
> +	const struct usb_host_interface *data_desc;
> +	struct usb_interface *data_intf;
> +	struct usb_device *usbdev = interface_to_usbdev(intf);
> +	struct net_device *dev;
> +	struct usbpn_dev *pnd;
> +	u8 *data;
> +	int len, err;
> +
> +	data = intf->altsetting->extra;
> +	len = intf->altsetting->extralen;
> +	while (len >= 3) {
> +		u8 dlen = data[0];
> +		if (dlen < 3)
> +			return -EINVAL;
> +
> +		/* bDescriptorType */
> +		if (data[1] == USB_DT_CS_INTERFACE) {
> +			/* bDescriptorSubType */
> +			switch (data[2]) {
> +			case USB_CDC_UNION_TYPE:
> +				if (union_header || dlen < 5)
> +					break;
> +				union_header =
> +					(struct usb_cdc_union_desc *)data;
> +				break;
> +			case 0xAB:
> +				if (phonet_header || dlen < 5)
> +					break;
> +				phonet_header =
> +					(struct usb_cdc_header_desc *)data;
> +				break;
> +			}
> +		}
> +		data += dlen;
> +		len -= dlen;
> +	}
> +
> +	if (!union_header || !phonet_header)
> +		return -EINVAL;
> +
> +	data_intf = usb_ifnum_to_if(usbdev, union_header->bSlaveInterface0);
> +	if (data_intf == NULL)
> +		return -ENODEV;
> +	/* Data interface has one inactive and one active setting */
> +	if (data_intf->num_altsetting != 2)
> +		return -EINVAL;
> +	if (data_intf->altsetting[0].desc.bNumEndpoints == 0
> +	 && data_intf->altsetting[1].desc.bNumEndpoints == 2)
> +		data_desc = data_intf->altsetting + 1;
> +	else
> +	if (data_intf->altsetting[0].desc.bNumEndpoints == 2
> +	 && data_intf->altsetting[1].desc.bNumEndpoints == 0)
> +		data_desc = data_intf->altsetting;
> +	else
> +		return -EINVAL;
> +
> +	dev = alloc_netdev(sizeof(*pnd) + sizeof(pnd->urbs[0]) * rxq_size,
> +				ifname, usbpn_setup);
> +	if (!dev)
> +		return -ENOMEM;
> +
> +	pnd = netdev_priv(dev);
> +	SET_NETDEV_DEV(dev, &intf->dev);
> +	netif_stop_queue(dev);
> +
> +	pnd->dev = dev;
> +	pnd->usb = usb_get_dev(usbdev);
> +	pnd->intf = intf;
> +	pnd->data_intf = data_intf;
> +	spin_lock_init(&pnd->tx_lock);
> +	spin_lock_init(&pnd->rx_lock);
> +	/* Endpoints */
> +	if (usb_pipein(data_desc->endpoint[0].desc.bEndpointAddress)) {
> +		pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
> +			data_desc->endpoint[0].desc.bEndpointAddress);
> +		pnd->tx_pipe = usb_sndbulkpipe(usbdev,
> +			data_desc->endpoint[1].desc.bEndpointAddress);
> +	} else {
> +		pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
> +			data_desc->endpoint[1].desc.bEndpointAddress);
> +		pnd->tx_pipe = usb_sndbulkpipe(usbdev,
> +			data_desc->endpoint[0].desc.bEndpointAddress);
> +	}
> +	pnd->active_setting = data_desc - data_intf->altsetting;
> +
> +	err = usb_driver_claim_interface(&usbpn_driver, data_intf, pnd);
> +	if (err)
> +		goto out;
> +
> +	/* Force inactive mode until the network device is brought UP */
> +	usb_set_interface(usbdev, union_header->bSlaveInterface0,
> +				!pnd->active_setting);
> +	usb_set_intfdata(intf, pnd);
> +
> +	err = register_netdev(dev);
> +	if (err) {
> +		usb_driver_release_interface(&usbpn_driver, data_intf);
> +		goto out;
> +	}
> +
> +	dev_dbg(&dev->dev, "USB CDC Phonet device found\n");
> +	return 0;
> +
> +out:
> +	usb_set_intfdata(intf, NULL);
> +	free_netdev(dev);
> +	return err;
> +}
> +
> +static void usbpn_disconnect(struct usb_interface *intf)
> +{
> +	struct usbpn_dev *pnd = usb_get_intfdata(intf);
> +	struct usb_device *usb = pnd->usb;
> +
> +	if (pnd->disconnected)
> +		return;
> +
> +	pnd->disconnected = 1;
> +	usb_driver_release_interface(&usbpn_driver,
> +			(pnd->intf == intf) ? pnd->data_intf : pnd->intf);
> +	unregister_netdev(pnd->dev);
> +	usb_put_dev(usb);
> +}
> +
> +static struct usb_driver usbpn_driver = {
> +	.name =		"cdc_phonet",
> +	.probe =	usbpn_probe,
> +	.disconnect =	usbpn_disconnect,
> +	.id_table =	usbpn_ids,
> +};
> +
> +static int __init usbpn_init(void)
> +{
> +	return usb_register(&usbpn_driver);
> +}
> +
> +static void __exit usbpn_exit(void)
> +{
> +	usb_deregister(&usbpn_driver);
> +}
> +
> +module_init(usbpn_init);
> +module_exit(usbpn_exit);
> +
> +MODULE_AUTHOR("Remi Denis-Courmont");
> +MODULE_DESCRIPTION("USB CDC Phonet host interface");
> +MODULE_LICENSE("GPL");

Re: [PATCH] USB host CDC Phonet network interface driver

[Marcel Holtmann]

Hi Dan,

> > Many Nokia handsets support a Phonet interface to the cellular modem
> > via a vendor-specific USB interface. CDC Phonet follows the
> > Communications Device Class model, with one control interface, and
> > and a pair of inactive and active data alternative interface. The later
> > has two bulk endpoint, one per direction.
> > 
> > This was tested against Nokia E61, Nokia N95, and the existing Phonet
> > gadget function for the Linux composite USB gadget framework.
> 
> Is there an example somewhere of how to use Phonet to get a mobile
> broadband connection in place of usb-serial and PPP?  I've read the
> Phonet protocol description and other random docs I can find, but can't
> figure out how that would work.  Or does "PC Suite" mode not support
> that?

I talked with Remi about it and he told me that currently there is no
public example on how to do it. Also it involves a bit more event
handling and other details via ISI modem library. At some point we will
have an example inside oFono on how to do it, but not right now.

Regards

Marcel


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

Re: [PATCH] USB host CDC Phonet network interface driver

[Rémi Denis-Courmont]

On Tuesday 21 July 2009 16:52:53 ext Dan Williams wrote:
> > This was tested against Nokia E61, Nokia N95, and the existing Phonet
> > gadget function for the Linux composite USB gadget framework.
>
> Is there an example somewhere of how to use Phonet to get a mobile
> broadband connection in place of usb-serial and PPP?

Not yet, I'm afraid.

> I've read the
> Phonet protocol description and other random docs I can find, but can't
> figure out how that would work.  Or does "PC Suite" mode not support
> that?

PC suite does provide access to packet data. The existing pn_pep kernel module 
provides data path (IPv4 or IPv6 packets) and flow control. But GPRS context 
credentials, IP/DNS parameters, setup/teardown, etc, are missing. I am not 
sure where the user/kernel split should be here...

I guess in a perfect world, Linux would provide a common control interface for 
GPRS as it already does WiFi (and maybe soon will for WiMAX?). But I am not 
sure we can unify AT+PPP, Phonet, AT+Ethernet and who knows what else, behind 
a common Netlink interface, especially as AT commands have always been done in 
userland anyway.

Or what did you have in mind?

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki


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

Re: [PATCH] USB host CDC Phonet network interface driver

[Alan Cox]

> provides data path (IPv4 or IPv6 packets) and flow control. But GPRS context 
> credentials, IP/DNS parameters, setup/teardown, etc, are missing. I am not 
> sure where the user/kernel split should be here...

As much as possible in user space for set up and tear down processing of
connections (negotiation etc). I've been poking in the background at a
kernel side GSM mux implementation but keep getting side tracked into
other work. other than the mux (where you have 3G stuff with high data
rates to demux efficiently) I'm not sure much else should be kernel side.

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

Re: [PATCH] USB host CDC Phonet network interface driver

[Oliver Neukum]

Am Mittwoch, 22. Juli 2009 10:37:06 schrieb Rémi Denis-Courmont:
> I guess in a perfect world, Linux would provide a common control interface
> for GPRS as it already does WiFi (and maybe soon will for WiMAX?). But I am
> not sure we can unify AT+PPP, Phonet, AT+Ethernet and who knows what else,
> behind a common Netlink interface, especially as AT commands have always
> been done in userland anyway.

The AT stuff is really problematic. Look at the hoops ISDN and software
modem drivers go through to emulate AT commands. I know even of a CDC-ACM
modem which can't deal with AT commands inline (that's within spec).
It seems to me we should have a modem API in kernel.

	Regards
		Oliver

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

Re: [PATCH] USB host CDC Phonet network interface driver

[Alan Cox]

> The AT stuff is really problematic. Look at the hoops ISDN and software
> modem drivers go through to emulate AT commands. I know even of a CDC-ACM
> modem which can't deal with AT commands inline (that's within spec).
> It seems to me we should have a modem API in kernel.

For devices which don't deal in AT commands probably but for devices
whose firmware provides an AT command interface over serial I would
disagree.

Re: [PATCH] USB host CDC Phonet network interface driver

[Marcel Holtmann]

Hi Alan,

> > The AT stuff is really problematic. Look at the hoops ISDN and software
> > modem drivers go through to emulate AT commands. I know even of a CDC-ACM
> > modem which can't deal with AT commands inline (that's within spec).
> > It seems to me we should have a modem API in kernel.
> 
> For devices which don't deal in AT commands probably but for devices
> whose firmware provides an AT command interface over serial I would
> disagree.

I fully agree here. Even if you think you get AT commands under control,
you really won't in the end. That standard is so wildly mis-interpreted
that it is not even funny anymore.

Regards

Marcel

Re: [PATCH] USB host CDC Phonet network interface driver

[Rémi Denis-Courmont]

On Wednesday 22 July 2009 12:15:34 ext Marcel Holtmann wrote:
> Hi Alan,
>
> > > The AT stuff is really problematic. Look at the hoops ISDN and software
> > > modem drivers go through to emulate AT commands. I know even of a
> > > CDC-ACM modem which can't deal with AT commands inline (that's within
> > > spec). It seems to me we should have a modem API in kernel.
> >
> > For devices which don't deal in AT commands probably but for devices
> > whose firmware provides an AT command interface over serial I would
> > disagree.
>
> I fully agree here. Even if you think you get AT commands under control,
> you really won't in the end. That standard is so wildly mis-interpreted
> that it is not even funny anymore.

Possibly. Does anyone known of any "species" bypassing AT commands entirely, 
other than Phonet? as far as GPRS is concerned?

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki

Re: [PATCH] USB host CDC Phonet network interface driver

[Dan Williams]

On Wed, 2009-07-22 at 14:07 +0300, Rémi Denis-Courmont wrote:
> On Wednesday 22 July 2009 12:15:34 ext Marcel Holtmann wrote:
> > Hi Alan,
> >
> > > > The AT stuff is really problematic. Look at the hoops ISDN and software
> > > > modem drivers go through to emulate AT commands. I know even of a
> > > > CDC-ACM modem which can't deal with AT commands inline (that's within
> > > > spec). It seems to me we should have a modem API in kernel.
> > >
> > > For devices which don't deal in AT commands probably but for devices
> > > whose firmware provides an AT command interface over serial I would
> > > disagree.
> >
> > I fully agree here. Even if you think you get AT commands under control,
> > you really won't in the end. That standard is so wildly mis-interpreted
> > that it is not even funny anymore.
> 
> Possibly. Does anyone known of any "species" bypassing AT commands entirely, 
> other than Phonet? as far as GPRS is concerned?

There are any number of devices that expose only one AT port (which of
course gets used by PPP for data) and then a proprietary port.  The
proprietary port usually uses a custom USB protocol that can also handle
SMS, status, RSSI, etc while the AT/PPP port is being used by ppp.

So if that's the case for phonet, maybe you just want to provide either
(a) documentation of the setup/status/sms/etc protocols, or (b) a shim
library implementation for them that handles communication with the
device itself.

But IMHO, the call setup stuff is probably better done from userspace.
It's a really hard API to abstract.  I guess you could write a netlink
API for it like we have for 802.11, but with GSM/CDMA the standards are
so widely ignored that it's just going to be impossible to get a stable
kernel API for anything like this.  The actual logic and higher-level
stuff has gotta be in userspace, while the low-level communication
channel bits (serial, USB, phonet, etc) should live in the kernel.

Dan

Re: [PATCH] USB host CDC Phonet network interface driver

[Rémi Denis-Courmont]

On Thursday 23 July 2009 17:34:52 ext Dan Williams wrote:
> There are any number of devices that expose only one AT port (which of
> course gets used by PPP for data) and then a proprietary port.  The
> proprietary port usually uses a custom USB protocol that can also handle
> SMS, status, RSSI, etc while the AT/PPP port is being used by ppp.
>
> So if that's the case for phonet, maybe you just want to provide either
> (a) documentation of the setup/status/sms/etc protocols, or (b) a shim
> library implementation for them that handles communication with the
> device itself.

Eventually, Ofono should get the missing bits for GPRS-via-Phonet.

> But IMHO, the call setup stuff is probably better done from userspace.

I was referring to cellular data connectivity. Indeed, I wouldn't propose 
Netlink for network selection and circuit-switched services like SMS or voice 
calls :)

> It's a really hard API to abstract.  I guess you could write a netlink
> API for it like we have for 802.11, but with GSM/CDMA the standards are
> so widely ignored that it's just going to be impossible to get a stable
> kernel API for anything like this.

Really, you only need a way to:
- create a device, e.g. like IPIP tunnel devices,
- set it up: access point name, and optionally username and password,
- bring it up, get IP and DNS configurations from the network,
- watch for context suspend events: IFF_NOCARRIER could do that,
- and eventually tear down.

In fact, it seems much simpler than 802.11. Well OK, I ignored "details" such 
as secondary contexts and quality of service.

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki

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

Re: [PATCH] USB host CDC Phonet network interface driver

[Oliver Neukum]

Am Mittwoch, 22. Juli 2009 11:15:34 schrieb Marcel Holtmann:
> > > The AT stuff is really problematic. Look at the hoops ISDN and software
> > > modem drivers go through to emulate AT commands. I know even of a
> > > CDC-ACM modem which can't deal with AT commands inline (that's within
> > > spec). It seems to me we should have a modem API in kernel.
> >
> > For devices which don't deal in AT commands probably but for devices
> > whose firmware provides an AT command interface over serial I would
> > disagree.
>
> I fully agree here. Even if you think you get AT commands under control,
> you really won't in the end. That standard is so wildly mis-interpreted
> that it is not even funny anymore.

For these devices we could at least separate the data channel from the
control channel.

	Regards
		Oliver

Re: [PATCH] USB host CDC Phonet network interface driver

[Rémi Denis-Courmont]

On Friday 24 July 2009 15:01:19 ext Oliver Neukum wrote:
> Am Mittwoch, 22. Juli 2009 11:15:34 schrieb Marcel Holtmann:
> > > > The AT stuff is really problematic. Look at the hoops ISDN and
> > > > software modem drivers go through to emulate AT commands. I know even
> > > > of a CDC-ACM modem which can't deal with AT commands inline (that's
> > > > within spec). It seems to me we should have a modem API in kernel.
> > >
> > > For devices which don't deal in AT commands probably but for devices
> > > whose firmware provides an AT command interface over serial I would
> > > disagree.
> >
> > I fully agree here. Even if you think you get AT commands under control,
> > you really won't in the end. That standard is so wildly mis-interpreted
> > that it is not even funny anymore.
>
> For these devices we could at least separate the data channel from the
> control channel.

"These" as in AT devices? You want a line discipline to multiplex AT commands 
inspite of PPP? I wonder if that'd work.

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki

Re: [PATCH] USB host CDC Phonet network interface driver

[Oliver Neukum]

Am Freitag, 24. Juli 2009 14:14:17 schrieb Rémi Denis-Courmont:
> On Friday 24 July 2009 15:01:19 ext Oliver Neukum wrote:
> > Am Mittwoch, 22. Juli 2009 11:15:34 schrieb Marcel Holtmann:
> > > > > The AT stuff is really problematic. Look at the hoops ISDN and
> > > > > software modem drivers go through to emulate AT commands. I know
> > > > > even of a CDC-ACM modem which can't deal with AT commands inline
> > > > > (that's within spec). It seems to me we should have a modem API in
> > > > > kernel.
> > > >
> > > > For devices which don't deal in AT commands probably but for devices
> > > > whose firmware provides an AT command interface over serial I would
> > > > disagree.
> > >
> > > I fully agree here. Even if you think you get AT commands under
> > > control, you really won't in the end. That standard is so wildly
> > > mis-interpreted that it is not even funny anymore.
> >
> > For these devices we could at least separate the data channel from the
> > control channel.
>
> "These" as in AT devices? You want a line discipline to multiplex AT
> commands inspite of PPP? I wonder if that'd work.

No, I was thinking of having two full devices, a data channel and a control
channel for devices that really talk AT commands natively.

	Regards
		Oliver

Re: [PATCH] USB host CDC Phonet network interface driver

[Alan Cox]

> No, I was thinking of having two full devices, a data channel and a control
> channel for devices that really talk AT commands natively.

If the hardware does it great, however for things like a 3G modem you
have the problem that the PPP is over the AT command channel which may
itself be multiplexed. And the muxing in question is *ugly* - sort of
HDLC and LAP-B done wrong.

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

Re: [PATCH] USB host CDC Phonet network interface driver

[Rémi Denis-Courmont]

On Friday 24 July 2009 15:48:05 ext Alan Cox wrote:
> > No, I was thinking of having two full devices, a data channel and a
> > control channel for devices that really talk AT commands natively.
>
> If the hardware does it great, however for things like a 3G modem you
> have the problem that the PPP is over the AT command channel which may
> itself be multiplexed. And the muxing in question is *ugly* - sort of
> HDLC and LAP-B done wrong.

I guess he meant that AT commands could be sent out-of-band, for those USB 
modems that fully implement the ACM profile of CDC, if any.

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki

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

Re: [PATCH] USB host CDC Phonet network interface driver

[Oliver Neukum]

Am Freitag, 24. Juli 2009 14:48:05 schrieb Alan Cox:
> > No, I was thinking of having two full devices, a data channel and a
> > control channel for devices that really talk AT commands natively.
>
> If the hardware does it great, however for things like a 3G modem you
> have the problem that the PPP is over the AT command channel which may
> itself be multiplexed. And the muxing in question is *ugly* - sort of
> HDLC and LAP-B done wrong.

Well, yes, but we would really like a separate control channel, so we
can query parameters like signal strength, while we do PPP over the data
channel.

	Regards
		Oliver

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

Re: [PATCH] USB host CDC Phonet network interface driver

[Alan Cox]

On Fri, 24 Jul 2009 16:12:41 +0200
Oliver Neukum <oliver@neukum.org> wrote:

> Am Freitag, 24. Juli 2009 14:48:05 schrieb Alan Cox:
> > > No, I was thinking of having two full devices, a data channel and a
> > > control channel for devices that really talk AT commands natively.
> >
> > If the hardware does it great, however for things like a 3G modem you
> > have the problem that the PPP is over the AT command channel which may
> > itself be multiplexed. And the muxing in question is *ugly* - sort of
> > HDLC and LAP-B done wrong.
> 
> Well, yes, but we would really like a separate control channel, so we
> can query parameters like signal strength, while we do PPP over the data
> channel.

Thats the notion of the AT mux stuff - you get multiple AT channels and
you can stuff PPP down one of them. It's still butt ugly.

Re: [PATCH] USB host CDC Phonet network interface driver

[Oliver Neukum]

Am Freitag, 24. Juli 2009 16:19:41 schrieb Alan Cox:
> On Fri, 24 Jul 2009 16:12:41 +0200
>
> Oliver Neukum <oliver@neukum.org> wrote:
> > Am Freitag, 24. Juli 2009 14:48:05 schrieb Alan Cox:
> > > > No, I was thinking of having two full devices, a data channel and a
> > > > control channel for devices that really talk AT commands natively.
> > >
> > > If the hardware does it great, however for things like a 3G modem you
> > > have the problem that the PPP is over the AT command channel which may
> > > itself be multiplexed. And the muxing in question is *ugly* - sort of
> > > HDLC and LAP-B done wrong.
> >
> > Well, yes, but we would really like a separate control channel, so we
> > can query parameters like signal strength, while we do PPP over the data
> > channel.
>
> Thats the notion of the AT mux stuff - you get multiple AT channels and
> you can stuff PPP down one of them. It's still butt ugly.

That is a problem. But is it so ugly that we'd forgo the added functionality
rather than implement it in the kernel?
If not, I maintain that we should not maintain several different control channel
interfaces, like some devices use cdc-wdm, others a ttyUSB and the AT mux
stuff shall use yet a different method.

	Regards
		Oliver

Re: [PATCH] USB host CDC Phonet network interface driver

[Marcel Holtmann]

Hi Oliver,

> > > No, I was thinking of having two full devices, a data channel and a
> > > control channel for devices that really talk AT commands natively.
> >
> > If the hardware does it great, however for things like a 3G modem you
> > have the problem that the PPP is over the AT command channel which may
> > itself be multiplexed. And the muxing in question is *ugly* - sort of
> > HDLC and LAP-B done wrong.
> 
> Well, yes, but we would really like a separate control channel, so we
> can query parameters like signal strength, while we do PPP over the data
> channel.

we don't want PPP at all. It is just plain stupid and a total braindead
idea. Non of the GSM/UMTS networks talk PPP over the air interface or
actually anywhere in their stack. The PPP is just between the host OS
and the card. It is a pointless encapsulation of IP packets that comes
from the POTS stuff where PPP over a telephone line made sense.

And on top of that we have these magic *99# phone numbers to establish a
PDP context, because the OS still sees them as POTS modem. All stupid
and braindead. I am so happy that Ericsson and Option moved to proper
high speed network devices and that Nokia Phonet had this all along.

So besides the GPRS data access, the other problem with AT command
control channels is that they are by no means async. Every single
command is essentially blocking and so you need the TTY mux anyway so
you can do scanning, text messaging and network monitoring at the same
time without having your application look like a total dork.

Regards

Marcel

Re: [PATCH] USB host CDC Phonet network interface driver

[Alan Cox]

> we don't want PPP at all. It is just plain stupid and a total braindead
> idea. Non of the GSM/UMTS networks talk PPP over the air interface or
> actually anywhere in their stack. The PPP is just between the host OS
> and the card. It is a pointless encapsulation of IP packets that comes
> from the POTS stuff where PPP over a telephone line made sense.

Well yes. A 3G USB modem is a lovely example of compatibility gone mad

It runs

	an emulated PPP session
	over an emulated serial port
	over a serial port multiplex emulating multiple serial ports
	over an emulated serial port
	over a USB link which has perfectly good packet facilities
	into the Linux kernel
	which makes it look like a legacy modem device
	which is managed by a set of software tools written for 9600 baud
		modems


Unfortunately fixing that has to start at the vendor firmware end, and
that is a lot of interfaces for SMS, voice dialing, fax and god knows
what else as well as data.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] USB host CDC Phonet network interface driver

[Dan Williams]

On Fri, 2009-07-24 at 20:00 +0200, Marcel Holtmann wrote:
> Hi Oliver,
> 
> > > > No, I was thinking of having two full devices, a data channel and a
> > > > control channel for devices that really talk AT commands natively.
> > >
> > > If the hardware does it great, however for things like a 3G modem you
> > > have the problem that the PPP is over the AT command channel which may
> > > itself be multiplexed. And the muxing in question is *ugly* - sort of
> > > HDLC and LAP-B done wrong.
> > 
> > Well, yes, but we would really like a separate control channel, so we
> > can query parameters like signal strength, while we do PPP over the data
> > channel.
> 
> we don't want PPP at all. It is just plain stupid and a total braindead
> idea. Non of the GSM/UMTS networks talk PPP over the air interface or
> actually anywhere in their stack. The PPP is just between the host OS
> and the card. It is a pointless encapsulation of IP packets that comes
> from the POTS stuff where PPP over a telephone line made sense.

Feel free to convince all the hardware vendors to move over to that
model.  Many of them are, but there are still *boatloads* of devices
that do PPP.  It'll be at least 3 or 4 years before you can even think
about ignoring PPP entirely.

This is totally a firmware thing and not something under our control at
all.  Either the vendor implements a non-PPP data channel, or they
don't.  We have to live with that, or ignore devices that use PPP.  Most
of the devices out there use PPP.  Maybe 80 or 90%.

The only reason vendors ditched PPP was that it was too much overhead to
achieve full speed on HSDPA 7.2 networks.  Guess how many operators have
actually deployed HSDPA 7.2?  Count them on your hands.  Yes, over the
next year or two we'll see a lot more HSDPA 7.2-capable networks, and
that means more devices will show up that ditch PPP.  But at the moment,
PPP can't be ignored.

The next problem is that not all vendors implement the non-PPP data
channel using cdc-ether, or provide specs/drivers for that channel.  So
just because a vendor ditches PPP doesn't automatically mean it's better
for Linux and a driver is available.

Dan

> And on top of that we have these magic *99# phone numbers to establish a
> PDP context, because the OS still sees them as POTS modem. All stupid
> and braindead. I am so happy that Ericsson and Option moved to proper
> high speed network devices and that Nokia Phonet had this all along.
> 
> So besides the GPRS data access, the other problem with AT command
> control channels is that they are by no means async. Every single
> command is essentially blocking and so you need the TTY mux anyway so
> you can do scanning, text messaging and network monitoring at the same
> time without having your application look like a total dork.
> 
> Regards
> 
> Marcel
> 
> 

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

Re: [PATCH] USB host CDC Phonet network interface driver

[Marcel Holtmann]

Hi Dan,

> > > > > No, I was thinking of having two full devices, a data channel and a
> > > > > control channel for devices that really talk AT commands natively.
> > > >
> > > > If the hardware does it great, however for things like a 3G modem you
> > > > have the problem that the PPP is over the AT command channel which may
> > > > itself be multiplexed. And the muxing in question is *ugly* - sort of
> > > > HDLC and LAP-B done wrong.
> > > 
> > > Well, yes, but we would really like a separate control channel, so we
> > > can query parameters like signal strength, while we do PPP over the data
> > > channel.
> > 
> > we don't want PPP at all. It is just plain stupid and a total braindead
> > idea. Non of the GSM/UMTS networks talk PPP over the air interface or
> > actually anywhere in their stack. The PPP is just between the host OS
> > and the card. It is a pointless encapsulation of IP packets that comes
> > from the POTS stuff where PPP over a telephone line made sense.
> 
> Feel free to convince all the hardware vendors to move over to that
> model.  Many of them are, but there are still *boatloads* of devices
> that do PPP.  It'll be at least 3 or 4 years before you can even think
> about ignoring PPP entirely.
> 
> This is totally a firmware thing and not something under our control at
> all.  Either the vendor implements a non-PPP data channel, or they
> don't.  We have to live with that, or ignore devices that use PPP.  Most
> of the devices out there use PPP.  Maybe 80 or 90%.

I know that and it is sad. It is also sad that a firmware upgrade could
bring most cards away from PPP, but besides Option I haven't seen it
from anybody.

> The only reason vendors ditched PPP was that it was too much overhead to
> achieve full speed on HSDPA 7.2 networks.  Guess how many operators have
> actually deployed HSDPA 7.2?  Count them on your hands.  Yes, over the
> next year or two we'll see a lot more HSDPA 7.2-capable networks, and
> that means more devices will show up that ditch PPP.  But at the moment,
> PPP can't be ignored.

Actually HSDPA 7.2 is deployed more than you think. At least in Europe
the base stations in the big cities are 7.2 capable. Some of them even
with proper HSUPA support. It is impressive what you can get through
these networks.

> The next problem is that not all vendors implement the non-PPP data
> channel using cdc-ether, or provide specs/drivers for that channel.  So
> just because a vendor ditches PPP doesn't automatically mean it's better
> for Linux and a driver is available.

The CDC Ethernet support of the Ericsson MBM cards is kinda a nice idea.
You get proper IFF_LOWER_UP and can just run DHCP on it. I am not such a
big fan of the DHCP part, but it is the same for WiMAX, so I assume that
is what they will be settling for. Nice would to have just plain and
simple IPv6 on UMTS networks.

Regards

Marcel

[PATCH] USB host CDC Phonet network interface driver

[Rémi Denis-Courmont]

From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

Many Nokia handsets support a Phonet interface to the cellular modem
via a vendor-specific USB interface. CDC Phonet follows the
Communications Device Class model, with one control interface, and
and a pair of inactive and active data alternative interface. The later
has two bulk endpoint, one per direction.

This was tested against Nokia E61, Nokia N95, and the existing Phonet
gadget function for the Linux composite USB gadget framework.

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 drivers/net/usb/Kconfig      |    8 +
 drivers/net/usb/Makefile     |    1 +
 drivers/net/usb/cdc-phonet.c |  454 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 463 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/usb/cdc-phonet.c

diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index a906d39..c47237c 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -369,4 +369,12 @@ config USB_NET_INT51X1
 	  (Powerline Communications) solution with an Intellon
 	  INT51x1/INT5200 chip, like the "devolo dLan duo".
 
+config USB_CDC_PHONET
+	tristate "CDC Phonet support"
+	depends on PHONET
+	help
+	  Choose this option to support the Phonet interface to a Nokia
+	  cellular modem, as found on most Nokia handsets with the
+	  "PC suite" USB profile.
+
 endmenu
diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index b870b0b..e17afb7 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -21,4 +21,5 @@ obj-$(CONFIG_USB_NET_ZAURUS)	+= zaurus.o
 obj-$(CONFIG_USB_NET_MCS7830)	+= mcs7830.o
 obj-$(CONFIG_USB_USBNET)	+= usbnet.o
 obj-$(CONFIG_USB_NET_INT51X1)	+= int51x1.o
+obj-$(CONFIG_USB_CDC_PHONET)	+= cdc-phonet.o
 
diff --git a/drivers/net/usb/cdc-phonet.c b/drivers/net/usb/cdc-phonet.c
new file mode 100644
index 0000000..a22d2c9
--- /dev/null
+++ b/drivers/net/usb/cdc-phonet.c
@@ -0,0 +1,454 @@
+/*
+ * phonet.c -- USB CDC Phonet host driver
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation. All rights reserved.
+ *
+ * Author: Rémi Denis-Courmont
+ *
+ * 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.
+ *
+ * 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., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/usb.h>
+#include <linux/usb/cdc.h>
+#include <linux/netdevice.h>
+#include <linux/if_arp.h>
+#include <linux/if_phonet.h>
+
+#define PN_MEDIA_USB	0x1B
+
+static const unsigned rxq_size = 17;
+
+struct usbpn_dev {
+	struct net_device	*dev;
+	atomic_t		tx_queue;
+
+	struct usb_interface	*intf, *data_intf;
+	struct usb_device	*usb;
+	unsigned int		tx_pipe, rx_pipe;
+	u8 active_setting;
+	u8 disconnected;
+
+	spinlock_t		rx_lock;
+	struct sk_buff		*rx_skb;
+	struct urb		*urbs[0];
+};
+
+static void tx_complete(struct urb *req);
+static void rx_complete(struct urb *req);
+
+/*
+ * Network device callbacks
+ */
+static int usbpn_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	struct urb *req = NULL;
+	int err;
+
+	if (skb->protocol != htons(ETH_P_PHONET))
+		goto drop;
+
+	req = usb_alloc_urb(0, GFP_ATOMIC);
+	if (!req)
+		goto drop;
+	usb_fill_bulk_urb(req, pnd->usb, pnd->tx_pipe, skb->data, skb->len,
+				tx_complete, skb);
+	req->transfer_flags = URB_ZERO_PACKET;
+	err = usb_submit_urb(req, GFP_ATOMIC);
+	if (err) {
+		usb_free_urb(req);
+		goto drop;
+	}
+
+	netif_stop_queue(dev);
+	if (atomic_inc_return(&pnd->tx_queue) < dev->tx_queue_len)
+		netif_wake_queue(dev);
+	return 0;
+
+drop:
+	dev_kfree_skb(skb);
+	dev->stats.tx_dropped++;
+	return 0;
+}
+
+static void tx_complete(struct urb *req)
+{
+	struct sk_buff *skb = req->context;
+	struct net_device *dev = skb->dev;
+	struct usbpn_dev *pnd = netdev_priv(dev);
+
+	switch (req->status) {
+	case 0:
+		dev->stats.tx_bytes += skb->len;
+		break;
+
+	case -ENOENT:
+	case -ECONNRESET:
+	case -ESHUTDOWN:
+		dev->stats.tx_aborted_errors++;
+	default:
+		dev->stats.tx_errors++;
+		dev_dbg(&dev->dev, "TX error (%d)\n", req->status);
+	}
+	dev->stats.tx_packets++;
+
+	atomic_dec(&pnd->tx_queue);
+	netif_wake_queue(dev);
+
+	dev_kfree_skb_any(skb);
+	usb_free_urb(req);
+}
+
+static int rx_submit(struct usbpn_dev *pnd, struct urb *req, gfp_t gfp_flags)
+{
+	struct net_device *dev = pnd->dev;
+	struct page *page;
+	int err;
+
+	page = __netdev_alloc_page(dev, gfp_flags);
+	if (!page)
+		return -ENOMEM;
+
+	usb_fill_bulk_urb(req, pnd->usb, pnd->rx_pipe, page_address(page),
+				PAGE_SIZE, rx_complete, dev);
+	req->transfer_flags = 0;
+	err = usb_submit_urb(req, gfp_flags);
+	if (unlikely(err)) {
+		dev_dbg(&dev->dev, "RX submit error (%d)\n", err);
+		netdev_free_page(dev, page);
+	}
+	return err;
+}
+
+static void rx_complete(struct urb *req)
+{
+	struct net_device *dev = req->context;
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	struct page *page = virt_to_page(req->transfer_buffer);
+	struct sk_buff *skb;
+	unsigned long flags;
+
+	switch (req->status) {
+	case 0:
+		spin_lock_irqsave(&pnd->rx_lock, flags);
+		skb = pnd->rx_skb;
+		if (!skb) {
+			skb = pnd->rx_skb = netdev_alloc_skb(dev, 12);
+			if (likely(skb)) {
+				/* Can't use pskb_pull() on page in IRQ */
+				memcpy(skb_put(skb, 1), page_address(page), 1);
+				skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
+						page, 1, req->actual_length);
+				page = NULL;
+			}
+		} else {
+			skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
+					page, 0, req->actual_length);
+			page = NULL;
+		}
+		if (req->actual_length < PAGE_SIZE)
+			pnd->rx_skb = NULL; /* Last fragment */
+		else
+			skb = NULL;
+		spin_unlock_irqrestore(&pnd->rx_lock, flags);
+		if (skb) {
+			skb->protocol = htons(ETH_P_PHONET);
+			skb_reset_mac_header(skb);
+			__skb_pull(skb, 1);
+			skb->dev = dev;
+			dev->stats.rx_packets++;
+			dev->stats.rx_bytes += skb->len;
+
+			netif_rx(skb);
+		}
+		goto resubmit;
+
+	case -ENOENT:
+	case -ECONNRESET:
+	case -ESHUTDOWN:
+		req = NULL;
+		break;
+
+	case -EOVERFLOW:
+		dev->stats.rx_over_errors++;
+		dev_dbg(&dev->dev, "RX overflow\n");
+		break;
+
+	case -EILSEQ:
+		dev->stats.rx_crc_errors++;
+		break;
+	}
+
+	dev->stats.rx_errors++;
+resubmit:
+	if (page)
+		netdev_free_page(dev, page);
+	if (req)
+		rx_submit(pnd, req, GFP_ATOMIC);
+}
+
+static int usbpn_close(struct net_device *dev);
+
+static int usbpn_open(struct net_device *dev)
+{
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	int err;
+	unsigned i;
+	unsigned num = pnd->data_intf->cur_altsetting->desc.bInterfaceNumber;
+
+	err = usb_set_interface(pnd->usb, num, pnd->active_setting);
+	if (err)
+		return err;
+
+	for (i = 0; i < rxq_size; i++) {
+		struct urb *req = usb_alloc_urb(0, GFP_KERNEL);
+
+		if (!req || rx_submit(pnd, req, GFP_KERNEL)) {
+			usbpn_close(dev);
+			return -ENOMEM;
+		}
+		pnd->urbs[i] = req;
+	}
+
+	netif_wake_queue(dev);
+	return 0;
+}
+
+static int usbpn_close(struct net_device *dev)
+{
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	unsigned i;
+	unsigned num = pnd->data_intf->cur_altsetting->desc.bInterfaceNumber;
+
+	netif_stop_queue(dev);
+
+	for (i = 0; i < rxq_size; i++) {
+		struct urb *req = pnd->urbs[i];
+
+		if (!req)
+			continue;
+		usb_kill_urb(req);
+		usb_free_urb(req);
+		pnd->urbs[i] = NULL;
+	}
+
+	return usb_set_interface(pnd->usb, num, !pnd->active_setting);
+}
+
+static int usbpn_set_mtu(struct net_device *dev, int new_mtu)
+{
+	if ((new_mtu < PHONET_MIN_MTU) || (new_mtu > PHONET_MAX_MTU))
+		return -EINVAL;
+
+	dev->mtu = new_mtu;
+	return 0;
+}
+
+static const struct net_device_ops usbpn_ops = {
+	.ndo_open	= usbpn_open,
+	.ndo_stop	= usbpn_close,
+	.ndo_start_xmit = usbpn_xmit,
+	.ndo_change_mtu = usbpn_set_mtu,
+};
+
+static void usbpn_setup(struct net_device *dev)
+{
+	dev->features		= 0;
+	dev->netdev_ops		= &usbpn_ops,
+	dev->header_ops		= &phonet_header_ops;
+	dev->type		= ARPHRD_PHONET;
+	dev->flags		= IFF_POINTOPOINT | IFF_NOARP;
+	dev->mtu		= PHONET_MAX_MTU;
+	dev->hard_header_len	= 1;
+	dev->dev_addr[0]	= PN_MEDIA_USB;
+	dev->addr_len		= 1;
+	dev->tx_queue_len	= 3;
+
+	dev->destructor		= free_netdev;
+}
+
+/*
+ * USB driver callbacks
+ */
+static struct usb_device_id usbpn_ids[] = {
+	{
+		.match_flags = USB_DEVICE_ID_MATCH_VENDOR
+			| USB_DEVICE_ID_MATCH_INT_CLASS
+			| USB_DEVICE_ID_MATCH_INT_SUBCLASS,
+		.idVendor = 0x0421, /* Nokia */
+		.bInterfaceClass = USB_CLASS_COMM,
+		.bInterfaceSubClass = 0xFE,
+	},
+	{ },
+};
+
+MODULE_DEVICE_TABLE(usb, usbpn_ids);
+
+static struct usb_driver usbpn_driver;
+
+int usbpn_probe(struct usb_interface *intf, const struct usb_device_id *id)
+{
+	static const char ifname[] = "usbpn%d";
+	const struct usb_cdc_union_desc *union_header = NULL;
+	const struct usb_cdc_header_desc *phonet_header = NULL;
+	const struct usb_host_interface *data_desc;
+	struct usb_interface *data_intf;
+	struct usb_device *usbdev = interface_to_usbdev(intf);
+	struct net_device *dev;
+	struct usbpn_dev *pnd;
+	u8 *data;
+	int len, err;
+
+	data = intf->altsetting->extra;
+	len = intf->altsetting->extralen;
+	while (len >= 3) {
+		u8 dlen = data[0];
+		if (dlen < 3)
+			return -EINVAL;
+
+		/* bDescriptorType */
+		if (data[1] == USB_DT_CS_INTERFACE) {
+			/* bDescriptorSubType */
+			switch (data[2]) {
+			case USB_CDC_UNION_TYPE:
+				if (union_header || dlen < 5)
+					break;
+				union_header =
+					(struct usb_cdc_union_desc *)data;
+				break;
+			case 0xAB:
+				if (phonet_header || dlen < 5)
+					break;
+				phonet_header =
+					(struct usb_cdc_header_desc *)data;
+				break;
+			}
+		}
+		data += dlen;
+		len -= dlen;
+	}
+
+	if (!union_header || !phonet_header)
+		return -EINVAL;
+
+	data_intf = usb_ifnum_to_if(usbdev, union_header->bSlaveInterface0);
+	if (data_intf == NULL)
+		return -ENODEV;
+	/* Data interface has one inactive and one active setting */
+	if (data_intf->num_altsetting != 2)
+		return -EINVAL;
+	if (data_intf->altsetting[0].desc.bNumEndpoints == 0
+	 && data_intf->altsetting[1].desc.bNumEndpoints == 2)
+		data_desc = data_intf->altsetting + 1;
+	else
+	if (data_intf->altsetting[0].desc.bNumEndpoints == 2
+	 && data_intf->altsetting[1].desc.bNumEndpoints == 0)
+		data_desc = data_intf->altsetting;
+	else
+		return -EINVAL;
+
+	dev = alloc_netdev(sizeof(*pnd) + sizeof(pnd->urbs[0]) * rxq_size,
+				ifname, usbpn_setup);
+	if (!dev)
+		return -ENOMEM;
+
+	pnd = netdev_priv(dev);
+	SET_NETDEV_DEV(dev, &intf->dev);
+	netif_stop_queue(dev);
+
+	pnd->dev = dev;
+	atomic_set(&pnd->tx_queue, 0);
+	pnd->usb = usb_get_dev(usbdev);
+	pnd->intf = intf;
+	pnd->data_intf = data_intf;
+	spin_lock_init(&pnd->rx_lock);
+	/* Endpoints */
+	if (usb_pipein(data_desc->endpoint[0].desc.bEndpointAddress)) {
+		pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
+			data_desc->endpoint[0].desc.bEndpointAddress);
+		pnd->tx_pipe = usb_sndbulkpipe(usbdev,
+			data_desc->endpoint[1].desc.bEndpointAddress);
+	} else {
+		pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
+			data_desc->endpoint[1].desc.bEndpointAddress);
+		pnd->tx_pipe = usb_sndbulkpipe(usbdev,
+			data_desc->endpoint[0].desc.bEndpointAddress);
+	}
+	pnd->active_setting = data_desc - data_intf->altsetting;
+
+	err = usb_driver_claim_interface(&usbpn_driver, data_intf, pnd);
+	if (err)
+		goto out;
+
+	/* Force inactive mode until the network device is brought UP */
+	usb_set_interface(usbdev, union_header->bSlaveInterface0,
+				!pnd->active_setting);
+	usb_set_intfdata(intf, pnd);
+
+	err = register_netdev(dev);
+	if (err) {
+		usb_driver_release_interface(&usbpn_driver, data_intf);
+		goto out;
+	}
+
+	dev_dbg(&dev->dev, "USB CDC Phonet device found\n");
+	return 0;
+
+out:
+	usb_set_intfdata(intf, NULL);
+	free_netdev(dev);
+	return err;
+}
+
+static void usbpn_disconnect(struct usb_interface *intf)
+{
+	struct usbpn_dev *pnd = usb_get_intfdata(intf);
+	struct usb_device *usb = pnd->usb;
+
+	if (pnd->disconnected)
+		return;
+
+	pnd->disconnected = 1;
+	usb_driver_release_interface(&usbpn_driver,
+			(pnd->intf == intf) ? pnd->data_intf : pnd->intf);
+	unregister_netdev(pnd->dev);
+	usb_put_dev(usb);
+}
+
+static struct usb_driver usbpn_driver = {
+	.name =		"cdc_phonet",
+	.probe =	usbpn_probe,
+	.disconnect =	usbpn_disconnect,
+	.id_table =	usbpn_ids,
+};
+
+static int __init usbpn_init(void)
+{
+	return usb_register(&usbpn_driver);
+}
+
+static void __exit usbpn_exit(void)
+{
+	usb_deregister(&usbpn_driver);
+}
+
+module_init(usbpn_init);
+module_exit(usbpn_exit);
+
+MODULE_AUTHOR("Remi Denis-Courmont");
+MODULE_DESCRIPTION("USB CDC Phonet host interface");
+MODULE_LICENSE("GPL");
-- 
1.6.0.4

Re: [PATCH] USB host CDC Phonet network interface driver

[Oliver Neukum]

Am Freitag, 17. Juli 2009 11:56:06 schrieb Rémi Denis-Courmont:
> +       atomic_dec(&pnd->tx_queue);
> +       netif_wake_queue(dev);

Now that I think about it this seems to be a race condition.
What makes sure that your are still below the limit when you
wake the queue?

	Regards
		Oliver

Re: [PATCH] USB host CDC Phonet network interface driver

[Rémi Denis-Courmont]

On Friday 17 July 2009 16:47:38 ext Oliver Neukum wrote:
> Am Freitag, 17. Juli 2009 11:56:06 schrieb Rémi Denis-Courmont:
> > +       atomic_dec(&pnd->tx_queue);
> > +       netif_wake_queue(dev);
>
> Now that I think about it this seems to be a race condition.
> What makes sure that your are still below the limit when you
> wake the queue?

The netif TX queue serializes usbpn_xmit() against itself. Then, at all times: 
tx_queue + !netif_queue_stopped() <= tx_queue_len (in other words either 
tx_queue = tx_queue_len and queue is stopped, or tx_queue < tx_queue_len).

Initially:
0 + 1 <= tx_queue_len, the assertion is initially true.

Recurrently:
If usbpn_xmit() is called, then the queue was not stopped, so tx_queue < 
tx_queue_len. Then the queue is stopped. In the race free cases, either 
tx_queue is incremented up to the limit, and the queue remains stopped; the 
assertion is still valid.
Alternatively tx_queue remains below limit and the queue gets woken again; the 
assertion is still valid.

In the racy case, tx_complete() fires, incrementation in usbpn_xmit() and 
decrementation in tx_complete() will cancel each other. So, regardless of 
their respective order, tx_queue will be unchanged, and the assertion remains 
valid. A fortiori, it works fine if usbpn_xmit() races with more than one call 
of tx_complete().

The cases that tx_complete() runs while the queue is not transmitting is 
evident.


-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki

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

Re: [PATCH] USB host CDC Phonet network interface driver

[Oliver Neukum]

Am Montag, 20. Juli 2009 08:35:18 schrieb Rémi Denis-Courmont:
> Recurrently:
> If usbpn_xmit() is called, then the queue was not stopped, so tx_queue <
> tx_queue_len. Then the queue is stopped. In the race free cases, either
> tx_queue is incremented up to the limit, and the queue remains stopped; the
> assertion is still valid.
> Alternatively tx_queue remains below limit and the queue gets woken again;
> the assertion is still valid.

Yes, you are correct. I find no flaws with the driver.

	Regards
		Oliver

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

Re: [PATCH] USB host CDC Phonet network interface driver

[David Miller]

From: "Rémi Denis-Courmont" <remi.denis-courmont@nokia.com>
Date: Mon, 20 Jul 2009 09:35:18 +0300

> In the racy case, tx_complete() fires, incrementation in
> usbpn_xmit() and decrementation in tx_complete() will cancel each
> other. So, regardless of their respective order, tx_queue will be
> unchanged, and the assertion remains valid. A fortiori, it works
> fine if usbpn_xmit() races with more than one call of tx_complete().

You can only do the wakeup race free without deadlocking the TX queue
state if you grab the TX path lock, as TG3 does:

	/* Need to make the tx_cons update visible to tg3_start_xmit()
	 * before checking for netif_queue_stopped().  Without the
	 * memory barrier, there is a small possibility that tg3_start_xmit()
	 * will miss it and cause the queue to be stopped forever.
	 */
	smp_mb();

	if (unlikely(netif_queue_stopped(tp->dev) &&
		     (tg3_tx_avail(tp) > TG3_TX_WAKEUP_THRESH(tp)))) {
		netif_tx_lock(tp->dev);
		if (netif_queue_stopped(tp->dev) &&
		    (tg3_tx_avail(tp) > TG3_TX_WAKEUP_THRESH(tp)))
			netif_wake_queue(tp->dev);
		netif_tx_unlock(tp->dev);
	}

Re: [PATCH] USB host CDC Phonet network interface driver

[David Miller]

From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Date: Fri, 17 Jul 2009 12:56:06 +0300

> From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> 
> Many Nokia handsets support a Phonet interface to the cellular modem
> via a vendor-specific USB interface. CDC Phonet follows the
> Communications Device Class model, with one control interface, and
> and a pair of inactive and active data alternative interface. The later
> has two bulk endpoint, one per direction.
> 
> This was tested against Nokia E61, Nokia N95, and the existing Phonet
> gadget function for the Linux composite USB gadget framework.
> 
> Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

Applied, thanks.

Re: [PATCH] USB host CDC Phonet network interface driver

[Marcel Holtmann]

Hi Dave,

> > Many Nokia handsets support a Phonet interface to the cellular modem
> > via a vendor-specific USB interface. CDC Phonet follows the
> > Communications Device Class model, with one control interface, and
> > and a pair of inactive and active data alternative interface. The later
> > has two bulk endpoint, one per direction.
> > 
> > This was tested against Nokia E61, Nokia N95, and the existing Phonet
> > gadget function for the Linux composite USB gadget framework.
> > 
> > Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> 
> Applied, thanks.

I think that Remi sent an updated version today.

Regards

Marcel


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

Re: [PATCH] USB host CDC Phonet network interface driver

[David Miller]

From: Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
Date: Tue, 21 Jul 2009 22:14:00 +0200

> Hi Dave,
> 
>> > Many Nokia handsets support a Phonet interface to the cellular modem
>> > via a vendor-specific USB interface. CDC Phonet follows the
>> > Communications Device Class model, with one control interface, and
>> > and a pair of inactive and active data alternative interface. The later
>> > has two bulk endpoint, one per direction.
>> > 
>> > This was tested against Nokia E61, Nokia N95, and the existing Phonet
>> > gadget function for the Linux composite USB gadget framework.
>> > 
>> > Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont-xNZwKgViW5g@public.gmane.orgm>
>> 
>> Applied, thanks.
> 
> I think that Remi sent an updated version today.

I'm pretty sure I applied the updated patch.

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

Re: [PATCH] USB host CDC Phonet network interface driver

[Marcel Holtmann]

Hi Dave,

> >> > Many Nokia handsets support a Phonet interface to the cellular modem
> >> > via a vendor-specific USB interface. CDC Phonet follows the
> >> > Communications Device Class model, with one control interface, and
> >> > and a pair of inactive and active data alternative interface. The later
> >> > has two bulk endpoint, one per direction.
> >> > 
> >> > This was tested against Nokia E61, Nokia N95, and the existing Phonet
> >> > gadget function for the Linux composite USB gadget framework.
> >> > 
> >> > Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> >> 
> >> Applied, thanks.
> > 
> > I think that Remi sent an updated version today.
> 
> I'm pretty sure I applied the updated patch.
> 
> I'll double check to make sure.

I only mentioned it because your reply is to this patch and not the
other one from today. He sent 3 versions of it if I am not mis-counting.

Regards

Marcel


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

Re: [PATCH] USB host CDC Phonet network interface driver

[David Miller]

From: David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
Date: Tue, 21 Jul 2009 13:18:16 -0700 (PDT)

> From: Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
> Date: Tue, 21 Jul 2009 22:14:00 +0200
> 
>> Hi Dave,
>> 
>>> > Many Nokia handsets support a Phonet interface to the cellular modem
>>> > via a vendor-specific USB interface. CDC Phonet follows the
>>> > Communications Device Class model, with one control interface, and
>>> > and a pair of inactive and active data alternative interface. The later
>>> > has two bulk endpoint, one per direction.
>>> > 
>>> > This was tested against Nokia E61, Nokia N95, and the existing Phonet
>>> > gadget function for the Linux composite USB gadget framework.
>>> > 
>>> > Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
>>> 
>>> Applied, thanks.
>> 
>> I think that Remi sent an updated version today.
> 
> I'm pretty sure I applied the updated patch.
> 
> I'll double check to make sure.

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

Re: [PATCH] USB host CDC Phonet network interface driver

[Rémi Denis-Courmont]

On Tuesday 21 July 2009 23:18:16 ext David Miller wrote:
> >> Applied, thanks.
> >
> > I think that Remi sent an updated version today.
>
> I'm pretty sure I applied the updated patch.
>
> I'll double check to make sure.

I guess I should put updated version as answers to older ones then?

Thanks every one for the reviews anyway!

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki

[PATCH] USB host CDC Phonet network interface driver

[Rémi Denis-Courmont]

From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

Many Nokia handsets support a Phonet interface to the cellular modem
via a vendor-specific USB interface. CDC Phonet follows the
Communications Device Class model, with one control interface, and
and a pair of inactive and active data alternative interface. The later
has two bulk endpoint, one per direction.

This was tested against Nokia E61, Nokia N95, and the existing Phonet
gadget function for the Linux composite USB gadget framework.

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 drivers/net/usb/Kconfig      |    8 +
 drivers/net/usb/Makefile     |    1 +
 drivers/net/usb/cdc-phonet.c |  451 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 460 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/usb/cdc-phonet.c

diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index a906d39..c47237c 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -369,4 +369,12 @@ config USB_NET_INT51X1
 	  (Powerline Communications) solution with an Intellon
 	  INT51x1/INT5200 chip, like the "devolo dLan duo".
 
+config USB_CDC_PHONET
+	tristate "CDC Phonet support"
+	depends on PHONET
+	help
+	  Choose this option to support the Phonet interface to a Nokia
+	  cellular modem, as found on most Nokia handsets with the
+	  "PC suite" USB profile.
+
 endmenu
diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index b870b0b..e17afb7 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -21,4 +21,5 @@ obj-$(CONFIG_USB_NET_ZAURUS)	+= zaurus.o
 obj-$(CONFIG_USB_NET_MCS7830)	+= mcs7830.o
 obj-$(CONFIG_USB_USBNET)	+= usbnet.o
 obj-$(CONFIG_USB_NET_INT51X1)	+= int51x1.o
+obj-$(CONFIG_USB_CDC_PHONET)	+= cdc-phonet.o
 
diff --git a/drivers/net/usb/cdc-phonet.c b/drivers/net/usb/cdc-phonet.c
new file mode 100644
index 0000000..d335b72
--- /dev/null
+++ b/drivers/net/usb/cdc-phonet.c
@@ -0,0 +1,451 @@
+/*
+ * phonet.c -- USB CDC Phonet host driver
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation. All rights reserved.
+ *
+ * Author: Rémi Denis-Courmont
+ *
+ * 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.
+ *
+ * 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., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/usb.h>
+#include <linux/usb/cdc.h>
+#include <linux/netdevice.h>
+#include <linux/if_arp.h>
+#include <linux/if_phonet.h>
+
+#define PN_MEDIA_USB	0x1B
+
+static const unsigned rxq_size = 17;
+
+struct usbpn_dev {
+	struct net_device	*dev;
+	atomic_t		tx_queue;
+
+	struct usb_interface	*intf, *data_intf;
+	struct usb_device	*usb;
+	unsigned int		tx_pipe, rx_pipe;
+	u8 active_setting;
+
+	spinlock_t		rx_lock;
+	struct sk_buff		*rx_skb;
+	struct urb		*urbs[0];
+};
+
+static void tx_complete(struct urb *req);
+static void rx_complete(struct urb *req);
+
+/*
+ * Network device callbacks
+ */
+static int usbpn_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	struct urb *req = NULL;
+	int err;
+
+	if (skb->protocol != htons(ETH_P_PHONET))
+		goto drop;
+
+	req = usb_alloc_urb(0, GFP_ATOMIC);
+	if (!req)
+		goto drop;
+	usb_fill_bulk_urb(req, pnd->usb, pnd->tx_pipe, skb->data, skb->len,
+				tx_complete, skb);
+	req->transfer_flags = URB_ZERO_PACKET;
+	err = usb_submit_urb(req, GFP_ATOMIC);
+	if (err) {
+		usb_free_urb(req);
+		goto drop;
+	}
+
+	netif_stop_queue(dev);
+	if (atomic_inc_return(&pnd->tx_queue) < dev->tx_queue_len)
+		netif_wake_queue(dev);
+	return 0;
+
+drop:
+	dev_kfree_skb(skb);
+	dev->stats.tx_dropped++;
+	return 0;
+}
+
+static void tx_complete(struct urb *req)
+{
+	struct sk_buff *skb = req->context;
+	struct net_device *dev = skb->dev;
+	struct usbpn_dev *pnd = netdev_priv(dev);
+
+	switch (req->status) {
+	case 0:
+		dev->stats.tx_bytes += skb->len;
+		break;
+
+	case -ENOENT:
+	case -ECONNRESET:
+	case -ESHUTDOWN:
+		dev->stats.tx_aborted_errors++;
+	default:
+		dev->stats.tx_errors++;
+		dev_dbg(&dev->dev, "TX error (%d)\n", req->status);
+	}
+	dev->stats.tx_packets++;
+
+	atomic_dec(&pnd->tx_queue);
+	netif_wake_queue(dev);
+
+	dev_kfree_skb_any(skb);
+	usb_free_urb(req);
+}
+
+static int rx_submit(struct usbpn_dev *pnd, struct urb *req, gfp_t gfp_flags)
+{
+	struct net_device *dev = pnd->dev;
+	struct page *page;
+	int err;
+
+	page = __netdev_alloc_page(dev, gfp_flags);
+	if (!page)
+		return -ENOMEM;
+
+	usb_fill_bulk_urb(req, pnd->usb, pnd->rx_pipe, page_address(page),
+				PAGE_SIZE, rx_complete, dev);
+	req->transfer_flags = 0;
+	err = usb_submit_urb(req, gfp_flags);
+	if (unlikely(err)) {
+		dev_dbg(&dev->dev, "RX submit error (%d)\n", err);
+		netdev_free_page(dev, page);
+	}
+	return err;
+}
+
+static void rx_complete(struct urb *req)
+{
+	struct net_device *dev = req->context;
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	struct page *page = virt_to_page(req->transfer_buffer);
+	struct sk_buff *skb;
+	unsigned long flags;
+
+	switch (req->status) {
+	case 0:
+		spin_lock_irqsave(&pnd->rx_lock, flags);
+		skb = pnd->rx_skb;
+		if (!skb) {
+			skb = pnd->rx_skb = netdev_alloc_skb(dev, 12);
+			if (likely(skb)) {
+				/* Can't use pskb_pull() on page in IRQ */
+				memcpy(skb_put(skb, 1), page_address(page), 1);
+				skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
+						page, 1, req->actual_length);
+				page = NULL;
+			}
+		} else {
+			skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
+					page, 0, req->actual_length);
+			page = NULL;
+		}
+		if (req->actual_length < PAGE_SIZE)
+			pnd->rx_skb = NULL; /* Last fragment */
+		else
+			skb = NULL;
+		spin_unlock_irqrestore(&pnd->rx_lock, flags);
+		if (skb) {
+			skb->protocol = htons(ETH_P_PHONET);
+			skb_reset_mac_header(skb);
+			__skb_pull(skb, 1);
+			skb->dev = dev;
+			dev->stats.rx_packets++;
+			dev->stats.rx_bytes += skb->len;
+
+			netif_rx(skb);
+		}
+		goto resubmit;
+
+	case -ENOENT:
+	case -ECONNRESET:
+	case -ESHUTDOWN:
+		req = NULL;
+		break;
+
+	case -EOVERFLOW:
+		dev->stats.rx_over_errors++;
+		dev_dbg(&dev->dev, "RX overflow\n");
+		break;
+
+	case -EILSEQ:
+		dev->stats.rx_crc_errors++;
+		break;
+	}
+
+	dev->stats.rx_errors++;
+resubmit:
+	if (page)
+		netdev_free_page(dev, page);
+	if (req)
+		rx_submit(pnd, req, GFP_ATOMIC);
+}
+
+static int usbpn_open(struct net_device *dev)
+{
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	int err;
+	unsigned i;
+	unsigned num = pnd->data_intf->cur_altsetting->desc.bInterfaceNumber;
+
+	err = usb_set_interface(pnd->usb, num, pnd->active_setting);
+	if (err)
+		return err;
+
+	for (i = 0; i < rxq_size; i++) {
+		struct urb *req = usb_alloc_urb(0, GFP_KERNEL);
+
+		if (req && rx_submit(pnd, req, GFP_KERNEL)) {
+			usb_free_urb(req);
+			req = NULL;
+		}
+		pnd->urbs[i] = req;
+	}
+
+	netif_wake_queue(dev);
+	return 0;
+}
+
+static int usbpn_close(struct net_device *dev)
+{
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	unsigned i;
+	unsigned num = pnd->data_intf->cur_altsetting->desc.bInterfaceNumber;
+
+	netif_stop_queue(dev);
+
+	for (i = 0; i < rxq_size; i++) {
+		struct urb *req = pnd->urbs[i];
+
+		if (!req)
+			continue;
+		usb_kill_urb(req);
+		usb_free_urb(req);
+		pnd->urbs[i] = NULL;
+	}
+
+	return usb_set_interface(pnd->usb, num, !pnd->active_setting);
+}
+
+static int usbpn_set_mtu(struct net_device *dev, int new_mtu)
+{
+	if ((new_mtu < PHONET_MIN_MTU) || (new_mtu > PHONET_MAX_MTU))
+		return -EINVAL;
+
+	dev->mtu = new_mtu;
+	return 0;
+}
+
+static const struct net_device_ops usbpn_ops = {
+	.ndo_open	= usbpn_open,
+	.ndo_stop	= usbpn_close,
+	.ndo_start_xmit = usbpn_xmit,
+	.ndo_change_mtu = usbpn_set_mtu,
+};
+
+static void usbpn_setup(struct net_device *dev)
+{
+	dev->features		= 0;
+	dev->netdev_ops		= &usbpn_ops,
+	dev->header_ops		= &phonet_header_ops;
+	dev->type		= ARPHRD_PHONET;
+	dev->flags		= IFF_POINTOPOINT | IFF_NOARP;
+	dev->mtu		= PHONET_MAX_MTU;
+	dev->hard_header_len	= 1;
+	dev->dev_addr[0]	= PN_MEDIA_USB;
+	dev->addr_len		= 1;
+	dev->tx_queue_len	= 3;
+
+	dev->destructor		= free_netdev;
+}
+
+/*
+ * USB driver callbacks
+ */
+static struct usb_device_id usbpn_ids[] = {
+	{
+		.match_flags = USB_DEVICE_ID_MATCH_VENDOR
+			| USB_DEVICE_ID_MATCH_INT_CLASS
+			| USB_DEVICE_ID_MATCH_INT_SUBCLASS,
+		.idVendor = 0x0421, /* Nokia */
+		.bInterfaceClass = USB_CLASS_COMM,
+		.bInterfaceSubClass = 0xFE,
+	},
+	{ },
+};
+
+MODULE_DEVICE_TABLE(usb, usbpn_ids);
+
+static struct usb_driver usbpn_driver;
+
+int usbpn_probe(struct usb_interface *intf, const struct usb_device_id *id)
+{
+	static const char ifname[] = "usbpn%d";
+	const struct usb_cdc_union_desc *union_header = NULL;
+	const struct usb_cdc_header_desc *phonet_header = NULL;
+	const struct usb_host_interface *data_desc;
+	struct usb_interface *data_intf;
+	struct usb_device *usbdev = interface_to_usbdev(intf);
+	struct net_device *dev;
+	struct usbpn_dev *pnd;
+	u8 *data;
+	int len, err;
+
+	data = intf->altsetting->extra;
+	len = intf->altsetting->extralen;
+	while (len >= 3) {
+		u8 dlen = data[0];
+		if (dlen < 3)
+			return -EINVAL;
+
+		/* bDescriptorType */
+		if (data[1] == USB_DT_CS_INTERFACE) {
+			/* bDescriptorSubType */
+			switch (data[2]) {
+			case USB_CDC_UNION_TYPE:
+				if (union_header || dlen < 5)
+					break;
+				union_header =
+					(struct usb_cdc_union_desc *)data;
+				break;
+			case 0xAB:
+				if (phonet_header || dlen < 5)
+					break;
+				phonet_header =
+					(struct usb_cdc_header_desc *)data;
+				break;
+			}
+		}
+		data += dlen;
+		len -= dlen;
+	}
+
+	if (!union_header || !phonet_header)
+		return -EINVAL;
+
+	data_intf = usb_ifnum_to_if(usbdev, union_header->bSlaveInterface0);
+	if (data_intf == NULL)
+		return -ENODEV;
+	/* Data interface has one inactive and one active setting */
+	if (data_intf->num_altsetting != 2)
+		return -EINVAL;
+	if (data_intf->altsetting[0].desc.bNumEndpoints == 0
+	 && data_intf->altsetting[1].desc.bNumEndpoints == 2)
+		data_desc = data_intf->altsetting + 1;
+	else
+	if (data_intf->altsetting[0].desc.bNumEndpoints == 2
+	 && data_intf->altsetting[1].desc.bNumEndpoints == 0)
+		data_desc = data_intf->altsetting;
+	else
+		return -EINVAL;
+
+	dev = alloc_netdev(sizeof(*pnd) + sizeof(pnd->urbs[0]) * rxq_size,
+				ifname, usbpn_setup);
+	if (!dev)
+		return -ENOMEM;
+
+	pnd = netdev_priv(dev);
+	SET_NETDEV_DEV(dev, &intf->dev);
+	netif_stop_queue(dev);
+
+	pnd->dev = dev;
+	atomic_set(&pnd->tx_queue, 0);
+	pnd->usb = usb_get_dev(usbdev);
+	pnd->intf = intf;
+	pnd->data_intf = data_intf;
+	spin_lock_init(&pnd->rx_lock);
+	/* Endpoints */
+	if ((data_desc->endpoint[0].desc.bEndpointAddress & USB_DIR_IN) ==
+			USB_DIR_IN) {
+		pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
+			data_desc->endpoint[0].desc.bEndpointAddress);
+		pnd->tx_pipe = usb_sndbulkpipe(usbdev,
+			data_desc->endpoint[1].desc.bEndpointAddress);
+	} else {
+		pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
+			data_desc->endpoint[1].desc.bEndpointAddress);
+		pnd->tx_pipe = usb_sndbulkpipe(usbdev,
+			data_desc->endpoint[0].desc.bEndpointAddress);
+	}
+	pnd->active_setting = data_desc - data_intf->altsetting;
+
+	err = usb_driver_claim_interface(&usbpn_driver, data_intf, pnd);
+	if (err)
+		goto out;
+
+	/* Force inactive mode until the network device is brought UP */
+	usb_set_interface(usbdev, union_header->bSlaveInterface0,
+				!pnd->active_setting);
+	usb_set_intfdata(intf, pnd);
+	BUG_ON(!usb_get_intfdata(intf));
+
+	err = register_netdev(dev);
+	if (err) {
+		usb_driver_release_interface(&usbpn_driver, data_intf);
+		goto out;
+	}
+
+	dev_dbg(&dev->dev, "USB CDC Phonet device found\n");
+	return 0;
+
+out:
+	usb_set_intfdata(intf, NULL);
+	free_netdev(dev);
+	return err;
+}
+
+static void usbpn_disconnect(struct usb_interface *intf)
+{
+	struct usbpn_dev *pnd = usb_get_intfdata(intf);
+
+	if (intf != pnd->data_intf) {
+		usb_driver_release_interface(&usbpn_driver, pnd->data_intf);
+		return;
+	}
+
+	unregister_netdev(pnd->dev);
+	usb_put_dev(pnd->usb);
+}
+
+static struct usb_driver usbpn_driver = {
+	.name =		"cdc_phonet",
+	.probe =	usbpn_probe,
+	.disconnect =	usbpn_disconnect,
+	.id_table =	usbpn_ids,
+};
+
+static int __init usbpn_init(void)
+{
+	return usb_register(&usbpn_driver);
+}
+
+static void __exit usbpn_exit(void)
+{
+	usb_deregister(&usbpn_driver);
+}
+
+module_init(usbpn_init);
+module_exit(usbpn_exit);
+
+MODULE_AUTHOR("Remi Denis-Courmont");
+MODULE_DESCRIPTION("USB CDC Phonet host interface");
+MODULE_LICENSE("GPL");
-- 
1.6.0.4

Re: [PATCH] USB host CDC Phonet network interface driver

[Oliver Neukum]

Am Mittwoch, 15. Juli 2009 15:59:01 schrieb Rémi Denis-Courmont:

Hello,

> +	netif_stop_queue(dev);
> +	if (atomic_inc_return(&pnd->tx_queue) < dev->tx_queue_len)
> +		netif_wake_queue(dev);

This is unelegant.


> +	err = usb_set_interface(pnd->usb, num, pnd->active_setting);
> +	if (err)
> +		return err;
> +
> +	for (i = 0; i < rxq_size; i++) {
> +		struct urb *req = usb_alloc_urb(0, GFP_KERNEL);
> +
> +		if (req && rx_submit(pnd, req, GFP_KERNEL)) {
> +			usb_free_urb(req);
> +			req = NULL;
> +		}
> +		pnd->urbs[i] = req;
> +	}

You should fail if you cannot get a minimum number of URBs running.
> +
> +	netif_wake_queue(dev);
> +	return 0;
> +}
[..]
> +	/* Endpoints */
> +	if ((data_desc->endpoint[0].desc.bEndpointAddress & USB_DIR_IN) ==
> +			USB_DIR_IN) {

Please use the macro.

> +		pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
> +			data_desc->endpoint[0].desc.bEndpointAddress);
> +		pnd->tx_pipe = usb_sndbulkpipe(usbdev,
> +			data_desc->endpoint[1].desc.bEndpointAddress);
> +	} else {
> +		pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
> +			data_desc->endpoint[1].desc.bEndpointAddress);
> +		pnd->tx_pipe = usb_sndbulkpipe(usbdev,
> +			data_desc->endpoint[0].desc.bEndpointAddress);
> +	}
> +	pnd->active_setting = data_desc - data_intf->altsetting;
> +
> +	err = usb_driver_claim_interface(&usbpn_driver, data_intf, pnd);
> +	if (err)
> +		goto out;
> +
> +	/* Force inactive mode until the network device is brought UP */
> +	usb_set_interface(usbdev, union_header->bSlaveInterface0,
> +				!pnd->active_setting);
> +	usb_set_intfdata(intf, pnd);
> +	BUG_ON(!usb_get_intfdata(intf));

What for?

> +
> +	err = register_netdev(dev);
> +	if (err) {
> +		usb_driver_release_interface(&usbpn_driver, data_intf);
> +		goto out;
> +	}
> +
> +	dev_dbg(&dev->dev, "USB CDC Phonet device found\n");
> +	return 0;
> +
> +out:
> +	usb_set_intfdata(intf, NULL);
> +	free_netdev(dev);
> +	return err;
> +}
> +
> +static void usbpn_disconnect(struct usb_interface *intf)
> +{
> +	struct usbpn_dev *pnd = usb_get_intfdata(intf);
> +

What happens if this happens in the unexpected order?

> +	if (intf != pnd->data_intf) {
> +		usb_driver_release_interface(&usbpn_driver, pnd->data_intf);
> +		return;
> +	}
> +
> +	unregister_netdev(pnd->dev);
> +	usb_put_dev(pnd->usb);
> +}

	Regards
		Oliver

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

Re: [PATCH] USB host CDC Phonet network interface driver

[Rémi Denis-Courmont]

On Wednesday 15 July 2009 17:20:31 ext Oliver Neukum wrote:
> Am Mittwoch, 15. Juli 2009 15:59:01 schrieb Rémi Denis-Courmont:
>
> Hello,
>
> > +	netif_stop_queue(dev);
> > +	if (atomic_inc_return(&pnd->tx_queue) < dev->tx_queue_len)
> > +		netif_wake_queue(dev);
>
> This is unelegant.

But I cannot think of any alternative that would not involve a spin lock and 
disable IRQs. There is a race between qdisc and USB TX completion here.

I'm not sure what you're trying to suggest...?

> > +static void usbpn_disconnect(struct usb_interface *intf)
> > +{
> > +	struct usbpn_dev *pnd = usb_get_intfdata(intf);
> > +
>
> What happens if this happens in the unexpected order?

Right, the data interface gets released twice. This "works" since 
usb_driver_release_interface() has an explicit check. I will try to clean it 
up anyway.

> > +	if (intf != pnd->data_intf) {
> > +		usb_driver_release_interface(&usbpn_driver, pnd->data_intf);
> > +		return;
> > +	}
> > +
> > +	unregister_netdev(pnd->dev);
> > +	usb_put_dev(pnd->usb);
> > +}

Thanks for the review.

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki