netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC] cdc_ncm.c: make rx and tx functions exportable
@ 2013-07-02 10:43 Enrico Mioso
  2013-07-02 12:01 ` Bjørn Mork
  0 siblings, 1 reply; 40+ messages in thread
From: Enrico Mioso @ 2013-07-02 10:43 UTC (permalink / raw)
  To: bjorn, netdev

This patch factors out rx and tx fixup functions from cdc_ncm.c driver.
This will be useful once implementing a specific driverto handle huawei 
specific ncm implementation.


diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 43afde8..7fdd7b9 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -885,6 +885,7 @@ error:

  	return NULL;
  }
+EXPORT_SYMBOL_GPL(cdc_ncm_tx_fixup);

  /* verify NTB header and return offset of first NDP, or negative error */
  int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
@@ -1040,6 +1041,7 @@ err_ndp:
  error:
  	return 0;
  }
+EXPORT_SYMBOL_GPL(cdc_ncm_rx_fixup);

  static void
  cdc_ncm_speed_change(struct cdc_ncm_ctx *ctx,

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

* Re: [PATCH RFC] cdc_ncm.c: make rx and tx functions exportable
  2013-07-02 10:43 [PATCH RFC] cdc_ncm.c: make rx and tx functions exportable Enrico Mioso
@ 2013-07-02 12:01 ` Bjørn Mork
  2013-07-02 12:05   ` Enrico Mioso
                     ` (4 more replies)
  0 siblings, 5 replies; 40+ messages in thread
From: Bjørn Mork @ 2013-07-02 12:01 UTC (permalink / raw)
  To: Enrico Mioso; +Cc: netdev

Enrico Mioso <mrkiko.rs@gmail.com> writes:

> This patch factors out rx and tx fixup functions from cdc_ncm.c driver.
> This will be useful once implementing a specific driverto handle
> huawei specific ncm implementation.

It is good to keep this change as a separate patch, but it needs to be
sumbitted as part of a series including the code using it.  Most
reviewers are unable to understand plain text - they can only read code
:)

So it is better to keep this for yourself until you have a working
driver using the exported functions, and then post it all as a patch
series.  That's the best way to show how you intend to use stuff like
this.

And I suggest you CC Alexey Orishko for all changes to the cdc_ncm
driver.  The ST-Ericsson address in the driver is probably bouncing now,
but you can reach him at "Alexey Orishko <alexey.orishko@gmail.com>"



Bjørn

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

* Re: [PATCH RFC] cdc_ncm.c: make rx and tx functions exportable
  2013-07-02 12:01 ` Bjørn Mork
@ 2013-07-02 12:05   ` Enrico Mioso
  2013-07-02 17:04   ` [RFC] huawei_cdc_ncm driver - status Enrico Mioso
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-02 12:05 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: Enrico Mioso, netdev

Thank you very much Bjorn! I will do so!



On Tue, 2 Jul 2013, Bj?rn Mork wrote:

==Date: Tue, 02 Jul 2013 14:01:38 +0200
==From: Bj?rn Mork <bjorn@mork.no>
==To: Enrico Mioso <mrkiko.rs@gmail.com>
==Cc: netdev@vger.kernel.org
==Subject: Re: [PATCH RFC] cdc_ncm.c: make rx and tx functions exportable
==
==Enrico Mioso <mrkiko.rs@gmail.com> writes:
==
==> This patch factors out rx and tx fixup functions from cdc_ncm.c driver.
==> This will be useful once implementing a specific driverto handle
==> huawei specific ncm implementation.
==
==It is good to keep this change as a separate patch, but it needs to be
==sumbitted as part of a series including the code using it.  Most
==reviewers are unable to understand plain text - they can only read code
==:)
==
==So it is better to keep this for yourself until you have a working
==driver using the exported functions, and then post it all as a patch
==series.  That's the best way to show how you intend to use stuff like
==this.
==
==And I suggest you CC Alexey Orishko for all changes to the cdc_ncm
==driver.  The ST-Ericsson address in the driver is probably bouncing now,
==but you can reach him at "Alexey Orishko <alexey.orishko@gmail.com>"
==
==
==
==Bj?rn
==

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

* [RFC] huawei_cdc_ncm driver - status
  2013-07-02 12:01 ` Bjørn Mork
  2013-07-02 12:05   ` Enrico Mioso
@ 2013-07-02 17:04   ` Enrico Mioso
  2013-07-02 19:06     ` Enrico Mioso
  2013-07-02 20:06     ` [PATCH 0/3] make cdc_ncm_tx_fixup and cdc_ncm_rx_fixup non-static functions Enrico Mioso
  2013-07-10 12:27   ` [PATCH 1/3] net: cdc_ncm: Export cdc_ncm_{tx,rx}_fixup functions for re-use Enrico Mioso
                     ` (2 subsequent siblings)
  4 siblings, 2 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-02 17:04 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: netdev

Hi all guys!
I'm writing this mail to inform you about the progresso f the "new" driver - 
huawei_cdc_ncm.c, which should handle generally devices wich resembles the 
cdc_ncm standard, but embed another protocol on it (AT or something).
This driver has been developed starting from a Bjorn idea, and is based on the 
cdc_mbim.c driver.

Now - i need some help and corrections, review.
the driver currently loads and doesn't cause panics, and that's already a very 
good result for a first-time kernel programmer! :)
Even the driver name may be inappropriate, any ideas and suggestions are 
welcome!

Here is the driver .c file: in addition:
- I managed to export cdc_ncm {rx,tx}_fixup functions, modfying the needed .h 
file and adding symbol exporting.
- I removed from the cdc_ncm driver entries that made it bind t my dongle
- I added product and vendor id for my dongle, but probably doing something 
wrong...
- updated the build infrastructure in accordance (Kconfig, Makefile ...) and 
that works.


And here it is - please help me if you can.

/* copyrights and other things are importand but will be added later */
#include <linux/module.h>
#include <linux/netdevice.h>
#include <linux/ethtool.h>
#include <linux/if_vlan.h>
#include <linux/ip.h>
#include <linux/mii.h>
#include <linux/usb.h>
#include <linux/usb/cdc.h>
#include <linux/usb/usbnet.h>
#include <linux/usb/cdc-wdm.h>
#include <linux/usb/cdc_ncm.h>

/* Specific driver data */
struct huawei_cdc_ncm_state {
	struct cdc_ncm_ctx *ctx;
	atomic_t pmcount;
	struct usb_driver *subdriver;
	struct usb_interface *control;
	struct usb_interface *data;
};

static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
{
	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
	int rv = 0;

	if ((on && atomic_add_return(1, &drvstate->pmcount) == 1) || (!on && atomic_dec_and_test(&drvstate->pmcount))) {
		rv = usb_autopm_get_interface(usbnet_dev->intf);
		if (rv < 0)
			goto err;
		usbnet_dev->intf->needs_remote_wakeup = on;
		usb_autopm_put_interface(usbnet_dev->intf);
	}
err:
	return rv;
}

static int huawei_cdc_ncm_wdm_manage_power(struct usb_interface *intf, int status)
{
	struct usbnet *usbnet_dev = usb_get_intfdata(intf);

	/* can be called while disconnecting */
	if (!usbnet_dev)
		return 0;

	return huawei_cdc_ncm_manage_power(usbnet_dev, status);
}


/* Bind our driver to needed interfaces. Heavily inspired by cdc_mbim.c one! */
static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev, struct usb_interface *intf) {

	/* Some general use variables */
	struct cdc_ncm_ctx *ctx;
	struct usb_driver *subdriver = ERR_PTR(-ENODEV);
	int ret = -ENODEV;
	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
	
	/* Actually binds us to the device: use standard ncm function */
	ret = cdc_ncm_bind_common(usbnet_dev, intf, 1); /* altsetting should be 1 */
	
	/* Did the ncm bind function fail? */
	if (ret)
		goto err;
	
	/* Let cdc data ctx pointer point to our state structure */
	ctx = drvstate->ctx;
	
	if (usbnet_dev->status) 
		subdriver = usb_cdc_wdm_register(ctx->control, &usbnet_dev->status->desc, 4096, huawei_cdc_ncm_wdm_manage_power); /* a smarter way to calculate that constant? */
	if (IS_ERR(subdriver)) {
	 ret = PTR_ERR(subdriver);
	 cdc_ncm_unbind(usbnet_dev, intf);
   goto err;
	}
	
	/* Prevent usbnet from using the status descriptor */
	usbnet_dev->status = NULL;
	
	drvstate->subdriver = subdriver;
	
	/* FIXME: should we handle the vlan header in some way? */
	/* FIXME2: in my opinion we also could not do ARP, hoping somebody can help... */
err:
	return ret; /* ret = -ENODEV if not changed in the meanwhile */
}

static void huawei_cdc_ncm_unbind(struct usbnet *usbnet_dev, struct usb_interface *intf) {
	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
	struct cdc_ncm_ctx *ctx = drvstate->ctx;
	
	/* Bye cdc-wdm! And see you soon! :) */
	if (drvstate->subdriver && drvstate->subdriver->disconnect)
		drvstate->subdriver->disconnect(ctx->control);
	drvstate->subdriver = NULL;
	
	/* Unbind from both control and data interface */
	cdc_ncm_unbind(usbnet_dev, intf);
}

/* Suspend function - the logic has been taken from cdc_mbim.c directly. */
static int huawei_cdc_ncm_suspend(struct usb_interface *intf, pm_message_t message)
{
	int ret = 0;
	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
	struct cdc_ncm_ctx *ctx = drvstate->ctx;

	if (ctx == NULL) {
		ret = -1;
		goto error;
	}

	/*
	 * Both usbnet_suspend() and subdriver->suspend() MUST return 0
	 * in system sleep context, otherwise, the resume callback has
	 * to recover device from previous suspend failure.
	 */
	ret = usbnet_suspend(intf, message);
	if (ret < 0)
		goto error;

	if (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->suspend)
		ret = drvstate->subdriver->suspend(intf, message);
	if (ret < 0)
		usbnet_resume(intf);

error:
	return ret;
}

/* Resume function - logic took completely from huawei_cdc.c */
static int huawei_cdc_ncm_resume(struct usb_interface *intf)
{
	int ret = 0;
	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
	struct cdc_ncm_ctx *ctx = drvstate->ctx;
	bool callsub = (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->resume); /* should we call subdriver's resume? ? */

	if (callsub)
		ret = drvstate->subdriver->resume(intf);
	if (ret < 0)
		goto err;
	ret = usbnet_resume(intf);
	if (ret < 0 && callsub && drvstate->subdriver->suspend)
		drvstate->subdriver->suspend(intf, PMSG_SUSPEND);
err:
	return ret;
}


/* General driver informations, exposed partly to modutils */
static const struct driver_info huawei_cdc_info = {
	.description = "Huawei-style CDC NCM",
	.flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN,
	.bind = huawei_cdc_ncm_bind,
	.unbind = huawei_cdc_ncm_unbind,
	.manage_power = huawei_cdc_ncm_manage_power,
	/* NCM RX and TX fixup functions work properly on these devices */
	.rx_fixup = cdc_ncm_rx_fixup,
	.tx_fixup = cdc_ncm_tx_fixup,
};

static const struct usb_device_id huawei_cdc_ncm_devs[] = {
	/* The correct NCM handling will be implemented. For now, only my dongle
		will be handled.
		*/
	{ USB_DEVICE(0x12d1, 0x1506) },
	{
		/* Terminating entry */
	},
};
/* Expose table in module info */
MODULE_DEVICE_TABLE(usb, huawei_cdc_ncm_devs);


/* USB driver struct - used by USB stack */
static struct usb_driver huawei_cdc_ncm_driver = {
	.name = "huawei_cdc_ncm",
	.id_table = huawei_cdc_ncm_devs,
	.probe = usbnet_probe,
	.disconnect = usbnet_disconnect,
	.suspend = huawei_cdc_ncm_suspend,
	.resume = huawei_cdc_ncm_resume,
	.reset_resume = huawei_cdc_ncm_resume,
	.supports_autosuspend = 1,
	.disable_hub_initiated_lpm = 1,
};
module_usb_driver(huawei_cdc_ncm_driver);
MODULE_AUTHOR("Enrico Mioso <mrkiko.rs@gmail.com>");
MODULE_DESCRIPTION("USB CDC NCM host driver with encapsulated protocol support");
MODULE_LICENSE("GPL");

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

* Re: [RFC] huawei_cdc_ncm driver - status
  2013-07-02 17:04   ` [RFC] huawei_cdc_ncm driver - status Enrico Mioso
@ 2013-07-02 19:06     ` Enrico Mioso
  2013-07-02 20:06     ` [PATCH 0/3] make cdc_ncm_tx_fixup and cdc_ncm_rx_fixup non-static functions Enrico Mioso
  1 sibling, 0 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-02 19:06 UTC (permalink / raw)
  To: Enrico Mioso; +Cc: Bjørn Mork, netdev

Ok, I fixed my code - I forgot one of the most important things eve :) the 
registration.
Ok - now: the cdc-wdm character device is ok and working, the wwan interface 
has been created and for sure it will not work... :)
But the thing seems stable.



On Tue, 2 Jul 2013, Enrico Mioso wrote:

==Date: Tue, 2 Jul 2013 19:04:05 +0200 (CEST)
==From: Enrico Mioso <mrkiko.rs@gmail.com>
==To: Bj?rn Mork <bjorn@mork.no>
==Cc: netdev@vger.kernel.org
==Subject: [RFC] huawei_cdc_ncm driver - status
==
==Hi all guys!
==I'm writing this mail to inform you about the progresso f the "new" driver - 
==huawei_cdc_ncm.c, which should handle generally devices wich resembles the 
==cdc_ncm standard, but embed another protocol on it (AT or something).
==This driver has been developed starting from a Bjorn idea, and is based on the 
==cdc_mbim.c driver.
==
==Now - i need some help and corrections, review.
==the driver currently loads and doesn't cause panics, and that's already a very 
==good result for a first-time kernel programmer! :)
==Even the driver name may be inappropriate, any ideas and suggestions are 
==welcome!
==
==Here is the driver .c file: in addition:
==- I managed to export cdc_ncm {rx,tx}_fixup functions, modfying the needed .h 
==file and adding symbol exporting.
==- I removed from the cdc_ncm driver entries that made it bind t my dongle
==- I added product and vendor id for my dongle, but probably doing something 
==wrong...
==- updated the build infrastructure in accordance (Kconfig, Makefile ...) and 
==that works.
==
==
==And here it is - please help me if you can.
==
==/* copyrights and other things are importand but will be added later */
==#include <linux/module.h>
==#include <linux/netdevice.h>
==#include <linux/ethtool.h>
==#include <linux/if_vlan.h>
==#include <linux/ip.h>
==#include <linux/mii.h>
==#include <linux/usb.h>
==#include <linux/usb/cdc.h>
==#include <linux/usb/usbnet.h>
==#include <linux/usb/cdc-wdm.h>
==#include <linux/usb/cdc_ncm.h>
==
==/* Specific driver data */
==struct huawei_cdc_ncm_state {
==	struct cdc_ncm_ctx *ctx;
==	atomic_t pmcount;
==	struct usb_driver *subdriver;
==	struct usb_interface *control;
==	struct usb_interface *data;
==};
==
==static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
=={
==	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
==	int rv = 0;
==
==	if ((on && atomic_add_return(1, &drvstate->pmcount) == 1) || (!on && atomic_dec_and_test(&drvstate->pmcount))) {
==		rv = usb_autopm_get_interface(usbnet_dev->intf);
==		if (rv < 0)
==			goto err;
==		usbnet_dev->intf->needs_remote_wakeup = on;
==		usb_autopm_put_interface(usbnet_dev->intf);
==	}
==err:
==	return rv;
==}
==
==static int huawei_cdc_ncm_wdm_manage_power(struct usb_interface *intf, int status)
=={
==	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
==
==	/* can be called while disconnecting */
==	if (!usbnet_dev)
==		return 0;
==
==	return huawei_cdc_ncm_manage_power(usbnet_dev, status);
==}
==
==
==/* Bind our driver to needed interfaces. Heavily inspired by cdc_mbim.c one! */
==static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev, struct usb_interface *intf) {
==
==	/* Some general use variables */
==	struct cdc_ncm_ctx *ctx;
==	struct usb_driver *subdriver = ERR_PTR(-ENODEV);
==	int ret = -ENODEV;
==	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
==	
==	/* Actually binds us to the device: use standard ncm function */
==	ret = cdc_ncm_bind_common(usbnet_dev, intf, 1); /* altsetting should be 1 */
==	
==	/* Did the ncm bind function fail? */
==	if (ret)
==		goto err;
==	
==	/* Let cdc data ctx pointer point to our state structure */
==	ctx = drvstate->ctx;
==	
==	if (usbnet_dev->status) 
==		subdriver = usb_cdc_wdm_register(ctx->control, &usbnet_dev->status->desc, 4096, huawei_cdc_ncm_wdm_manage_power); /* a smarter way to calculate that constant? */
==	if (IS_ERR(subdriver)) {
==	 ret = PTR_ERR(subdriver);
==	 cdc_ncm_unbind(usbnet_dev, intf);
==   goto err;
==	}
==	
==	/* Prevent usbnet from using the status descriptor */
==	usbnet_dev->status = NULL;
==	
==	drvstate->subdriver = subdriver;
==	
==	/* FIXME: should we handle the vlan header in some way? */
==	/* FIXME2: in my opinion we also could not do ARP, hoping somebody can help... */
==err:
==	return ret; /* ret = -ENODEV if not changed in the meanwhile */
==}
==
==static void huawei_cdc_ncm_unbind(struct usbnet *usbnet_dev, struct usb_interface *intf) {
==	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
==	struct cdc_ncm_ctx *ctx = drvstate->ctx;
==	
==	/* Bye cdc-wdm! And see you soon! :) */
==	if (drvstate->subdriver && drvstate->subdriver->disconnect)
==		drvstate->subdriver->disconnect(ctx->control);
==	drvstate->subdriver = NULL;
==	
==	/* Unbind from both control and data interface */
==	cdc_ncm_unbind(usbnet_dev, intf);
==}
==
==/* Suspend function - the logic has been taken from cdc_mbim.c directly. */
==static int huawei_cdc_ncm_suspend(struct usb_interface *intf, pm_message_t message)
=={
==	int ret = 0;
==	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
==	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
==	struct cdc_ncm_ctx *ctx = drvstate->ctx;
==
==	if (ctx == NULL) {
==		ret = -1;
==		goto error;
==	}
==
==	/*
==	 * Both usbnet_suspend() and subdriver->suspend() MUST return 0
==	 * in system sleep context, otherwise, the resume callback has
==	 * to recover device from previous suspend failure.
==	 */
==	ret = usbnet_suspend(intf, message);
==	if (ret < 0)
==		goto error;
==
==	if (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->suspend)
==		ret = drvstate->subdriver->suspend(intf, message);
==	if (ret < 0)
==		usbnet_resume(intf);
==
==error:
==	return ret;
==}
==
==/* Resume function - logic took completely from huawei_cdc.c */
==static int huawei_cdc_ncm_resume(struct usb_interface *intf)
=={
==	int ret = 0;
==	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
==	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
==	struct cdc_ncm_ctx *ctx = drvstate->ctx;
==	bool callsub = (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->resume); /* should we call subdriver's resume? ? */
==
==	if (callsub)
==		ret = drvstate->subdriver->resume(intf);
==	if (ret < 0)
==		goto err;
==	ret = usbnet_resume(intf);
==	if (ret < 0 && callsub && drvstate->subdriver->suspend)
==		drvstate->subdriver->suspend(intf, PMSG_SUSPEND);
==err:
==	return ret;
==}
==
==
==/* General driver informations, exposed partly to modutils */
==static const struct driver_info huawei_cdc_info = {
==	.description = "Huawei-style CDC NCM",
==	.flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN,
==	.bind = huawei_cdc_ncm_bind,
==	.unbind = huawei_cdc_ncm_unbind,
==	.manage_power = huawei_cdc_ncm_manage_power,
==	/* NCM RX and TX fixup functions work properly on these devices */
==	.rx_fixup = cdc_ncm_rx_fixup,
==	.tx_fixup = cdc_ncm_tx_fixup,
==};
==
==static const struct usb_device_id huawei_cdc_ncm_devs[] = {
==	/* The correct NCM handling will be implemented. For now, only my dongle
==		will be handled.
==		*/
==	{ USB_DEVICE(0x12d1, 0x1506) },
==	{
==		/* Terminating entry */
==	},
==};
==/* Expose table in module info */
==MODULE_DEVICE_TABLE(usb, huawei_cdc_ncm_devs);
==
==
==/* USB driver struct - used by USB stack */
==static struct usb_driver huawei_cdc_ncm_driver = {
==	.name = "huawei_cdc_ncm",
==	.id_table = huawei_cdc_ncm_devs,
==	.probe = usbnet_probe,
==	.disconnect = usbnet_disconnect,
==	.suspend = huawei_cdc_ncm_suspend,
==	.resume = huawei_cdc_ncm_resume,
==	.reset_resume = huawei_cdc_ncm_resume,
==	.supports_autosuspend = 1,
==	.disable_hub_initiated_lpm = 1,
==};
==module_usb_driver(huawei_cdc_ncm_driver);
==MODULE_AUTHOR("Enrico Mioso <mrkiko.rs@gmail.com>");
==MODULE_DESCRIPTION("USB CDC NCM host driver with encapsulated protocol support");
==MODULE_LICENSE("GPL");
==

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

* [PATCH 0/3] make cdc_ncm_tx_fixup and cdc_ncm_rx_fixup non-static functions
  2013-07-02 17:04   ` [RFC] huawei_cdc_ncm driver - status Enrico Mioso
  2013-07-02 19:06     ` Enrico Mioso
@ 2013-07-02 20:06     ` Enrico Mioso
  2013-07-02 20:10       ` [PATCH 1/3] Export cdc_ncm_{tx,rx}_fixup functions Enrico Mioso
  2013-07-03  7:45       ` [PATCH 0/3] make cdc_ncm_tx_fixup and cdc_ncm_rx_fixup non-static functions Bjørn Mork
  1 sibling, 2 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-02 20:06 UTC (permalink / raw)
  To: netdev; +Cc: Bjørn Mork

Let cdc_ncm_tx_fixup and cdc_ncm_rx_fixup be non-static

This patchs makes those functions non-static, so that they could be exported 
for use by other drivers.

Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>


diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 43afde8..493277e 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -858,7 +858,7 @@ static void cdc_ncm_txpath_bh(unsigned long param)
 	}
 }
 
-static struct sk_buff *
+struct sk_buff *
 cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
 {
 	struct sk_buff *skb_out;
@@ -965,7 +965,7 @@ error:
 }
 EXPORT_SYMBOL_GPL(cdc_ncm_rx_verify_ndp16);
 
-static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
+int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 {
 	struct sk_buff *skb;
 	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];



On Tue, 2 Jul 2013, Enrico Mioso wrote:

==Date: Tue, 2 Jul 2013 19:04:05 +0200 (CEST)
==From: Enrico Mioso <mrkiko.rs@gmail.com>
==To: Bj?rn Mork <bjorn@mork.no>
==Cc: netdev@vger.kernel.org
==Subject: [RFC] huawei_cdc_ncm driver - status
==
==Hi all guys!
==I'm writing this mail to inform you about the progresso f the "new" driver - 
==huawei_cdc_ncm.c, which should handle generally devices wich resembles the 
==cdc_ncm standard, but embed another protocol on it (AT or something).
==This driver has been developed starting from a Bjorn idea, and is based on the 
==cdc_mbim.c driver.
==
==Now - i need some help and corrections, review.
==the driver currently loads and doesn't cause panics, and that's already a very 
==good result for a first-time kernel programmer! :)
==Even the driver name may be inappropriate, any ideas and suggestions are 
==welcome!
==
==Here is the driver .c file: in addition:
==- I managed to export cdc_ncm {rx,tx}_fixup functions, modfying the needed .h 
==file and adding symbol exporting.
==- I removed from the cdc_ncm driver entries that made it bind t my dongle
==- I added product and vendor id for my dongle, but probably doing something 
==wrong...
==- updated the build infrastructure in accordance (Kconfig, Makefile ...) and 
==that works.
==
==
==And here it is - please help me if you can.
==
==/* copyrights and other things are importand but will be added later */
==#include <linux/module.h>
==#include <linux/netdevice.h>
==#include <linux/ethtool.h>
==#include <linux/if_vlan.h>
==#include <linux/ip.h>
==#include <linux/mii.h>
==#include <linux/usb.h>
==#include <linux/usb/cdc.h>
==#include <linux/usb/usbnet.h>
==#include <linux/usb/cdc-wdm.h>
==#include <linux/usb/cdc_ncm.h>
==
==/* Specific driver data */
==struct huawei_cdc_ncm_state {
==	struct cdc_ncm_ctx *ctx;
==	atomic_t pmcount;
==	struct usb_driver *subdriver;
==	struct usb_interface *control;
==	struct usb_interface *data;
==};
==
==static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
=={
==	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
==	int rv = 0;
==
==	if ((on && atomic_add_return(1, &drvstate->pmcount) == 1) || (!on && atomic_dec_and_test(&drvstate->pmcount))) {
==		rv = usb_autopm_get_interface(usbnet_dev->intf);
==		if (rv < 0)
==			goto err;
==		usbnet_dev->intf->needs_remote_wakeup = on;
==		usb_autopm_put_interface(usbnet_dev->intf);
==	}
==err:
==	return rv;
==}
==
==static int huawei_cdc_ncm_wdm_manage_power(struct usb_interface *intf, int status)
=={
==	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
==
==	/* can be called while disconnecting */
==	if (!usbnet_dev)
==		return 0;
==
==	return huawei_cdc_ncm_manage_power(usbnet_dev, status);
==}
==
==
==/* Bind our driver to needed interfaces. Heavily inspired by cdc_mbim.c one! */
==static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev, struct usb_interface *intf) {
==
==	/* Some general use variables */
==	struct cdc_ncm_ctx *ctx;
==	struct usb_driver *subdriver = ERR_PTR(-ENODEV);
==	int ret = -ENODEV;
==	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
==	
==	/* Actually binds us to the device: use standard ncm function */
==	ret = cdc_ncm_bind_common(usbnet_dev, intf, 1); /* altsetting should be 1 */
==	
==	/* Did the ncm bind function fail? */
==	if (ret)
==		goto err;
==	
==	/* Let cdc data ctx pointer point to our state structure */
==	ctx = drvstate->ctx;
==	
==	if (usbnet_dev->status) 
==		subdriver = usb_cdc_wdm_register(ctx->control, &usbnet_dev->status->desc, 4096, huawei_cdc_ncm_wdm_manage_power); /* a smarter way to calculate that constant? */
==	if (IS_ERR(subdriver)) {
==	 ret = PTR_ERR(subdriver);
==	 cdc_ncm_unbind(usbnet_dev, intf);
==   goto err;
==	}
==	
==	/* Prevent usbnet from using the status descriptor */
==	usbnet_dev->status = NULL;
==	
==	drvstate->subdriver = subdriver;
==	
==	/* FIXME: should we handle the vlan header in some way? */
==	/* FIXME2: in my opinion we also could not do ARP, hoping somebody can help... */
==err:
==	return ret; /* ret = -ENODEV if not changed in the meanwhile */
==}
==
==static void huawei_cdc_ncm_unbind(struct usbnet *usbnet_dev, struct usb_interface *intf) {
==	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
==	struct cdc_ncm_ctx *ctx = drvstate->ctx;
==	
==	/* Bye cdc-wdm! And see you soon! :) */
==	if (drvstate->subdriver && drvstate->subdriver->disconnect)
==		drvstate->subdriver->disconnect(ctx->control);
==	drvstate->subdriver = NULL;
==	
==	/* Unbind from both control and data interface */
==	cdc_ncm_unbind(usbnet_dev, intf);
==}
==
==/* Suspend function - the logic has been taken from cdc_mbim.c directly. */
==static int huawei_cdc_ncm_suspend(struct usb_interface *intf, pm_message_t message)
=={
==	int ret = 0;
==	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
==	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
==	struct cdc_ncm_ctx *ctx = drvstate->ctx;
==
==	if (ctx == NULL) {
==		ret = -1;
==		goto error;
==	}
==
==	/*
==	 * Both usbnet_suspend() and subdriver->suspend() MUST return 0
==	 * in system sleep context, otherwise, the resume callback has
==	 * to recover device from previous suspend failure.
==	 */
==	ret = usbnet_suspend(intf, message);
==	if (ret < 0)
==		goto error;
==
==	if (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->suspend)
==		ret = drvstate->subdriver->suspend(intf, message);
==	if (ret < 0)
==		usbnet_resume(intf);
==
==error:
==	return ret;
==}
==
==/* Resume function - logic took completely from huawei_cdc.c */
==static int huawei_cdc_ncm_resume(struct usb_interface *intf)
=={
==	int ret = 0;
==	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
==	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
==	struct cdc_ncm_ctx *ctx = drvstate->ctx;
==	bool callsub = (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->resume); /* should we call subdriver's resume? ? */
==
==	if (callsub)
==		ret = drvstate->subdriver->resume(intf);
==	if (ret < 0)
==		goto err;
==	ret = usbnet_resume(intf);
==	if (ret < 0 && callsub && drvstate->subdriver->suspend)
==		drvstate->subdriver->suspend(intf, PMSG_SUSPEND);
==err:
==	return ret;
==}
==
==
==/* General driver informations, exposed partly to modutils */
==static const struct driver_info huawei_cdc_info = {
==	.description = "Huawei-style CDC NCM",
==	.flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN,
==	.bind = huawei_cdc_ncm_bind,
==	.unbind = huawei_cdc_ncm_unbind,
==	.manage_power = huawei_cdc_ncm_manage_power,
==	/* NCM RX and TX fixup functions work properly on these devices */
==	.rx_fixup = cdc_ncm_rx_fixup,
==	.tx_fixup = cdc_ncm_tx_fixup,
==};
==
==static const struct usb_device_id huawei_cdc_ncm_devs[] = {
==	/* The correct NCM handling will be implemented. For now, only my dongle
==		will be handled.
==		*/
==	{ USB_DEVICE(0x12d1, 0x1506) },
==	{
==		/* Terminating entry */
==	},
==};
==/* Expose table in module info */
==MODULE_DEVICE_TABLE(usb, huawei_cdc_ncm_devs);
==
==
==/* USB driver struct - used by USB stack */
==static struct usb_driver huawei_cdc_ncm_driver = {
==	.name = "huawei_cdc_ncm",
==	.id_table = huawei_cdc_ncm_devs,
==	.probe = usbnet_probe,
==	.disconnect = usbnet_disconnect,
==	.suspend = huawei_cdc_ncm_suspend,
==	.resume = huawei_cdc_ncm_resume,
==	.reset_resume = huawei_cdc_ncm_resume,
==	.supports_autosuspend = 1,
==	.disable_hub_initiated_lpm = 1,
==};
==module_usb_driver(huawei_cdc_ncm_driver);
==MODULE_AUTHOR("Enrico Mioso <mrkiko.rs@gmail.com>");
==MODULE_DESCRIPTION("USB CDC NCM host driver with encapsulated protocol support");
==MODULE_LICENSE("GPL");
==

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

* [PATCH 1/3] Export cdc_ncm_{tx,rx}_fixup functions
  2013-07-02 20:06     ` [PATCH 0/3] make cdc_ncm_tx_fixup and cdc_ncm_rx_fixup non-static functions Enrico Mioso
@ 2013-07-02 20:10       ` Enrico Mioso
  2013-07-02 20:13         ` [PATCH RFC 2/3] Introduce huawei_cdc_ncm driver Enrico Mioso
  2013-07-03  7:43         ` [PATCH 1/3] Export cdc_ncm_{tx,rx}_fixup functions Bjørn Mork
  2013-07-03  7:45       ` [PATCH 0/3] make cdc_ncm_tx_fixup and cdc_ncm_rx_fixup non-static functions Bjørn Mork
  1 sibling, 2 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-02 20:10 UTC (permalink / raw)
  To: netdev; +Cc: Bjørn Mork

Exports those functions to be able to re-use them in other drivers if needed.

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 493277e..22ed51c 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -885,6 +885,8 @@ error:
 
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(cdc_ncm_tx_fixup);
+
 
 /* verify NTB header and return offset of first NDP, or negative error */
 int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
@@ -1040,6 +1042,7 @@ err_ndp:
 error:
 	return 0;
 }
+EXPORT_SYMBOL_GPL(cdc_ncm_rx_fixup);
 
 static void
 cdc_ncm_speed_change(struct cdc_ncm_ctx *ctx,
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index cc25b70..163244b 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -133,3 +133,6 @@ extern void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
 extern struct sk_buff *cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign);
 extern int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in);
 extern int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
+struct sk_buff *
+cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags);
+int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in);

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

* [PATCH RFC 2/3] Introduce huawei_cdc_ncm driver
  2013-07-02 20:10       ` [PATCH 1/3] Export cdc_ncm_{tx,rx}_fixup functions Enrico Mioso
@ 2013-07-02 20:13         ` Enrico Mioso
  2013-07-02 20:15           ` [PATCH RFC 3/3] huawei_cdc_ncm base skeleton Enrico Mioso
  2013-07-03  7:40           ` [PATCH RFC 2/3] Introduce huawei_cdc_ncm driver Bjørn Mork
  2013-07-03  7:43         ` [PATCH 1/3] Export cdc_ncm_{tx,rx}_fixup functions Bjørn Mork
  1 sibling, 2 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-02 20:13 UTC (permalink / raw)
  To: netdev; +Cc: Bjørn Mork

This patch, introduce the huawei_cdc_ncm driver, modifying the build 
infrastructure as needed.

Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>


diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 287cc62..33cb175 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -246,6 +246,18 @@ config USB_NET_CDC_NCM
 	    * ST-Ericsson M343 HSPA Mobile Broadband Modem (reference design)
 	    * Ericsson F5521gw Mobile Broadband Module
 
+config USB_NET_HUAWEI_CDC_NCM
+	tristate "Huawei-style CDC NCM support"
+	depends on USB_USBNET
+	default y
+	select USB_WDM
+	select USB_NET_CDC_NCM
+	help
+		This driver aims to support huawei-style NCM devices, that use ncm as a 
+		transport for other protols.
+		To compile this driver as a module, choose M here: the module will be 
+		called huawei_cdc_ncm.
+
 config USB_NET_CDC_MBIM
 	tristate "CDC MBIM support"
 	depends on USB_USBNET
diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index 9ab5c9d..12a0279 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_USB_IPHETH)	+= ipheth.o
 obj-$(CONFIG_USB_SIERRA_NET)	+= sierra_net.o
 obj-$(CONFIG_USB_NET_CX82310_ETH)	+= cx82310_eth.o
 obj-$(CONFIG_USB_NET_CDC_NCM)	+= cdc_ncm.o
+obj-$(CONFIG_USB_NET_HUAWEI_CDC_NCM)	+= huawei_cdc_ncm.o
 obj-$(CONFIG_USB_VL600)		+= lg-vl600.o
 obj-$(CONFIG_USB_NET_QMI_WWAN)	+= qmi_wwan.o
 obj-$(CONFIG_USB_NET_CDC_MBIM)	+= cdc_mbim.o

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

* Re: [PATCH RFC 3/3] huawei_cdc_ncm base skeleton
  2013-07-02 20:13         ` [PATCH RFC 2/3] Introduce huawei_cdc_ncm driver Enrico Mioso
@ 2013-07-02 20:15           ` Enrico Mioso
  2013-07-03  6:40             ` Huaei E3131 - connected! Enrico Mioso
  2013-07-03  7:38             ` [PATCH RFC 3/3] huawei_cdc_ncm base skeleton Bjørn Mork
  2013-07-03  7:40           ` [PATCH RFC 2/3] Introduce huawei_cdc_ncm driver Bjørn Mork
  1 sibling, 2 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-02 20:15 UTC (permalink / raw)
  To: Enrico Mioso; +Cc: netdev, Bjørn Mork

This is more an RFC than a patch.
This is the huawei_cdc_ncm driver skeleton - it handles the embedded AT channel 
on my device, and is stable upon usage, installing and removal.
The wwan interface is not functional - and surely there are lots of errors!

Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>

diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c
new file mode 100644
index 0000000..2e76c87
--- /dev/null
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -0,0 +1,239 @@
+/*
+ * huawei_cdc_ncm.c - handles hawei-style NCM devices.
+ * Copyright (C) Enrico Mioso <mrkiko.rs@gmail.com>
+ * This driver handles devices resembling the CDC NCM standard, but 
+ * encapsulating another protocl inside it. An example are some Huawei modems.
+ * This code is based on the original cdc_ncm implementation, that is:
+ * Copyright (C) ST-Ericsson 2010-2012
+ * Contact: Alexey Orishko <alexey.orishko@stericsson.com>
+ * Original author: Hans Petter Selasky <hans.petter.selasky@stericsson.com>
+ *
+ * USB Host Driver for Network Control Model (NCM)
+ * http://www.usb.org/developers/devclass_docs/NCM10.zip
+ *
+ * The NCM encoding, decoding and initialization logic
+ * derives from FreeBSD 8.x. if_cdce.c and if_cdcereg.h
+ *
+ * This software is available to you under a choice of one of two
+ * licenses. You may choose this file to be licensed under the terms
+ * of the GNU General Public License (GPL) Version 2 or the 2-clause
+ * BSD license listed below:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/ethtool.h>
+#include <linux/if_vlan.h>
+#include <linux/ip.h>
+#include <linux/mii.h>
+#include <linux/usb.h>
+#include <linux/usb/cdc.h>
+#include <linux/usb/usbnet.h>
+#include <linux/usb/cdc-wdm.h>
+#include <linux/usb/cdc_ncm.h>
+
+/* Specific driver data */
+struct huawei_cdc_ncm_state {
+	struct cdc_ncm_ctx *ctx;
+	atomic_t pmcount;
+	struct usb_driver *subdriver;
+	struct usb_interface *control;
+	struct usb_interface *data;
+};
+
+static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
+{
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	int rv = 0;
+
+	if ((on && atomic_add_return(1, &drvstate->pmcount) == 1) || (!on && atomic_dec_and_test(&drvstate->pmcount))) {
+		rv = usb_autopm_get_interface(usbnet_dev->intf);
+		if (rv < 0)
+			goto err;
+		usbnet_dev->intf->needs_remote_wakeup = on;
+		usb_autopm_put_interface(usbnet_dev->intf);
+	}
+err:
+	return rv;
+}
+
+static int huawei_cdc_ncm_wdm_manage_power(struct usb_interface *intf, int status)
+{
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+
+	/* can be called while disconnecting */
+	if (!usbnet_dev)
+		return 0;
+
+	return huawei_cdc_ncm_manage_power(usbnet_dev, status);
+}
+
+
+/* Bind our driver to needed interfaces. Heavily inspired by cdc_mbim.c one! */
+static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev, struct usb_interface *intf) {
+
+	/* Some general use variables */
+	struct cdc_ncm_ctx *ctx;
+	struct usb_driver *subdriver = ERR_PTR(-ENODEV);
+	int ret = -ENODEV;
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	
+	/* Actually binds us to the device: use standard ncm function */
+	ret = cdc_ncm_bind_common(usbnet_dev, intf, 1); /* altsetting should be 1 */
+	
+	/* Did the ncm bind function fail? */
+	if (ret)
+		goto err;
+	
+	/* Let cdc data ctx pointer point to our state structure */
+	ctx = drvstate->ctx;
+	
+	if (usbnet_dev->status) 
+		subdriver = usb_cdc_wdm_register(ctx->control, &usbnet_dev->status->desc, 4096, huawei_cdc_ncm_wdm_manage_power); /* a smarter way to calculate that constant? */
+	if (IS_ERR(subdriver)) {
+	 ret = PTR_ERR(subdriver);
+	 cdc_ncm_unbind(usbnet_dev, intf);
+   goto err;
+	}
+	
+	/* Prevent usbnet from using the status descriptor */
+	usbnet_dev->status = NULL;
+	
+	drvstate->subdriver = subdriver;
+	
+	/* FIXME: should we handle the vlan header in some way? */
+	/* FIXME2: in my opinion we also could not do ARP, hoping somebody can help... */
+err:
+	return ret; /* ret = -ENODEV if not changed in the meanwhile */
+}
+
+static void huawei_cdc_ncm_unbind(struct usbnet *usbnet_dev, struct usb_interface *intf) {
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+	
+	/* Bye cdc-wdm! And see you soon! :) */
+	if (drvstate->subdriver && drvstate->subdriver->disconnect)
+		drvstate->subdriver->disconnect(ctx->control);
+	drvstate->subdriver = NULL;
+	
+	/* Unbind from both control and data interface */
+	cdc_ncm_unbind(usbnet_dev, intf);
+}
+
+/* Suspend function - the logic has been taken from cdc_mbim.c directly. */
+static int huawei_cdc_ncm_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	int ret = 0;
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+
+	if (ctx == NULL) {
+		ret = -1;
+		goto error;
+	}
+
+	/*
+	 * Both usbnet_suspend() and subdriver->suspend() MUST return 0
+	 * in system sleep context, otherwise, the resume callback has
+	 * to recover device from previous suspend failure.
+	 */
+	ret = usbnet_suspend(intf, message);
+	if (ret < 0)
+		goto error;
+
+	if (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->suspend)
+		ret = drvstate->subdriver->suspend(intf, message);
+	if (ret < 0)
+		usbnet_resume(intf);
+
+error:
+	return ret;
+}
+
+/* Resume function - logic took completely from huawei_cdc.c */
+static int huawei_cdc_ncm_resume(struct usb_interface *intf)
+{
+	int ret = 0;
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+	bool callsub = (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->resume); /* should we call subdriver's resume? ? */
+
+	if (callsub)
+		ret = drvstate->subdriver->resume(intf);
+	if (ret < 0)
+		goto err;
+	ret = usbnet_resume(intf);
+	if (ret < 0 && callsub && drvstate->subdriver->suspend)
+		drvstate->subdriver->suspend(intf, PMSG_SUSPEND);
+err:
+	return ret;
+}
+
+
+/* General driver informations, exposed partly to modutils */
+static const struct driver_info huawei_cdc_ncm_info = {
+	.description = "Huawei-style CDC NCM",
+	.flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN,
+	.bind = huawei_cdc_ncm_bind,
+	.unbind = huawei_cdc_ncm_unbind,
+	.manage_power = huawei_cdc_ncm_manage_power,
+	/* NCM RX and TX fixup functions work properly on these devices */
+	.rx_fixup = cdc_ncm_rx_fixup,
+	.tx_fixup = cdc_ncm_tx_fixup,
+};
+
+static const struct usb_device_id huawei_cdc_ncm_devs[] = {
+	/* The correct NCM handling will be implemented. For now, only my dongle
+		will be handled.
+		*/
+	{ USB_DEVICE_INTERFACE_NUMBER(0x12d1, 0x1506, 1), \
+		.driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+
+	{
+		/* Terminating entry */
+	},
+};
+/* Expose table in module info */
+MODULE_DEVICE_TABLE(usb, huawei_cdc_ncm_devs);
+
+
+/* USB driver struct - used by USB stack */
+static struct usb_driver huawei_cdc_ncm_driver = {
+	.name = "huawei_cdc_ncm",
+	.id_table = huawei_cdc_ncm_devs,
+	.probe = usbnet_probe,
+	.disconnect = usbnet_disconnect,
+	.suspend = huawei_cdc_ncm_suspend,
+	.resume = huawei_cdc_ncm_resume,
+	.reset_resume = huawei_cdc_ncm_resume,
+	.supports_autosuspend = 1,
+	.disable_hub_initiated_lpm = 1,
+};
+module_usb_driver(huawei_cdc_ncm_driver);
+MODULE_AUTHOR("Enrico Mioso <mrkiko.rs@gmail.com>");
+MODULE_DESCRIPTION("USB CDC NCM host driver with encapsulated protocol support");
+MODULE_LICENSE("GPL");

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

* Huaei E3131 - connected!
  2013-07-02 20:15           ` [PATCH RFC 3/3] huawei_cdc_ncm base skeleton Enrico Mioso
@ 2013-07-03  6:40             ` Enrico Mioso
  2013-07-03  7:11               ` Huaei E3131 - status Enrico Mioso
  2013-07-03  7:13               ` Huaei E3131 - connected! Bjørn Mork
  2013-07-03  7:38             ` [PATCH RFC 3/3] huawei_cdc_ncm base skeleton Bjørn Mork
  1 sibling, 2 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-03  6:40 UTC (permalink / raw)
  To: netdev; +Cc: Bjørn Mork

Hi guys!
there's a problem ... I don't understand why even looking at the code (sure, I 
will look at it again): but, bringing up the wwan0 interface simply works. So 
now I'm sending this mail via the device's wwan interface and while looking and 
typing to the wdm character device, without any apparent problem.
I did not implement the status handler, but the driver seems to be able to 
work, probably missing some notifications?
This needs to be EXTRENSIVELY polished and reviewed - but it's encouraging!

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

* Huaei E3131 - status
  2013-07-03  6:40             ` Huaei E3131 - connected! Enrico Mioso
@ 2013-07-03  7:11               ` Enrico Mioso
  2013-07-03  8:15                 ` Bjørn Mork
  2013-07-03  7:13               ` Huaei E3131 - connected! Bjørn Mork
  1 sibling, 1 reply; 40+ messages in thread
From: Enrico Mioso @ 2013-07-03  7:11 UTC (permalink / raw)
  To: netdev; +Cc: Bjørn Mork

Ok ... As Bjorn stated, cdc-wdm is handling the notifications now - or, better: 
is not handling them, as it is not made to do these things! The connection 
stays up and the character device seems to work properly. Obviously cdc-wdm 
notices me about one single unknown notifications.
We're ignoring all the notifications from the NCM erspective, but all works 
because the device probably doesn't rely on them so much.

Aniway - now I'm conscious about why it works. Now it's time to improve the 
situation of the driver, and might be the api. Waiting for suggestions and 
injuries! :)

thank you everybody for the help...

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

* Re: Huaei E3131 - connected!
  2013-07-03  6:40             ` Huaei E3131 - connected! Enrico Mioso
  2013-07-03  7:11               ` Huaei E3131 - status Enrico Mioso
@ 2013-07-03  7:13               ` Bjørn Mork
  1 sibling, 0 replies; 40+ messages in thread
From: Bjørn Mork @ 2013-07-03  7:13 UTC (permalink / raw)
  To: Enrico Mioso; +Cc: netdev

Enrico Mioso <mrkiko.rs@gmail.com> writes:

> Hi guys!
> there's a problem ... I don't understand why even looking at the code (sure, I 
> will look at it again): but, bringing up the wwan0 interface simply works. So 
> now I'm sending this mail via the device's wwan interface and while looking and 
> typing to the wdm character device, without any apparent problem.
> I did not implement the status handler, but the driver seems to be able to 
> work, probably missing some notifications?
> This needs to be EXTRENSIVELY polished and reviewed - but it's encouraging!


Great work!

The NCM notifications should be handled, but I don't think it is crucial
to have that in place for the initial version of the driver. Adding that
support means changing the cdc-wdm API, which need some careful planning
and time because cdc-wdm is in another subsystem (usb vs net).

Since this driver is not dealing with NCM conformant devices, but only
implementing a NCM like vendor specific protocol, I believe it is
acceptable to handle the notifcations as "phase 2" of the driver.

In my opinion, all you need to do before submitting is fixup the device
list, moving the Huawei vendor specific entries from cdc_ncm to this
driver.  You might as well do that as part of introducing the driver.
Or make it a separate final patch if you prefer.



Bjørn

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

* Re: [PATCH RFC 3/3] huawei_cdc_ncm base skeleton
  2013-07-02 20:15           ` [PATCH RFC 3/3] huawei_cdc_ncm base skeleton Enrico Mioso
  2013-07-03  6:40             ` Huaei E3131 - connected! Enrico Mioso
@ 2013-07-03  7:38             ` Bjørn Mork
  2013-07-03 10:16               ` Enrico Mioso
  1 sibling, 1 reply; 40+ messages in thread
From: Bjørn Mork @ 2013-07-03  7:38 UTC (permalink / raw)
  To: Enrico Mioso; +Cc: netdev

Enrico Mioso <mrkiko.rs@gmail.com> writes:

> This is more an RFC than a patch.

It looks pretty complete to me :)

> + * This code is based on the original cdc_ncm implementation, that is:
> + * Copyright (C) ST-Ericsson 2010-2012
> + * Contact: Alexey Orishko <alexey.orishko@stericsson.com>
> + * Original author: Hans Petter Selasky <hans.petter.selasky@stericsson.com>
> + *
> + * USB Host Driver for Network Control Model (NCM)
> + * http://www.usb.org/developers/devclass_docs/NCM10.zip
> + *
> + * The NCM encoding, decoding and initialization logic
> + * derives from FreeBSD 8.x. if_cdce.c and if_cdcereg.h
> + *
> + * This software is available to you under a choice of one of two
> + * licenses. You may choose this file to be licensed under the terms
> + * of the GNU General Public License (GPL) Version 2 or the 2-clause
> + * BSD license listed below:

This text does not match the MODULE_LICENSE you have used. You should
probably take a few minutes to think about how you want your work to be
licensed and use the appropriate combination of license comment and
MODULE_LICENSE.

You do in any case not need to copy all this from the cdc_ncm driver.
You reuse code via the exported API, but the code in this driver is
mostly your own.  A small note of cdc_ncm use is nice, but I don't think
it's any more necessary than for other kernel code you use via #includes.


> +/* Bind our driver to needed interfaces. Heavily inspired by cdc_mbim.c one! */

Many of your comments look mostly like notes for yourself.  Which they
probably are, given that you sent this as a RFC :)

You should go over all these comments and think about whether they are
useful for others reading this code.  I don't think this one is, for
example.

> +static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev, struct usb_interface *intf) {
> +
> +	/* Some general use variables */
> +	struct cdc_ncm_ctx *ctx;
> +	struct usb_driver *subdriver = ERR_PTR(-ENODEV);
> +	int ret = -ENODEV;
> +	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
> +	
> +	/* Actually binds us to the device: use standard ncm function */
> +	ret = cdc_ncm_bind_common(usbnet_dev, intf, 1); /* altsetting should be 1 */
> +	
> +	/* Did the ncm bind function fail? */
> +	if (ret)
> +		goto err;
> +	
> +	/* Let cdc data ctx pointer point to our state structure */
> +	ctx = drvstate->ctx;
> +	
> +	if (usbnet_dev->status) 
> +		subdriver = usb_cdc_wdm_register(ctx->control, &usbnet_dev->status->desc, 4096, huawei_cdc_ncm_wdm_manage_power); /* a smarter way to calculate that constant? */

Yes, we were going to revisit that constant as soon as you discovered
the protocol.  Now you found that it is AT commands, which doesn't
really provide an absolute answer.  But AT commands is actually the
normal usecase for cdc-wdm, so I believe using the defaults from that
specification makes some sense.  As noted on the cdc-wdm driver:

  CDC-WMC r1.1 requires wMaxCommand to be "at least 256 decimal (0x100)"

Maybe 256 is enough here? Or are there Huawei specific AT commands with
larger responses than that?  I don't know.  Choose some number and
change the comment to explain the reasoning behind that choice.


> +	if (IS_ERR(subdriver)) {
> +	 ret = PTR_ERR(subdriver);
> +	 cdc_ncm_unbind(usbnet_dev, intf);
> +   goto err;
> +	}
> +	
> +	/* Prevent usbnet from using the status descriptor */
> +	usbnet_dev->status = NULL;
> +	
> +	drvstate->subdriver = subdriver;
> +	
> +	/* FIXME: should we handle the vlan header in some way? */

No, that is in cdc_mbim because it maps multiplexed MBIM IP sessions to
ethernet VLAN interfaces.  The devices supported by this driver cannot
support more than one IP session per USB function, so there is
absolutely nothing you or the firmware can map a VLAN to.  Just drop the
comment. 

> +	/* FIXME2: in my opinion we also could not do ARP, hoping somebody can help... */

The firmware obviously does ARP as it works whether you disable it or
not.  The usefulness can be discussed, but you cannot drop it without
testing that it does in fact work.  The firmware most likely use either
DHCP or ARP requests to figure out your MAC address, so the ARP requests
might be required even if they look silly.


> +static const struct usb_device_id huawei_cdc_ncm_devs[] = {
> +	/* The correct NCM handling will be implemented. For now, only my dongle
> +		will be handled.
> +		*/
> +	{ USB_DEVICE_INTERFACE_NUMBER(0x12d1, 0x1506, 1), \
> +		.driver_info = (unsigned long)&huawei_cdc_ncm_info,
> +	},
> +
> +	{
> +		/* Terminating entry */
> +	},
> +};


As you note, that table needs to match on class codes.  Just move the
Huawei vendor specific entries from cdc_ncm.



Bjørn

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

* Re: [PATCH RFC 2/3] Introduce huawei_cdc_ncm driver
  2013-07-02 20:13         ` [PATCH RFC 2/3] Introduce huawei_cdc_ncm driver Enrico Mioso
  2013-07-02 20:15           ` [PATCH RFC 3/3] huawei_cdc_ncm base skeleton Enrico Mioso
@ 2013-07-03  7:40           ` Bjørn Mork
  1 sibling, 0 replies; 40+ messages in thread
From: Bjørn Mork @ 2013-07-03  7:40 UTC (permalink / raw)
  To: Enrico Mioso; +Cc: netdev

Enrico Mioso <mrkiko.rs@gmail.com> writes:

> This patch, introduce the huawei_cdc_ncm driver, modifying the build 
> infrastructure as needed.
>
> Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
>
>
> diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
> index 287cc62..33cb175 100644
> --- a/drivers/net/usb/Kconfig
> +++ b/drivers/net/usb/Kconfig
> @@ -246,6 +246,18 @@ config USB_NET_CDC_NCM
>  	    * ST-Ericsson M343 HSPA Mobile Broadband Modem (reference design)
>  	    * Ericsson F5521gw Mobile Broadband Module
>  
> +config USB_NET_HUAWEI_CDC_NCM
> +	tristate "Huawei-style CDC NCM support"
> +	depends on USB_USBNET
> +	default y
> +	select USB_WDM
> +	select USB_NET_CDC_NCM
> +	help
> +		This driver aims to support huawei-style NCM devices, that use ncm as a 
> +		transport for other protols.
> +		To compile this driver as a module, choose M here: the module will be 
> +		called huawei_cdc_ncm.
> +
>  config USB_NET_CDC_MBIM
>  	tristate "CDC MBIM support"
>  	depends on USB_USBNET
> diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
> index 9ab5c9d..12a0279 100644
> --- a/drivers/net/usb/Makefile
> +++ b/drivers/net/usb/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_USB_IPHETH)	+= ipheth.o
>  obj-$(CONFIG_USB_SIERRA_NET)	+= sierra_net.o
>  obj-$(CONFIG_USB_NET_CX82310_ETH)	+= cx82310_eth.o
>  obj-$(CONFIG_USB_NET_CDC_NCM)	+= cdc_ncm.o
> +obj-$(CONFIG_USB_NET_HUAWEI_CDC_NCM)	+= huawei_cdc_ncm.o
>  obj-$(CONFIG_USB_VL600)		+= lg-vl600.o
>  obj-$(CONFIG_USB_NET_QMI_WWAN)	+= qmi_wwan.o
>  obj-$(CONFIG_USB_NET_CDC_MBIM)	+= cdc_mbim.o


This patch must come last or you break the build here.  Every single
point in the git history must build to ensure bisectability.


Bjørn

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

* Re: [PATCH 1/3] Export cdc_ncm_{tx,rx}_fixup functions
  2013-07-02 20:10       ` [PATCH 1/3] Export cdc_ncm_{tx,rx}_fixup functions Enrico Mioso
  2013-07-02 20:13         ` [PATCH RFC 2/3] Introduce huawei_cdc_ncm driver Enrico Mioso
@ 2013-07-03  7:43         ` Bjørn Mork
  2013-07-03 10:19           ` Enrico Mioso
  1 sibling, 1 reply; 40+ messages in thread
From: Bjørn Mork @ 2013-07-03  7:43 UTC (permalink / raw)
  To: Enrico Mioso; +Cc: netdev

Enrico Mioso <mrkiko.rs@gmail.com> writes:

> Exports those functions to be able to re-use them in other drivers if needed.
>
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 493277e..22ed51c 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -885,6 +885,8 @@ error:
>  
>  	return NULL;
>  }
> +EXPORT_SYMBOL_GPL(cdc_ncm_tx_fixup);
> +
>  

Don't add spurious empty lines.



>  /* verify NTB header and return offset of first NDP, or negative error */
>  int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
> @@ -1040,6 +1042,7 @@ err_ndp:
>  error:
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(cdc_ncm_rx_fixup);
>  
>  static void
>  cdc_ncm_speed_change(struct cdc_ncm_ctx *ctx,
> diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
> index cc25b70..163244b 100644
> --- a/include/linux/usb/cdc_ncm.h
> +++ b/include/linux/usb/cdc_ncm.h
> @@ -133,3 +133,6 @@ extern void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
>  extern struct sk_buff *cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign);
>  extern int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in);
>  extern int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
> +struct sk_buff *
> +cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags);

I would have put this on a single line, even if it breaks the 80 character limit.

BTW, is breaking that limit more of a problem for you?  I can certainly
understand that you want to strictly obey it if it is..

> +int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in);

Both of these need "export" I believe...


Bjørn

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

* Re: [PATCH 0/3] make cdc_ncm_tx_fixup and cdc_ncm_rx_fixup non-static functions
  2013-07-02 20:06     ` [PATCH 0/3] make cdc_ncm_tx_fixup and cdc_ncm_rx_fixup non-static functions Enrico Mioso
  2013-07-02 20:10       ` [PATCH 1/3] Export cdc_ncm_{tx,rx}_fixup functions Enrico Mioso
@ 2013-07-03  7:45       ` Bjørn Mork
  1 sibling, 0 replies; 40+ messages in thread
From: Bjørn Mork @ 2013-07-03  7:45 UTC (permalink / raw)
  To: Enrico Mioso; +Cc: netdev

Enrico Mioso <mrkiko.rs@gmail.com> writes:

> Let cdc_ncm_tx_fixup and cdc_ncm_rx_fixup be non-static
>
> This patchs makes those functions non-static, so that they could be exported 
> for use by other drivers.
>
> Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
>
>
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 43afde8..493277e 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -858,7 +858,7 @@ static void cdc_ncm_txpath_bh(unsigned long param)
>  	}
>  }
>  
> -static struct sk_buff *
> +struct sk_buff *
>  cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
>  {
>  	struct sk_buff *skb_out;
> @@ -965,7 +965,7 @@ error:
>  }
>  EXPORT_SYMBOL_GPL(cdc_ncm_rx_verify_ndp16);
>  
> -static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
> +int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
>  {
>  	struct sk_buff *skb;
>  	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
>


This should be part of patch #1.  And there is no patch #0 in any case.
Part 0 is only for describing the whole series - motivation, design
choices, changes between versions etc.




Bjørn

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

* Re: Huaei E3131 - status
  2013-07-03  7:11               ` Huaei E3131 - status Enrico Mioso
@ 2013-07-03  8:15                 ` Bjørn Mork
  2013-07-03 12:55                   ` [PATCH V2 1/3] usbnet: cdc_ncm: let cdc_ncm_tx_fixup and cdc_ncm_rx_fixup be non-static Enrico Mioso
                                     ` (3 more replies)
  0 siblings, 4 replies; 40+ messages in thread
From: Bjørn Mork @ 2013-07-03  8:15 UTC (permalink / raw)
  To: Enrico Mioso; +Cc: netdev

Enrico Mioso <mrkiko.rs@gmail.com> writes:

> Ok ... As Bjorn stated, cdc-wdm is handling the notifications now - or, better: 
> is not handling them, as it is not made to do these things! The connection 
> stays up and the character device seems to work properly. Obviously cdc-wdm 
> notices me about one single unknown notifications.
> We're ignoring all the notifications from the NCM erspective, but all works 
> because the device probably doesn't rely on them so much.
> Aniway - now I'm conscious about why it works. Now it's time to improve the 
> situation of the driver, and might be the api. Waiting for suggestions and 
> injuries! :)

The only notification actually used for anything by cdc_ncm is
USB_CDC_NOTIFY_NETWORK_CONNECTION, which it uses to set the link up or
down. That functionality is disabled in your driver, leaving the link
always up.

This is of course not entirely correct if we apply a strict NCM
specification to this driver. But it does no harm either.  You have
added support for the embedded management channel, which must be used to
configure and connect the device.  Connection status will also be
reported here, and the managing userspace application (for example
ModemManager) will use this to pick up the actual network connection
state.  So the link state reported by NCM is redundant for these
devices.

The issue is that the few simple notifications standardized in CDC NCM
are sufficient for management of ethernet connections, but not for
3G/LTE connections.  That's why you need access to the additional vendor
specific management channel in the first place.  And when you have that
channel, then the additional NCM notifications are redundant at best.
Or confusing at worst.

The second notification implemented by cdc_ncm is
USB_CDC_NOTIFY_SPEED_CHANGE, which is shown in your logs.  But there is
nothing cdc_ncm can do with this, so it will just log it.  That's mostly
useless, both for wired and wireless devices.  3G/LTE management
applications will pick up the same information via the appropriate
management channel instead.

So the main reason why you should implement support for these
notifications eventually is not so much to handle them, but to silence
cdc-wdm. It will otherwise complain, which will confuse some users.  But
this is a really minor issue, and I see no reason why it should hold
back this driver.  It is perfectly usable as it is, and all necessary
features are implemented.


Bjørn

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

* Re: [PATCH RFC 3/3] huawei_cdc_ncm base skeleton
  2013-07-03  7:38             ` [PATCH RFC 3/3] huawei_cdc_ncm base skeleton Bjørn Mork
@ 2013-07-03 10:16               ` Enrico Mioso
  0 siblings, 0 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-03 10:16 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: netdev

First of all - thank you for all your comments and suggestions!




On Wed, 3 Jul 2013, Bj?rn Mork wrote:

==Date: Wed, 03 Jul 2013 09:38:40 +0200
==From: Bj?rn Mork <bjorn@mork.no>
==To: Enrico Mioso <mrkiko.rs@gmail.com>
==Cc: netdev@vger.kernel.org
==Subject: Re: [PATCH RFC 3/3] huawei_cdc_ncm base skeleton
==
==Enrico Mioso <mrkiko.rs@gmail.com> writes:
==
==> This is more an RFC than a patch.
==
==It looks pretty complete to me :)
==
==> + * This code is based on the original cdc_ncm implementation, that is:
==> + * Copyright (C) ST-Ericsson 2010-2012
==> + * Contact: Alexey Orishko <alexey.orishko@stericsson.com>
==> + * Original author: Hans Petter Selasky <hans.petter.selasky@stericsson.com>
==> + *
==> + * USB Host Driver for Network Control Model (NCM)
==> + * http://www.usb.org/developers/devclass_docs/NCM10.zip
==> + *
==> + * The NCM encoding, decoding and initialization logic
==> + * derives from FreeBSD 8.x. if_cdce.c and if_cdcereg.h
==> + *
==> + * This software is available to you under a choice of one of two
==> + * licenses. You may choose this file to be licensed under the terms
==> + * of the GNU General Public License (GPL) Version 2 or the 2-clause
==> + * BSD license listed below:
==
==This text does not match the MODULE_LICENSE you have used. You should
==probably take a few minutes to think about how you want your work to be
==licensed and use the appropriate combination of license comment and
==MODULE_LICENSE.
==
==You do in any case not need to copy all this from the cdc_ncm driver.
==You reuse code via the exported API, but the code in this driver is
==mostly your own.  A small note of cdc_ncm use is nice, but I don't think
==it's any more necessary than for other kernel code you use via #includes.
==
==

Thank you - right. I want my code to be GPL, no bsd license. Yes, I understand 
it're purely a personal choice, but wanted to be sincere.

==> +/* Bind our driver to needed interfaces. Heavily inspired by cdc_mbim.c one! */
==
==Many of your comments look mostly like notes for yourself.  Which they
==probably are, given that you sent this as a RFC :)
==
==You should go over all these comments and think about whether they are
==useful for others reading this code.  I don't think this one is, for
==example.
==
Yeah !! :) I'll remove them - they where here to help me understand the logic 
structure.

==> +static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev, struct usb_interface *intf) {
==> +
==> +	/* Some general use variables */
==> +	struct cdc_ncm_ctx *ctx;
==> +	struct usb_driver *subdriver = ERR_PTR(-ENODEV);
==> +	int ret = -ENODEV;
==> +	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
==> +	
==> +	/* Actually binds us to the device: use standard ncm function */
==> +	ret = cdc_ncm_bind_common(usbnet_dev, intf, 1); /* altsetting should be 1 */
==> +	
==> +	/* Did the ncm bind function fail? */
==> +	if (ret)
==> +		goto err;
==> +	
==> +	/* Let cdc data ctx pointer point to our state structure */
==> +	ctx = drvstate->ctx;
==> +	
==> +	if (usbnet_dev->status) 
==> +		subdriver = usb_cdc_wdm_register(ctx->control, &usbnet_dev->status->desc, 4096, huawei_cdc_ncm_wdm_manage_power); /* a smarter way to calculate that constant? */
==
==Yes, we were going to revisit that constant as soon as you discovered
==the protocol.  Now you found that it is AT commands, which doesn't
==really provide an absolute answer.  But AT commands is actually the
==normal usecase for cdc-wdm, so I believe using the defaults from that
==specification makes some sense.  As noted on the cdc-wdm driver:
==
==  CDC-WMC r1.1 requires wMaxCommand to be "at least 256 decimal (0x100)"
==
==Maybe 256 is enough here? Or are there Huawei specific AT commands with
==larger responses than that?  I don't know.  Choose some number and
==change the comment to explain the reasoning behind that choice.
==
==
I don't know - a word from Huawei would be very helpful in this context. 
Aniway, I would think there are commands with very large payloads and messages.
What you think if we set this to 512?

==> +	if (IS_ERR(subdriver)) {
==> +	 ret = PTR_ERR(subdriver);
==> +	 cdc_ncm_unbind(usbnet_dev, intf);
==> +   goto err;
==> +	}
==> +	
==> +	/* Prevent usbnet from using the status descriptor */
==> +	usbnet_dev->status = NULL;
==> +	
==> +	drvstate->subdriver = subdriver;
==> +	
==> +	/* FIXME: should we handle the vlan header in some way? */
==
==No, that is in cdc_mbim because it maps multiplexed MBIM IP sessions to
==ethernet VLAN interfaces.  The devices supported by this driver cannot
==support more than one IP session per USB function, so there is
==absolutely nothing you or the firmware can map a VLAN to.  Just drop the
==comment. 
==
Fine!
==> +	/* FIXME2: in my opinion we also could not do ARP, hoping somebody can help... */
==
==The firmware obviously does ARP as it works whether you disable it or
==not.  The usefulness can be discussed, but you cannot drop it without
==testing that it does in fact work.  The firmware most likely use either
==DHCP or ARP requests to figure out your MAC address, so the ARP requests
==might be required even if they look silly.
==
==
I will proceed with the test in some minutes!

==> +static const struct usb_device_id huawei_cdc_ncm_devs[] = {
==> +	/* The correct NCM handling will be implemented. For now, only my dongle
==> +		will be handled.
==> +		*/
==> +	{ USB_DEVICE_INTERFACE_NUMBER(0x12d1, 0x1506, 1), \
==> +		.driver_info = (unsigned long)&huawei_cdc_ncm_info,
==> +	},
==> +
==> +	{
==> +		/* Terminating entry */
==> +	},
==> +};
==
==
==As you note, that table needs to match on class codes.  Just move the
==Huawei vendor specific entries from cdc_ncm.
==
Here I'm a little bit confused - it seems there are device exporting QMI inside 
ncm, devices exporting AT and even devices exporting other unspecified 
protocol... or at least this was what I understood from google and various 
reports.
I will move all the entries - hoping not to cause regressions for cdc_ncm 
users!!

==
==
==Bj?rn
==
Thank you again for your help.

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

* Re: [PATCH 1/3] Export cdc_ncm_{tx,rx}_fixup functions
  2013-07-03  7:43         ` [PATCH 1/3] Export cdc_ncm_{tx,rx}_fixup functions Bjørn Mork
@ 2013-07-03 10:19           ` Enrico Mioso
  0 siblings, 0 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-03 10:19 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: Enrico Mioso, netdev

Thank you! I'll follow your suggestions.
Regarding the 80 chars limit - not, it's not strictly a problem, but since my 
braille device is 80 chars and I'm using a terminal 80x50 (80 colums, 50 rows), 
I felt more confortable with it. No problems, I'll add them on a single line.



On Wed, 3 Jul 2013, Bj?rn Mork wrote:

==Date: Wed, 03 Jul 2013 09:43:49 +0200
==From: Bj?rn Mork <bjorn@mork.no>
==To: Enrico Mioso <mrkiko.rs@gmail.com>
==Cc: netdev@vger.kernel.org
==Subject: Re: [PATCH 1/3] Export cdc_ncm_{tx,rx}_fixup functions
==
==Enrico Mioso <mrkiko.rs@gmail.com> writes:
==
==> Exports those functions to be able to re-use them in other drivers if needed.
==>
==> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
==> index 493277e..22ed51c 100644
==> --- a/drivers/net/usb/cdc_ncm.c
==> +++ b/drivers/net/usb/cdc_ncm.c
==> @@ -885,6 +885,8 @@ error:
==>  
==>  	return NULL;
==>  }
==> +EXPORT_SYMBOL_GPL(cdc_ncm_tx_fixup);
==> +
==>  
==
==Don't add spurious empty lines.
==
==
==
==>  /* verify NTB header and return offset of first NDP, or negative error */
==>  int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
==> @@ -1040,6 +1042,7 @@ err_ndp:
==>  error:
==>  	return 0;
==>  }
==> +EXPORT_SYMBOL_GPL(cdc_ncm_rx_fixup);
==>  
==>  static void
==>  cdc_ncm_speed_change(struct cdc_ncm_ctx *ctx,
==> diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
==> index cc25b70..163244b 100644
==> --- a/include/linux/usb/cdc_ncm.h
==> +++ b/include/linux/usb/cdc_ncm.h
==> @@ -133,3 +133,6 @@ extern void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
==>  extern struct sk_buff *cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign);
==>  extern int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in);
==>  extern int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
==> +struct sk_buff *
==> +cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags);
==
==I would have put this on a single line, even if it breaks the 80 character limit.
==
==BTW, is breaking that limit more of a problem for you?  I can certainly
==understand that you want to strictly obey it if it is..
==
==> +int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in);
==
==Both of these need "export" I believe...
==
==
==Bj?rn
==

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

* [PATCH V2 1/3] usbnet: cdc_ncm: let cdc_ncm_tx_fixup and cdc_ncm_rx_fixup be non-static
  2013-07-03  8:15                 ` Bjørn Mork
@ 2013-07-03 12:55                   ` Enrico Mioso
  2013-07-03 13:00                   ` [PATCH V2 2/3] usbnet: cdc_ncm: export cdc_ncm_tx_fixup and cdc_ncm_rx_fixup Enrico Mioso
                                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-03 12:55 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: netdev

Make those functions non-static so that they can be exported and re-used in 
other drivers.

Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>


diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 43afde8..493277e 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -858,7 +858,7 @@ static void cdc_ncm_txpath_bh(unsigned long param)
 	}
 }
 
-static struct sk_buff *
+struct sk_buff *
 cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
 {
 	struct sk_buff *skb_out;
@@ -965,7 +965,7 @@ error:
 }
 EXPORT_SYMBOL_GPL(cdc_ncm_rx_verify_ndp16);
 
-static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
+int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 {
 	struct sk_buff *skb;
 	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];

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

* [PATCH V2 2/3] usbnet: cdc_ncm: export cdc_ncm_tx_fixup and cdc_ncm_rx_fixup
  2013-07-03  8:15                 ` Bjørn Mork
  2013-07-03 12:55                   ` [PATCH V2 1/3] usbnet: cdc_ncm: let cdc_ncm_tx_fixup and cdc_ncm_rx_fixup be non-static Enrico Mioso
@ 2013-07-03 13:00                   ` Enrico Mioso
  2013-07-03 13:19                     ` Sergei Shtylyov
  2013-07-03 13:05                   ` [PATCH V2 3/3] usbnet: introduce " Enrico Mioso
  2013-07-03 13:13                   ` [PATCH] usbnet: cdc_ncm: remove huawei devices handled by cdc_ncm_huawei.c Enrico Mioso
  3 siblings, 1 reply; 40+ messages in thread
From: Enrico Mioso @ 2013-07-03 13:00 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: netdev

Export cdc_ncm_{tx,rx}_fixup functions, which may be re-used in drivers that 
implement ncm-like protocols.

Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
v2: removed spurious blank lines


diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 493277e..62686be 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -885,6 +885,7 @@ error:
 
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(cdc_ncm_tx_fixup);
 
 /* verify NTB header and return offset of first NDP, or negative error */
 int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
@@ -1040,6 +1041,7 @@ err_ndp:
 error:
 	return 0;
 }
+EXPORT_SYMBOL_GPL(cdc_ncm_rx_fixup);
 
 static void
 cdc_ncm_speed_change(struct cdc_ncm_ctx *ctx,
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index cc25b70..7a97cda 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -133,3 +133,5 @@ extern void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
 extern struct sk_buff *cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign);
 extern int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in);
 extern int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
+struct sk_buff *cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags);
+int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in);

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

* [PATCH V2 3/3] usbnet: introduce huawei_cdc_ncm driver
  2013-07-03  8:15                 ` Bjørn Mork
  2013-07-03 12:55                   ` [PATCH V2 1/3] usbnet: cdc_ncm: let cdc_ncm_tx_fixup and cdc_ncm_rx_fixup be non-static Enrico Mioso
  2013-07-03 13:00                   ` [PATCH V2 2/3] usbnet: cdc_ncm: export cdc_ncm_tx_fixup and cdc_ncm_rx_fixup Enrico Mioso
@ 2013-07-03 13:05                   ` Enrico Mioso
  2013-07-03 13:13                   ` [PATCH] usbnet: cdc_ncm: remove huawei devices handled by cdc_ncm_huawei.c Enrico Mioso
  3 siblings, 0 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-03 13:05 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: netdev

This new driver, handles devices that mimic the NCM standard, but using NCM as 
a transport layer to encapsulate other protocol (i.e. AT protocol).


Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
v2:
  - added check_connect handler
  - cleaned up some comments
  - proper device binding entries added

diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 287cc62..33cb175 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -246,6 +246,18 @@ config USB_NET_CDC_NCM
 	    * ST-Ericsson M343 HSPA Mobile Broadband Modem (reference design)
 	    * Ericsson F5521gw Mobile Broadband Module
 
+config USB_NET_HUAWEI_CDC_NCM
+	tristate "Huawei-style CDC NCM support"
+	depends on USB_USBNET
+	default y
+	select USB_WDM
+	select USB_NET_CDC_NCM
+	help
+		This driver aims to support huawei-style NCM devices, that use ncm as a 
+		transport for other protols.
+		To compile this driver as a module, choose M here: the module will be 
+		called huawei_cdc_ncm.
+
 config USB_NET_CDC_MBIM
 	tristate "CDC MBIM support"
 	depends on USB_USBNET
diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index 9ab5c9d..12a0279 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_USB_IPHETH)	+= ipheth.o
 obj-$(CONFIG_USB_SIERRA_NET)	+= sierra_net.o
 obj-$(CONFIG_USB_NET_CX82310_ETH)	+= cx82310_eth.o
 obj-$(CONFIG_USB_NET_CDC_NCM)	+= cdc_ncm.o
+obj-$(CONFIG_USB_NET_HUAWEI_CDC_NCM)	+= huawei_cdc_ncm.o
 obj-$(CONFIG_USB_VL600)		+= lg-vl600.o
 obj-$(CONFIG_USB_NET_QMI_WWAN)	+= qmi_wwan.o
 obj-$(CONFIG_USB_NET_CDC_MBIM)	+= cdc_mbim.o
diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c
index e69de29..46591e7 100644
--- a/drivers/net/usb/huawei_cdc_ncm.c
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -0,0 +1,206 @@
+/*
+ * huawei_cdc_ncm.c - handles hawei-style NCM devices.
+ * Copyright (C) 1012	 Enrico Mioso <mrkiko.rs@gmail.com>
+ * This driver handles devices resembling the CDC NCM standard, but 
+ * encapsulating another protocol inside it. An example are some Huawei 3G devices.
+ * This code has been heavily inspired by the cdc_mbim.c driver, which is
+ * Copyright (c) 2012  Smith Micro Software, Inc. 
+ * Copyright (c) 2012  Bj??rn Mork <bjorn@mork.no>
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/ethtool.h>
+#include <linux/if_vlan.h>
+#include <linux/ip.h>
+#include <linux/mii.h>
+#include <linux/usb.h>
+#include <linux/usb/cdc.h>
+#include <linux/usb/usbnet.h>
+#include <linux/usb/cdc-wdm.h>
+#include <linux/usb/cdc_ncm.h>
+
+/* Specific driver data */
+struct huawei_cdc_ncm_state {
+	struct cdc_ncm_ctx *ctx;
+	atomic_t pmcount;
+	struct usb_driver *subdriver;
+	struct usb_interface *control;
+	struct usb_interface *data;
+};
+
+static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
+{
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	int rv = 0;
+
+	if ((on && atomic_add_return(1, &drvstate->pmcount) == 1) || (!on && atomic_dec_and_test(&drvstate->pmcount))) {
+		rv = usb_autopm_get_interface(usbnet_dev->intf);
+		if (rv < 0)
+			goto err;
+		usbnet_dev->intf->needs_remote_wakeup = on;
+		usb_autopm_put_interface(usbnet_dev->intf);
+	}
+err:
+	return rv;
+}
+
+static int huawei_cdc_ncm_wdm_manage_power(struct usb_interface *intf, int status)
+{
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+
+	/* can be called while disconnecting */
+	if (!usbnet_dev)
+		return 0;
+
+	return huawei_cdc_ncm_manage_power(usbnet_dev, status);
+}
+
+
+static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev, struct usb_interface *intf) {
+	struct cdc_ncm_ctx *ctx;
+	struct usb_driver *subdriver = ERR_PTR(-ENODEV);
+	int ret = -ENODEV;
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	
+	/* Actually binds us to the device: use standard ncm function */
+	ret = cdc_ncm_bind_common(usbnet_dev, intf, 1); /* altsetting should be 1 */
+	if (ret)
+		goto err;
+	
+	ctx = drvstate->ctx;
+	
+	if (usbnet_dev->status) 
+	/* The wdm buffer size is choosen to match the one specified in AT R1.1 Standard. */	
+	subdriver = usb_cdc_wdm_register(ctx->control, &usbnet_dev->status->desc, 256, huawei_cdc_ncm_wdm_manage_power); /* CDC-WMC r1.1 requires wMaxCommand to be "at least 256" */
+	if (IS_ERR(subdriver)) {
+	 ret = PTR_ERR(subdriver);
+	 cdc_ncm_unbind(usbnet_dev, intf);
+   goto err;
+	}
+	
+	/* Prevent usbnet from using the status descriptor */
+	usbnet_dev->status = NULL;
+	
+	drvstate->subdriver = subdriver;
+	
+err:
+	return ret;
+}
+
+static void huawei_cdc_ncm_unbind(struct usbnet *usbnet_dev, struct usb_interface *intf) {
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+	
+	if (drvstate->subdriver && drvstate->subdriver->disconnect)
+		drvstate->subdriver->disconnect(ctx->control);
+	drvstate->subdriver = NULL;
+
+	cdc_ncm_unbind(usbnet_dev, intf);
+}
+
+static int huawei_cdc_ncm_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	int ret = 0;
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+
+	if (ctx == NULL) {
+		ret = -1;
+		goto error;
+	}
+
+	ret = usbnet_suspend(intf, message);
+	if (ret < 0)
+		goto error;
+
+	if (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->suspend)
+		ret = drvstate->subdriver->suspend(intf, message);
+	if (ret < 0)
+		usbnet_resume(intf);
+
+error:
+	return ret;
+}
+
+static int huawei_cdc_ncm_resume(struct usb_interface *intf)
+{
+	int ret = 0;
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+	bool callsub = (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->resume); /* should we call subdriver's resume? ? */
+
+	if (callsub)
+		ret = drvstate->subdriver->resume(intf);
+	if (ret < 0)
+		goto err;
+	ret = usbnet_resume(intf);
+	if (ret < 0 && callsub && drvstate->subdriver->suspend)
+		drvstate->subdriver->suspend(intf, PMSG_SUSPEND);
+err:
+	return ret;
+}
+
+static int huawei_cdc_ncm_check_connect(struct usbnet *usbnet_dev) {
+	struct cdc_ncm_ctx *ctx;
+	
+	ctx = (struct cdc_ncm_ctx *)usbnet_dev->data[0];
+	
+	if (ctx == NULL)
+		return 1; /* disconnected */
+	
+	return !ctx->connected;
+}
+
+static const struct driver_info huawei_cdc_ncm_info = {
+	.description = "Huawei CDC NCM",
+	.flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN,
+	.bind = huawei_cdc_ncm_bind,
+	.unbind = huawei_cdc_ncm_unbind,
+	.check_connect = huawei_cdc_ncm_check_connect,
+	.manage_power = huawei_cdc_ncm_manage_power,
+	.rx_fixup = cdc_ncm_rx_fixup,
+	.tx_fixup = cdc_ncm_tx_fixup,
+};
+
+static const struct usb_device_id huawei_cdc_ncm_devs[] = {
+	/* Huawei NCM devices disguised as vendor specific */
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x16),
+	  .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x46),
+	  .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x76),
+	  .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+
+	/* Terminating entry */
+	{
+	},
+};
+MODULE_DEVICE_TABLE(usb, huawei_cdc_ncm_devs);
+
+
+/* USB driver struct - used by USB stack */
+static struct usb_driver huawei_cdc_ncm_driver = {
+	.name = "huawei_cdc_ncm",
+	.id_table = huawei_cdc_ncm_devs,
+	.probe = usbnet_probe,
+	.disconnect = usbnet_disconnect,
+	.suspend = huawei_cdc_ncm_suspend,
+	.resume = huawei_cdc_ncm_resume,
+	.reset_resume = huawei_cdc_ncm_resume,
+	.supports_autosuspend = 1,
+	.disable_hub_initiated_lpm = 1,
+};
+module_usb_driver(huawei_cdc_ncm_driver);
+MODULE_AUTHOR("Enrico Mioso <mrkiko.rs@gmail.com>");
+MODULE_DESCRIPTION("USB CDC NCM host driver with encapsulated protocol support");
+MODULE_LICENSE("GPL");

  

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

* [PATCH] usbnet: cdc_ncm: remove huawei devices handled by cdc_ncm_huawei.c
  2013-07-03  8:15                 ` Bjørn Mork
                                     ` (2 preceding siblings ...)
  2013-07-03 13:05                   ` [PATCH V2 3/3] usbnet: introduce " Enrico Mioso
@ 2013-07-03 13:13                   ` Enrico Mioso
  3 siblings, 0 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-03 13:13 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: netdev

Some huawei devices implements an NCM-like protocol, but require applications 
to manage the link in a different mamner.
So another driver is needed to handle them.

Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 62686be..31f43f7 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1236,17 +1236,6 @@ static const struct usb_device_id cdc_devs[] = {
 	  .driver_info = (unsigned long)&wwan_info,
 	},
 
-	/* Huawei NCM devices disguised as vendor specific */
-	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x16),
-	  .driver_info = (unsigned long)&wwan_info,
-	},
-	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x46),
-	  .driver_info = (unsigned long)&wwan_info,
-	},
-	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x76),
-	  .driver_info = (unsigned long)&wwan_info,
-	},
-
 	/* Infineon(now Intel) HSPA Modem platform */
 	{ USB_DEVICE_AND_INTERFACE_INFO(0x1519, 0x0443,
 		USB_CLASS_COMM,




On Wed, 3 Jul 2013, Bj?rn Mork wrote:

==Date: Wed, 03 Jul 2013 10:15:32 +0200
==From: Bj?rn Mork <bjorn@mork.no>
==To: Enrico Mioso <mrkiko.rs@gmail.com>
==Cc: netdev@vger.kernel.org
==Subject: Re: Huaei E3131 - status
==
==Enrico Mioso <mrkiko.rs@gmail.com> writes:
==
==> Ok ... As Bjorn stated, cdc-wdm is handling the notifications now - or, better: 
==> is not handling them, as it is not made to do these things! The connection 
==> stays up and the character device seems to work properly. Obviously cdc-wdm 
==> notices me about one single unknown notifications.
==> We're ignoring all the notifications from the NCM erspective, but all works 
==> because the device probably doesn't rely on them so much.
==> Aniway - now I'm conscious about why it works. Now it's time to improve the 
==> situation of the driver, and might be the api. Waiting for suggestions and 
==> injuries! :)
==
==The only notification actually used for anything by cdc_ncm is
==USB_CDC_NOTIFY_NETWORK_CONNECTION, which it uses to set the link up or
==down. That functionality is disabled in your driver, leaving the link
==always up.
==
==This is of course not entirely correct if we apply a strict NCM
==specification to this driver. But it does no harm either.  You have
==added support for the embedded management channel, which must be used to
==configure and connect the device.  Connection status will also be
==reported here, and the managing userspace application (for example
==ModemManager) will use this to pick up the actual network connection
==state.  So the link state reported by NCM is redundant for these
==devices.
==
==The issue is that the few simple notifications standardized in CDC NCM
==are sufficient for management of ethernet connections, but not for
==3G/LTE connections.  That's why you need access to the additional vendor
==specific management channel in the first place.  And when you have that
==channel, then the additional NCM notifications are redundant at best.
==Or confusing at worst.
==
==The second notification implemented by cdc_ncm is
==USB_CDC_NOTIFY_SPEED_CHANGE, which is shown in your logs.  But there is
==nothing cdc_ncm can do with this, so it will just log it.  That's mostly
==useless, both for wired and wireless devices.  3G/LTE management
==applications will pick up the same information via the appropriate
==management channel instead.
==
==So the main reason why you should implement support for these
==notifications eventually is not so much to handle them, but to silence
==cdc-wdm. It will otherwise complain, which will confuse some users.  But
==this is a really minor issue, and I see no reason why it should hold
==back this driver.  It is perfectly usable as it is, and all necessary
==features are implemented.
==
==
==Bj?rn
==

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

* Re: [PATCH V2 2/3] usbnet: cdc_ncm: export cdc_ncm_tx_fixup and cdc_ncm_rx_fixup
  2013-07-03 13:00                   ` [PATCH V2 2/3] usbnet: cdc_ncm: export cdc_ncm_tx_fixup and cdc_ncm_rx_fixup Enrico Mioso
@ 2013-07-03 13:19                     ` Sergei Shtylyov
  2013-07-03 13:22                       ` Enrico Mioso
                                         ` (2 more replies)
  0 siblings, 3 replies; 40+ messages in thread
From: Sergei Shtylyov @ 2013-07-03 13:19 UTC (permalink / raw)
  To: Enrico Mioso; +Cc: Bjørn Mork, netdev

Hello.

On 03-07-2013 17:00, Enrico Mioso wrote:

> Export cdc_ncm_{tx,rx}_fixup functions, which may be re-used in drivers that
> implement ncm-like protocols.

> Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
> v2: removed spurious blank lines

    Patches #1 and #2 should actually be merged.

WBR, Sergei

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

* Re: [PATCH V2 2/3] usbnet: cdc_ncm: export cdc_ncm_tx_fixup and cdc_ncm_rx_fixup
  2013-07-03 13:19                     ` Sergei Shtylyov
@ 2013-07-03 13:22                       ` Enrico Mioso
  2013-07-03 13:33                       ` [PATCH V3 1/2] Export cdc_ncm_{tx,rx]_fixup functions Enrico Mioso
  2013-07-03 13:38                       ` [PATCH V3 2/2] Introduce huawei_cdc_ncm driver Enrico Mioso
  2 siblings, 0 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-03 13:22 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: Enrico Mioso, Bjørn Mork, netdev

Thank you very much Serghi! Applying..


On Wed, 3 Jul 2013, Sergei Shtylyov wrote:

==Date: Wed, 03 Jul 2013 17:19:13 +0400
==From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
==To: Enrico Mioso <mrkiko.rs@gmail.com>
==Cc: Bj?rn Mork <bjorn@mork.no>, netdev@vger.kernel.org
==Subject: Re: [PATCH V2 2/3] usbnet: cdc_ncm: export cdc_ncm_tx_fixup and
==    cdc_ncm_rx_fixup
==
==Hello.
==
==On 03-07-2013 17:00, Enrico Mioso wrote:
==
==> Export cdc_ncm_{tx,rx}_fixup functions, which may be re-used in drivers that
==> implement ncm-like protocols.
==
==> Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
==> v2: removed spurious blank lines
==
==   Patches #1 and #2 should actually be merged.
==
==WBR, Sergei
==
==

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

* [PATCH V3 1/2] Export cdc_ncm_{tx,rx]_fixup functions
  2013-07-03 13:19                     ` Sergei Shtylyov
  2013-07-03 13:22                       ` Enrico Mioso
@ 2013-07-03 13:33                       ` Enrico Mioso
  2013-07-03 13:38                       ` [PATCH V3 2/2] Introduce huawei_cdc_ncm driver Enrico Mioso
  2 siblings, 0 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-03 13:33 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: netdev

Some drivers implementing NCM-like protocols may re-use these functions. So 
export them.

Signed-off-by: enrico Mioso <mrkiko.rs@gmail.com>
v3: merged the two patches who make up this one

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 43afde8..62686be 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -858,7 +858,7 @@ static void cdc_ncm_txpath_bh(unsigned long param)
 	}
 }
 
-static struct sk_buff *
+struct sk_buff *
 cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
 {
 	struct sk_buff *skb_out;
@@ -885,6 +885,7 @@ error:
 
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(cdc_ncm_tx_fixup);
 
 /* verify NTB header and return offset of first NDP, or negative error */
 int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
@@ -965,7 +966,7 @@ error:
 }
 EXPORT_SYMBOL_GPL(cdc_ncm_rx_verify_ndp16);
 
-static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
+int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 {
 	struct sk_buff *skb;
 	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
@@ -1040,6 +1041,7 @@ err_ndp:
 error:
 	return 0;
 }
+EXPORT_SYMBOL_GPL(cdc_ncm_rx_fixup);
 
 static void
 cdc_ncm_speed_change(struct cdc_ncm_ctx *ctx,
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index cc25b70..7a97cda 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -133,3 +133,5 @@ extern void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
 extern struct sk_buff *cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign);
 extern int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in);
 extern int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
+struct sk_buff *cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags);
+int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in);

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

* [PATCH V3 2/2] Introduce huawei_cdc_ncm driver
  2013-07-03 13:19                     ` Sergei Shtylyov
  2013-07-03 13:22                       ` Enrico Mioso
  2013-07-03 13:33                       ` [PATCH V3 1/2] Export cdc_ncm_{tx,rx]_fixup functions Enrico Mioso
@ 2013-07-03 13:38                       ` Enrico Mioso
  2013-07-03 16:18                         ` Ben Hutchings
  2 siblings, 1 reply; 40+ messages in thread
From: Enrico Mioso @ 2013-07-03 13:38 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: netdev

This new driver, handles devices that mimic the NCM standard, but using NCM as
a transport layer to encapsulate other protocols (i.e. AT protocol).

Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
V3:
  - fixed typo in Kconfig (help text)

diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 287cc62..33cb175 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -246,6 +246,18 @@ config USB_NET_CDC_NCM
 	    * ST-Ericsson M343 HSPA Mobile Broadband Modem (reference design)
 	    * Ericsson F5521gw Mobile Broadband Module
 
+config USB_NET_HUAWEI_CDC_NCM
+	tristate "Huawei-style CDC NCM support"
+	depends on USB_USBNET
+	default y
+	select USB_WDM
+	select USB_NET_CDC_NCM
+	help
+		This driver aims to support huawei-style NCM devices, that use NCM as a 
+		transport for other protocols.
+		To compile this driver as a module, choose M here: the module will be 
+		called huawei_cdc_ncm.
+
 config USB_NET_CDC_MBIM
 	tristate "CDC MBIM support"
 	depends on USB_USBNET
diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index 9ab5c9d..12a0279 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_USB_IPHETH)	+= ipheth.o
 obj-$(CONFIG_USB_SIERRA_NET)	+= sierra_net.o
 obj-$(CONFIG_USB_NET_CX82310_ETH)	+= cx82310_eth.o
 obj-$(CONFIG_USB_NET_CDC_NCM)	+= cdc_ncm.o
+obj-$(CONFIG_USB_NET_HUAWEI_CDC_NCM)	+= huawei_cdc_ncm.o
 obj-$(CONFIG_USB_VL600)		+= lg-vl600.o
 obj-$(CONFIG_USB_NET_QMI_WWAN)	+= qmi_wwan.o
 obj-$(CONFIG_USB_NET_CDC_MBIM)	+= cdc_mbim.o
diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c
index e69de29..46591e7 100644
--- a/drivers/net/usb/huawei_cdc_ncm.c
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -0,0 +1,206 @@
+/*
+ * huawei_cdc_ncm.c - handles hawei-style NCM devices.
+ * Copyright (C) 1012	 Enrico Mioso <mrkiko.rs@gmail.com>
+ * This driver handles devices resembling the CDC NCM standard, but 
+ * encapsulating another protocol inside it. An example are some Huawei 3G devices.
+ * This code has been heavily inspired by the cdc_mbim.c driver, which is
+ * Copyright (c) 2012  Smith Micro Software, Inc. 
+ * Copyright (c) 2012  Bj??rn Mork <bjorn@mork.no>
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/ethtool.h>
+#include <linux/if_vlan.h>
+#include <linux/ip.h>
+#include <linux/mii.h>
+#include <linux/usb.h>
+#include <linux/usb/cdc.h>
+#include <linux/usb/usbnet.h>
+#include <linux/usb/cdc-wdm.h>
+#include <linux/usb/cdc_ncm.h>
+
+/* Specific driver data */
+struct huawei_cdc_ncm_state {
+	struct cdc_ncm_ctx *ctx;
+	atomic_t pmcount;
+	struct usb_driver *subdriver;
+	struct usb_interface *control;
+	struct usb_interface *data;
+};
+
+static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
+{
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	int rv = 0;
+
+	if ((on && atomic_add_return(1, &drvstate->pmcount) == 1) || (!on && atomic_dec_and_test(&drvstate->pmcount))) {
+		rv = usb_autopm_get_interface(usbnet_dev->intf);
+		if (rv < 0)
+			goto err;
+		usbnet_dev->intf->needs_remote_wakeup = on;
+		usb_autopm_put_interface(usbnet_dev->intf);
+	}
+err:
+	return rv;
+}
+
+static int huawei_cdc_ncm_wdm_manage_power(struct usb_interface *intf, int status)
+{
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+
+	/* can be called while disconnecting */
+	if (!usbnet_dev)
+		return 0;
+
+	return huawei_cdc_ncm_manage_power(usbnet_dev, status);
+}
+
+
+static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev, struct usb_interface *intf) {
+	struct cdc_ncm_ctx *ctx;
+	struct usb_driver *subdriver = ERR_PTR(-ENODEV);
+	int ret = -ENODEV;
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	
+	/* Actually binds us to the device: use standard ncm function */
+	ret = cdc_ncm_bind_common(usbnet_dev, intf, 1); /* altsetting should be 1 */
+	if (ret)
+		goto err;
+	
+	ctx = drvstate->ctx;
+	
+	if (usbnet_dev->status) 
+	/* The wdm buffer size is choosen to match the one specified in AT R1.1 Standard. */	
+	subdriver = usb_cdc_wdm_register(ctx->control, &usbnet_dev->status->desc, 256, huawei_cdc_ncm_wdm_manage_power); /* CDC-WMC r1.1 requires wMaxCommand to be "at least 256" */
+	if (IS_ERR(subdriver)) {
+	 ret = PTR_ERR(subdriver);
+	 cdc_ncm_unbind(usbnet_dev, intf);
+   goto err;
+	}
+	
+	/* Prevent usbnet from using the status descriptor */
+	usbnet_dev->status = NULL;
+	
+	drvstate->subdriver = subdriver;
+	
+err:
+	return ret;
+}
+
+static void huawei_cdc_ncm_unbind(struct usbnet *usbnet_dev, struct usb_interface *intf) {
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+	
+	if (drvstate->subdriver && drvstate->subdriver->disconnect)
+		drvstate->subdriver->disconnect(ctx->control);
+	drvstate->subdriver = NULL;
+
+	cdc_ncm_unbind(usbnet_dev, intf);
+}
+
+static int huawei_cdc_ncm_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	int ret = 0;
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+
+	if (ctx == NULL) {
+		ret = -1;
+		goto error;
+	}
+
+	ret = usbnet_suspend(intf, message);
+	if (ret < 0)
+		goto error;
+
+	if (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->suspend)
+		ret = drvstate->subdriver->suspend(intf, message);
+	if (ret < 0)
+		usbnet_resume(intf);
+
+error:
+	return ret;
+}
+
+static int huawei_cdc_ncm_resume(struct usb_interface *intf)
+{
+	int ret = 0;
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+	bool callsub = (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->resume); /* should we call subdriver's resume? ? */
+
+	if (callsub)
+		ret = drvstate->subdriver->resume(intf);
+	if (ret < 0)
+		goto err;
+	ret = usbnet_resume(intf);
+	if (ret < 0 && callsub && drvstate->subdriver->suspend)
+		drvstate->subdriver->suspend(intf, PMSG_SUSPEND);
+err:
+	return ret;
+}
+
+static int huawei_cdc_ncm_check_connect(struct usbnet *usbnet_dev) {
+	struct cdc_ncm_ctx *ctx;
+	
+	ctx = (struct cdc_ncm_ctx *)usbnet_dev->data[0];
+	
+	if (ctx == NULL)
+		return 1; /* disconnected */
+	
+	return !ctx->connected;
+}
+
+static const struct driver_info huawei_cdc_ncm_info = {
+	.description = "Huawei CDC NCM",
+	.flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN,
+	.bind = huawei_cdc_ncm_bind,
+	.unbind = huawei_cdc_ncm_unbind,
+	.check_connect = huawei_cdc_ncm_check_connect,
+	.manage_power = huawei_cdc_ncm_manage_power,
+	.rx_fixup = cdc_ncm_rx_fixup,
+	.tx_fixup = cdc_ncm_tx_fixup,
+};
+
+static const struct usb_device_id huawei_cdc_ncm_devs[] = {
+	/* Huawei NCM devices disguised as vendor specific */
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x16),
+	  .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x46),
+	  .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x76),
+	  .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+
+	/* Terminating entry */
+	{
+	},
+};
+MODULE_DEVICE_TABLE(usb, huawei_cdc_ncm_devs);
+
+
+/* USB driver struct - used by USB stack */
+static struct usb_driver huawei_cdc_ncm_driver = {
+	.name = "huawei_cdc_ncm",
+	.id_table = huawei_cdc_ncm_devs,
+	.probe = usbnet_probe,
+	.disconnect = usbnet_disconnect,
+	.suspend = huawei_cdc_ncm_suspend,
+	.resume = huawei_cdc_ncm_resume,
+	.reset_resume = huawei_cdc_ncm_resume,
+	.supports_autosuspend = 1,
+	.disable_hub_initiated_lpm = 1,
+};
+module_usb_driver(huawei_cdc_ncm_driver);
+MODULE_AUTHOR("Enrico Mioso <mrkiko.rs@gmail.com>");
+MODULE_DESCRIPTION("USB CDC NCM host driver with encapsulated protocol support");
+MODULE_LICENSE("GPL");

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

* Re: [PATCH V3 2/2] Introduce huawei_cdc_ncm driver
  2013-07-03 13:38                       ` [PATCH V3 2/2] Introduce huawei_cdc_ncm driver Enrico Mioso
@ 2013-07-03 16:18                         ` Ben Hutchings
  2013-07-03 18:11                           ` [PATCH V3 2/2 RESEND] " Enrico Mioso
  2013-07-03 18:11                           ` [PATCH V3 2/2] " Enrico Mioso
  0 siblings, 2 replies; 40+ messages in thread
From: Ben Hutchings @ 2013-07-03 16:18 UTC (permalink / raw)
  To: Enrico Mioso; +Cc: Sergei Shtylyov, netdev

On Wed, 2013-07-03 at 15:38 +0200, Enrico Mioso wrote:
> This new driver, handles devices that mimic the NCM standard, but using NCM as
> a transport layer to encapsulate other protocols (i.e. AT protocol).
> 
> Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>

Add a '---' separator here, between the commit message and the list of
changes that doesn't need to go in the commit message.

> V3:
>   - fixed typo in Kconfig (help text)
>
> diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
> index 287cc62..33cb175 100644
> --- a/drivers/net/usb/Kconfig
> +++ b/drivers/net/usb/Kconfig
> @@ -246,6 +246,18 @@ config USB_NET_CDC_NCM
>  	    * ST-Ericsson M343 HSPA Mobile Broadband Modem (reference design)
>  	    * Ericsson F5521gw Mobile Broadband Module
>  
> +config USB_NET_HUAWEI_CDC_NCM
> +	tristate "Huawei-style CDC NCM support"
> +	depends on USB_USBNET
> +	default y
> +	select USB_WDM
> +	select USB_NET_CDC_NCM
[...]

The new config symbol should either depend on USB_NET_CDC_NCM (rather
than selecting it), or have no default setting.  It is not correct to
enable it by default just because USB_USBNET is selected.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

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

* [PATCH V3 2/2 RESEND] Introduce huawei_cdc_ncm driver
  2013-07-03 16:18                         ` Ben Hutchings
@ 2013-07-03 18:11                           ` Enrico Mioso
  2013-07-04 13:19                             ` Bjørn Mork
  2013-07-03 18:11                           ` [PATCH V3 2/2] " Enrico Mioso
  1 sibling, 1 reply; 40+ messages in thread
From: Enrico Mioso @ 2013-07-03 18:11 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev

This new driver, handles devices that mimic the NCM standard, but using NCM as
a transport layer to encapsulate other protocols (i.e. AT protocol).

Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
---
V3:
  - fixed typo in Kconfig (help text)

diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 287cc62..33cb175 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -246,6 +246,18 @@ config USB_NET_CDC_NCM
 	    * ST-Ericsson M343 HSPA Mobile Broadband Modem (reference design)
 	    * Ericsson F5521gw Mobile Broadband Module
 
+config USB_NET_HUAWEI_CDC_NCM
+	tristate "Huawei-style CDC NCM support"
+	depends on USB_USBNET
+	select USB_WDM
+	select USB_NET_CDC_NCM
+	help
+		This driver aims to support huawei-style NCM devices, that use ncm as a 
+		transport for other protocols.
+		To compile this driver as a module, choose M here: the module will be 
+		called huawei_cdc_ncm.
+
 config USB_NET_CDC_MBIM
 	tristate "CDC MBIM support"
 	depends on USB_USBNET
diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index 9ab5c9d..12a0279 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_USB_IPHETH)	+= ipheth.o
 obj-$(CONFIG_USB_SIERRA_NET)	+= sierra_net.o
 obj-$(CONFIG_USB_NET_CX82310_ETH)	+= cx82310_eth.o
 obj-$(CONFIG_USB_NET_CDC_NCM)	+= cdc_ncm.o
+obj-$(CONFIG_USB_NET_HUAWEI_CDC_NCM)	+= huawei_cdc_ncm.o
 obj-$(CONFIG_USB_VL600)		+= lg-vl600.o
 obj-$(CONFIG_USB_NET_QMI_WWAN)	+= qmi_wwan.o
 obj-$(CONFIG_USB_NET_CDC_MBIM)	+= cdc_mbim.o
diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c
index e69de29..46591e7 100644
--- a/drivers/net/usb/huawei_cdc_ncm.c
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -0,0 +1,206 @@
+/*
+ * huawei_cdc_ncm.c - handles hawei-style NCM devices.
+ * Copyright (C) 1012	 Enrico Mioso <mrkiko.rs@gmail.com>
+ * This driver handles devices resembling the CDC NCM standard, but 
+ * encapsulating another protocol inside it. An example are some Huawei 3G devices.
+ * This code has been heavily inspired by the cdc_mbim.c driver, which is
+ * Copyright (c) 2012  Smith Micro Software, Inc. 
+ * Copyright (c) 2012  Bj??rn Mork <bjorn@mork.no>
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/ethtool.h>
+#include <linux/if_vlan.h>
+#include <linux/ip.h>
+#include <linux/mii.h>
+#include <linux/usb.h>
+#include <linux/usb/cdc.h>
+#include <linux/usb/usbnet.h>
+#include <linux/usb/cdc-wdm.h>
+#include <linux/usb/cdc_ncm.h>
+
+/* Specific driver data */
+struct huawei_cdc_ncm_state {
+	struct cdc_ncm_ctx *ctx;
+	atomic_t pmcount;
+	struct usb_driver *subdriver;
+	struct usb_interface *control;
+	struct usb_interface *data;
+};
+
+static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
+{
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	int rv = 0;
+
+	if ((on && atomic_add_return(1, &drvstate->pmcount) == 1) || (!on && atomic_dec_and_test(&drvstate->pmcount))) {
+		rv = usb_autopm_get_interface(usbnet_dev->intf);
+		if (rv < 0)
+			goto err;
+		usbnet_dev->intf->needs_remote_wakeup = on;
+		usb_autopm_put_interface(usbnet_dev->intf);
+	}
+err:
+	return rv;
+}
+
+static int huawei_cdc_ncm_wdm_manage_power(struct usb_interface *intf, int status)
+{
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+
+	/* can be called while disconnecting */
+	if (!usbnet_dev)
+		return 0;
+
+	return huawei_cdc_ncm_manage_power(usbnet_dev, status);
+}
+
+
+static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev, struct usb_interface *intf) {
+	struct cdc_ncm_ctx *ctx;
+	struct usb_driver *subdriver = ERR_PTR(-ENODEV);
+	int ret = -ENODEV;
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	
+	/* Actually binds us to the device: use standard ncm function */
+	ret = cdc_ncm_bind_common(usbnet_dev, intf, 1); /* altsetting should be 1 */
+	if (ret)
+		goto err;
+	
+	ctx = drvstate->ctx;
+	
+	if (usbnet_dev->status) 
+	/* The wdm buffer size is choosen to match the one specified in AT R1.1 Standard. */	
+	subdriver = usb_cdc_wdm_register(ctx->control, &usbnet_dev->status->desc, 256, huawei_cdc_ncm_wdm_manage_power); /* CDC-WMC r1.1 requires wMaxCommand to be "at least 256" */
+	if (IS_ERR(subdriver)) {
+	 ret = PTR_ERR(subdriver);
+	 cdc_ncm_unbind(usbnet_dev, intf);
+   goto err;
+	}
+	
+	/* Prevent usbnet from using the status descriptor */
+	usbnet_dev->status = NULL;
+	
+	drvstate->subdriver = subdriver;
+	
+err:
+	return ret;
+}
+
+static void huawei_cdc_ncm_unbind(struct usbnet *usbnet_dev, struct usb_interface *intf) {
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+	
+	if (drvstate->subdriver && drvstate->subdriver->disconnect)
+		drvstate->subdriver->disconnect(ctx->control);
+	drvstate->subdriver = NULL;
+
+	cdc_ncm_unbind(usbnet_dev, intf);
+}
+
+static int huawei_cdc_ncm_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	int ret = 0;
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+
+	if (ctx == NULL) {
+		ret = -1;
+		goto error;
+	}
+
+	ret = usbnet_suspend(intf, message);
+	if (ret < 0)
+		goto error;
+
+	if (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->suspend)
+		ret = drvstate->subdriver->suspend(intf, message);
+	if (ret < 0)
+		usbnet_resume(intf);
+
+error:
+	return ret;
+}
+
+static int huawei_cdc_ncm_resume(struct usb_interface *intf)
+{
+	int ret = 0;
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+	bool callsub = (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->resume); /* should we call subdriver's resume? ? */
+
+	if (callsub)
+		ret = drvstate->subdriver->resume(intf);
+	if (ret < 0)
+		goto err;
+	ret = usbnet_resume(intf);
+	if (ret < 0 && callsub && drvstate->subdriver->suspend)
+		drvstate->subdriver->suspend(intf, PMSG_SUSPEND);
+err:
+	return ret;
+}
+
+static int huawei_cdc_ncm_check_connect(struct usbnet *usbnet_dev) {
+	struct cdc_ncm_ctx *ctx;
+	
+	ctx = (struct cdc_ncm_ctx *)usbnet_dev->data[0];
+	
+	if (ctx == NULL)
+		return 1; /* disconnected */
+	
+	return !ctx->connected;
+}
+
+static const struct driver_info huawei_cdc_ncm_info = {
+	.description = "Huawei CDC NCM",
+	.flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN,
+	.bind = huawei_cdc_ncm_bind,
+	.unbind = huawei_cdc_ncm_unbind,
+	.check_connect = huawei_cdc_ncm_check_connect,
+	.manage_power = huawei_cdc_ncm_manage_power,
+	.rx_fixup = cdc_ncm_rx_fixup,
+	.tx_fixup = cdc_ncm_tx_fixup,
+};
+
+static const struct usb_device_id huawei_cdc_ncm_devs[] = {
+	/* Huawei NCM devices disguised as vendor specific */
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x16),
+	  .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x46),
+	  .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x76),
+	  .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+
+	/* Terminating entry */
+	{
+	},
+};
+MODULE_DEVICE_TABLE(usb, huawei_cdc_ncm_devs);
+
+
+/* USB driver struct - used by USB stack */
+static struct usb_driver huawei_cdc_ncm_driver = {
+	.name = "huawei_cdc_ncm",
+	.id_table = huawei_cdc_ncm_devs,
+	.probe = usbnet_probe,
+	.disconnect = usbnet_disconnect,
+	.suspend = huawei_cdc_ncm_suspend,
+	.resume = huawei_cdc_ncm_resume,
+	.reset_resume = huawei_cdc_ncm_resume,
+	.supports_autosuspend = 1,
+	.disable_hub_initiated_lpm = 1,
+};
+module_usb_driver(huawei_cdc_ncm_driver);
+MODULE_AUTHOR("Enrico Mioso <mrkiko.rs@gmail.com>");
+MODULE_DESCRIPTION("USB CDC NCM host driver with encapsulated protocol support");
+MODULE_LICENSE("GPL");

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

* Re: [PATCH V3 2/2] Introduce huawei_cdc_ncm driver
  2013-07-03 16:18                         ` Ben Hutchings
  2013-07-03 18:11                           ` [PATCH V3 2/2 RESEND] " Enrico Mioso
@ 2013-07-03 18:11                           ` Enrico Mioso
  1 sibling, 0 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-03 18:11 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Enrico Mioso, Sergei Shtylyov, netdev

Thank you Ben !! :) Applying...


On Wed, 3 Jul 2013, Ben Hutchings wrote:

==Date: Wed, 3 Jul 2013 17:18:53 +0100
==From: Ben Hutchings <bhutchings@solarflare.com>
==To: Enrico Mioso <mrkiko.rs@gmail.com>
==Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>,
==    netdev@vger.kernel.org
==Subject: Re: [PATCH V3 2/2] Introduce huawei_cdc_ncm driver
==
==On Wed, 2013-07-03 at 15:38 +0200, Enrico Mioso wrote:
==> This new driver, handles devices that mimic the NCM standard, but using NCM as
==> a transport layer to encapsulate other protocols (i.e. AT protocol).
==> 
==> Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
==
==Add a '---' separator here, between the commit message and the list of
==changes that doesn't need to go in the commit message.
==
==> V3:
==>   - fixed typo in Kconfig (help text)
==>
==> diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
==> index 287cc62..33cb175 100644
==> --- a/drivers/net/usb/Kconfig
==> +++ b/drivers/net/usb/Kconfig
==> @@ -246,6 +246,18 @@ config USB_NET_CDC_NCM
==>  	    * ST-Ericsson M343 HSPA Mobile Broadband Modem (reference design)
==>  	    * Ericsson F5521gw Mobile Broadband Module
==>  
==> +config USB_NET_HUAWEI_CDC_NCM
==> +	tristate "Huawei-style CDC NCM support"
==> +	depends on USB_USBNET
==> +	default y
==> +	select USB_WDM
==> +	select USB_NET_CDC_NCM
==[...]
==
==The new config symbol should either depend on USB_NET_CDC_NCM (rather
==than selecting it), or have no default setting.  It is not correct to
==enable it by default just because USB_USBNET is selected.
==
==Ben.
==
==-- 
==Ben Hutchings, Staff Engineer, Solarflare
==Not speaking for my employer; that's the marketing department's job.
==They asked us to note that Solarflare product names are trademarked.
==
==

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

* Re: [PATCH V3 2/2 RESEND] Introduce huawei_cdc_ncm driver
  2013-07-03 18:11                           ` [PATCH V3 2/2 RESEND] " Enrico Mioso
@ 2013-07-04 13:19                             ` Bjørn Mork
  2013-07-04 17:03                               ` Enrico Mioso
  0 siblings, 1 reply; 40+ messages in thread
From: Bjørn Mork @ 2013-07-04 13:19 UTC (permalink / raw)
  To: Enrico Mioso; +Cc: Ben Hutchings, netdev

Enrico Mioso <mrkiko.rs@gmail.com> writes:

> This new driver, handles devices that mimic the NCM standard, but using NCM as
> a transport layer to encapsulate other protocols (i.e. AT protocol).
>
> Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
> ---
> V3:
>   - fixed typo in Kconfig (help text)
>
> diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
> index 287cc62..33cb175 100644
> --- a/drivers/net/usb/Kconfig
> +++ b/drivers/net/usb/Kconfig
> @@ -246,6 +246,18 @@ config USB_NET_CDC_NCM
>  	    * ST-Ericsson M343 HSPA Mobile Broadband Modem (reference design)
>  	    * Ericsson F5521gw Mobile Broadband Module
>  
> +config USB_NET_HUAWEI_CDC_NCM
> +	tristate "Huawei-style CDC NCM support"
> +	depends on USB_USBNET
> +	select USB_WDM
> +	select USB_NET_CDC_NCM
> +	help
> +		This driver aims to support huawei-style NCM devices, that use ncm as a 
> +		transport for other protocols.
> +		To compile this driver as a module, choose M here: the module will be 
> +		called huawei_cdc_ncm.
> +
>  config USB_NET_CDC_MBIM
>  	tristate "CDC MBIM support"
>  	depends on USB_USBNET

Sorry, but editing a patch directly is too risky.  You shouldn't do
that.  This does not apply anymore because there now is a line missing
here...

This is nitpicking, but it needs to be correct for the patches to be
accepted.

Before resubmitting, I suggest:

 - check all the patches with scripts/checkpatch.pl
 - fix absolutely all ERRORs.  You may ignore some WARNINGs if there are
   good reasons to do so (e.g the 80 character limit)
 - mail all patches to yourself, and save as you receive them
 - apply the received patches to a clean net-next based branch using
   "git am <patch>"
 - verify the result (build it after each patch is applied and play with
   related config options to make sure nothing breaks)

Also please note that net-next is currently closed due to the merge
window being open: http://www.spinics.net/lists/netdev/msg242161.html

I don't know exactly what the policy wrt new drivers is, but you can
assume that David is pretty busy with the stuff he already has queued.
So it is better to hold this work until after v3.11-rc1 is out and David
announces that he has opened net-next again.

And I also believe you should include your final patch to cdc_ncm.c as
part 3/3 of this series.  Otherwise the ordering and dependencies are
not clear.  That patch can be applied on it's own, but it won't make any
sense to do so.  Better make that clear.



Bjørn

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

* Re: [PATCH V3 2/2 RESEND] Introduce huawei_cdc_ncm driver
  2013-07-04 13:19                             ` Bjørn Mork
@ 2013-07-04 17:03                               ` Enrico Mioso
  0 siblings, 0 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-04 17:03 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: Enrico Mioso, Ben Hutchings, netdev

thank you !! you're right.
Enthusiasm took me! :)
I'll re-read SubmittingPatches and prepare some scripts (if they do not already 
exist).

:)

On Thu, 4 Jul 2013, Bj?rn Mork wrote:

==Date: Thu, 04 Jul 2013 15:19:46 +0200
==From: Bj?rn Mork <bjorn@mork.no>
==To: Enrico Mioso <mrkiko.rs@gmail.com>
==Cc: Ben Hutchings <bhutchings@solarflare.com>, netdev@vger.kernel.org
==Subject: Re: [PATCH V3 2/2 RESEND] Introduce huawei_cdc_ncm driver
==
==Enrico Mioso <mrkiko.rs@gmail.com> writes:
==
==> This new driver, handles devices that mimic the NCM standard, but using NCM as
==> a transport layer to encapsulate other protocols (i.e. AT protocol).
==>
==> Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
==> ---
==> V3:
==>   - fixed typo in Kconfig (help text)
==>
==> diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
==> index 287cc62..33cb175 100644
==> --- a/drivers/net/usb/Kconfig
==> +++ b/drivers/net/usb/Kconfig
==> @@ -246,6 +246,18 @@ config USB_NET_CDC_NCM
==>  	    * ST-Ericsson M343 HSPA Mobile Broadband Modem (reference design)
==>  	    * Ericsson F5521gw Mobile Broadband Module
==>  
==> +config USB_NET_HUAWEI_CDC_NCM
==> +	tristate "Huawei-style CDC NCM support"
==> +	depends on USB_USBNET
==> +	select USB_WDM
==> +	select USB_NET_CDC_NCM
==> +	help
==> +		This driver aims to support huawei-style NCM devices, that use ncm as a 
==> +		transport for other protocols.
==> +		To compile this driver as a module, choose M here: the module will be 
==> +		called huawei_cdc_ncm.
==> +
==>  config USB_NET_CDC_MBIM
==>  	tristate "CDC MBIM support"
==>  	depends on USB_USBNET
==
==Sorry, but editing a patch directly is too risky.  You shouldn't do
==that.  This does not apply anymore because there now is a line missing
==here...
==
==This is nitpicking, but it needs to be correct for the patches to be
==accepted.
==
==Before resubmitting, I suggest:
==
== - check all the patches with scripts/checkpatch.pl
== - fix absolutely all ERRORs.  You may ignore some WARNINGs if there are
==   good reasons to do so (e.g the 80 character limit)
== - mail all patches to yourself, and save as you receive them
== - apply the received patches to a clean net-next based branch using
==   "git am <patch>"
== - verify the result (build it after each patch is applied and play with
==   related config options to make sure nothing breaks)
==
==Also please note that net-next is currently closed due to the merge
==window being open: http://www.spinics.net/lists/netdev/msg242161.html
==
==I don't know exactly what the policy wrt new drivers is, but you can
==assume that David is pretty busy with the stuff he already has queued.
==So it is better to hold this work until after v3.11-rc1 is out and David
==announces that he has opened net-next again.
==
==And I also believe you should include your final patch to cdc_ncm.c as
==part 3/3 of this series.  Otherwise the ordering and dependencies are
==not clear.  That patch can be applied on it's own, but it won't make any
==sense to do so.  Better make that clear.
==
==
==
==Bj?rn
==

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

* [PATCH 1/3] net: cdc_ncm: Export cdc_ncm_{tx,rx}_fixup functions for re-use
  2013-07-02 12:01 ` Bjørn Mork
  2013-07-02 12:05   ` Enrico Mioso
  2013-07-02 17:04   ` [RFC] huawei_cdc_ncm driver - status Enrico Mioso
@ 2013-07-10 12:27   ` Enrico Mioso
       [not found]     ` <0cbf7c81-182e-493c-9c28-5b78160a08f1@email.android.com>
  2013-07-10 12:28   ` [PATCH 2/3] net: huawei_cdc_ncm: Introduce the " Enrico Mioso
  2013-07-10 12:30   ` [PATCH 3/3] net: cdc_ncm: remove Huawei non-standard NCM device IDs Enrico Mioso
  4 siblings, 1 reply; 40+ messages in thread
From: Enrico Mioso @ 2013-07-10 12:27 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: netdev

Some drivers implementing NCM-like protocols, may re-use those functions, as is 
the case in the huawei_cdc_ncm driver.
Export them via EXPORT_SYMBOL_GPL.

Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
---
 drivers/net/usb/cdc_ncm.c   |    6 ++++--
 include/linux/usb/cdc_ncm.h |    3 +++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 43afde8..62686be 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -858,7 +858,7 @@ static void cdc_ncm_txpath_bh(unsigned long param)
 	}
 }
 
-static struct sk_buff *
+struct sk_buff *
 cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
 {
 	struct sk_buff *skb_out;
@@ -885,6 +885,7 @@ error:
 
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(cdc_ncm_tx_fixup);
 
 /* verify NTB header and return offset of first NDP, or negative error */
 int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
@@ -965,7 +966,7 @@ error:
 }
 EXPORT_SYMBOL_GPL(cdc_ncm_rx_verify_ndp16);
 
-static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
+int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 {
 	struct sk_buff *skb;
 	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
@@ -1040,6 +1041,7 @@ err_ndp:
 error:
 	return 0;
 }
+EXPORT_SYMBOL_GPL(cdc_ncm_rx_fixup);
 
 static void
 cdc_ncm_speed_change(struct cdc_ncm_ctx *ctx,
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index cc25b70..163244b 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -133,3 +133,6 @@ extern void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
 extern struct sk_buff *cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign);
 extern int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in);
 extern int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
+struct sk_buff *
+cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags);
+int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in);

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

* [PATCH 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver
  2013-07-02 12:01 ` Bjørn Mork
                     ` (2 preceding siblings ...)
  2013-07-10 12:27   ` [PATCH 1/3] net: cdc_ncm: Export cdc_ncm_{tx,rx}_fixup functions for re-use Enrico Mioso
@ 2013-07-10 12:28   ` Enrico Mioso
  2013-07-10 12:30   ` [PATCH 3/3] net: cdc_ncm: remove Huawei non-standard NCM device IDs Enrico Mioso
  4 siblings, 0 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-10 12:28 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: netdev

This driver supports devices using the NCM protocol as an encapsulation layer
for other protocols, like the E3131 Huawei 3G modem.

Suggested-by: Bjorn Mork <bjorn@mork.no>
Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
---

 drivers/net/usb/Kconfig          |   11 ++
 drivers/net/usb/Makefile         |    1 +
 drivers/net/usb/huawei_cdc_ncm.c |  210 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 222 insertions(+)
diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index d84bfd4..6e56751 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -242,6 +242,17 @@ config USB_NET_CDC_NCM
 	    * ST-Ericsson M343 HSPA Mobile Broadband Modem (reference design)
 	    * Ericsson F5521gw Mobile Broadband Module
 
+config USB_NET_HUAWEI_CDC_NCM
+	tristate "Huawei-style CDC NCM support"
+	depends on USB_USBNET
+	select USB_WDM
+	select USB_NET_CDC_NCM
+	help
+		This driver aims to support huawei-style NCM devices, that use ncm as a
+		transport for other protocols.
+		To compile this driver as a module, choose M here: the module will be
+		called huawei_cdc_ncm.
+
 config USB_NET_CDC_MBIM
 	tristate "CDC MBIM support"
 	depends on USB_USBNET
diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index 9ab5c9d..12a0279 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_USB_IPHETH)	+= ipheth.o
 obj-$(CONFIG_USB_SIERRA_NET)	+= sierra_net.o
 obj-$(CONFIG_USB_NET_CX82310_ETH)	+= cx82310_eth.o
 obj-$(CONFIG_USB_NET_CDC_NCM)	+= cdc_ncm.o
+obj-$(CONFIG_USB_NET_HUAWEI_CDC_NCM)	+= huawei_cdc_ncm.o
 obj-$(CONFIG_USB_VL600)		+= lg-vl600.o
 obj-$(CONFIG_USB_NET_QMI_WWAN)	+= qmi_wwan.o
 obj-$(CONFIG_USB_NET_CDC_MBIM)	+= cdc_mbim.o
diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c
index e69de29..d3426b9 100644
--- a/drivers/net/usb/huawei_cdc_ncm.c
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -0,0 +1,210 @@
+/*
+ * huawei_cdc_ncm.c - handles huawei-style NCM devices.
+ * Copyright (C) 2013	 Enrico Mioso <mrkiko.rs@gmail.com>
+ * This driver handles devices resembling the CDC NCM standard, but
+ * encapsulating another protocol inside it. An example are some Huawei 3G
+ * devices, exposing an embedded AT channel where you can set up the NCM
+ * connection.
+ * This code has been heavily inspired by the cdc_mbim.c driver, which is
+ * Copyright (c) 2012  Smith Micro Software, Inc.
+ * Copyright (c) 2012  Bj??rn Mork <bjorn@mork.no>
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/ethtool.h>
+#include <linux/if_vlan.h>
+#include <linux/ip.h>
+#include <linux/mii.h>
+#include <linux/usb.h>
+#include <linux/usb/cdc.h>
+#include <linux/usb/usbnet.h>
+#include <linux/usb/cdc-wdm.h>
+#include <linux/usb/cdc_ncm.h>
+
+/* Driver data */
+struct huawei_cdc_ncm_state {
+	struct cdc_ncm_ctx *ctx;
+	atomic_t pmcount;
+	struct usb_driver *subdriver;
+	struct usb_interface *control;
+	struct usb_interface *data;
+};
+
+static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
+{
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	int rv = 0;
+
+	if ((on && atomic_add_return(1, &drvstate->pmcount) == 1) || (!on && atomic_dec_and_test(&drvstate->pmcount))) {
+		rv = usb_autopm_get_interface(usbnet_dev->intf);
+		if (rv < 0)
+			goto err;
+		usbnet_dev->intf->needs_remote_wakeup = on;
+		usb_autopm_put_interface(usbnet_dev->intf);
+	}
+err:
+	return rv;
+}
+
+static int huawei_cdc_ncm_wdm_manage_power(struct usb_interface *intf, int status)
+{
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+
+	/* can be called while disconnecting */
+	if (!usbnet_dev)
+		return 0;
+
+	return huawei_cdc_ncm_manage_power(usbnet_dev, status);
+}
+
+
+static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev, struct usb_interface *intf)
+{
+	struct cdc_ncm_ctx *ctx;
+	struct usb_driver *subdriver = ERR_PTR(-ENODEV);
+	int ret = -ENODEV;
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+
+	ret = cdc_ncm_bind_common(usbnet_dev, intf, 1); /* altsetting should be 1 for NCM devices */
+	if (ret)
+		goto err;
+
+	ctx = drvstate->ctx;
+
+	if (usbnet_dev->status)
+		subdriver = usb_cdc_wdm_register(ctx->control,
+						 &usbnet_dev->status->desc,
+						 256, /* CDC-WMC r1.1 requires wMaxCommand to be "at least 256 decimal (0x100)" */
+						 huawei_cdc_ncm_wdm_manage_power);
+	if (IS_ERR(subdriver)) {
+		ret = PTR_ERR(subdriver);
+		cdc_ncm_unbind(usbnet_dev, intf);
+		goto err;
+	}
+
+	/* Prevent usbnet from using the status descriptor */
+	usbnet_dev->status = NULL;
+
+	drvstate->subdriver = subdriver;
+
+err:
+	return ret;
+}
+
+static void huawei_cdc_ncm_unbind(struct usbnet *usbnet_dev, struct usb_interface *intf)
+{
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+
+	if (drvstate->subdriver && drvstate->subdriver->disconnect)
+		drvstate->subdriver->disconnect(ctx->control);
+	drvstate->subdriver = NULL;
+
+	cdc_ncm_unbind(usbnet_dev, intf);
+}
+
+static int huawei_cdc_ncm_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	int ret = 0;
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+
+	if (ctx == NULL) {
+		ret = -1;
+		goto error;
+	}
+
+	ret = usbnet_suspend(intf, message);
+	if (ret < 0)
+		goto error;
+
+	if (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->suspend)
+		ret = drvstate->subdriver->suspend(intf, message);
+	if (ret < 0)
+		usbnet_resume(intf);
+
+error:
+	return ret;
+}
+
+static int huawei_cdc_ncm_resume(struct usb_interface *intf)
+{
+	int ret = 0;
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+	bool callsub = (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->resume); /* should we call subdriver's resume? ? */
+
+	if (callsub)
+		ret = drvstate->subdriver->resume(intf);
+	if (ret < 0)
+		goto err;
+	ret = usbnet_resume(intf);
+	if (ret < 0 && callsub && drvstate->subdriver->suspend)
+		drvstate->subdriver->suspend(intf, PMSG_SUSPEND);
+err:
+	return ret;
+}
+
+static int huawei_cdc_ncm_check_connect(struct usbnet *usbnet_dev)
+{
+	struct cdc_ncm_ctx *ctx;
+
+	ctx = (struct cdc_ncm_ctx *)usbnet_dev->data[0];
+
+	if (ctx == NULL)
+		return 1; /* disconnected */
+
+	return !ctx->connected;
+}
+
+static const struct driver_info huawei_cdc_ncm_info = {
+	.description = "Huawei CDC NCM device",
+	.flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN,
+	.bind = huawei_cdc_ncm_bind,
+	.unbind = huawei_cdc_ncm_unbind,
+	.check_connect = huawei_cdc_ncm_check_connect,
+	.manage_power = huawei_cdc_ncm_manage_power,
+	.rx_fixup = cdc_ncm_rx_fixup,
+	.tx_fixup = cdc_ncm_tx_fixup,
+};
+
+static const struct usb_device_id huawei_cdc_ncm_devs[] = {
+	/* Huawei NCM devices disguised as vendor specific */
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x16),
+	  .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x46),
+	  .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x76),
+	  .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+
+	/* Terminating entry */
+	{
+	},
+};
+MODULE_DEVICE_TABLE(usb, huawei_cdc_ncm_devs);
+
+static struct usb_driver huawei_cdc_ncm_driver = {
+	.name = "huawei_cdc_ncm",
+	.id_table = huawei_cdc_ncm_devs,
+	.probe = usbnet_probe,
+	.disconnect = usbnet_disconnect,
+	.suspend = huawei_cdc_ncm_suspend,
+	.resume = huawei_cdc_ncm_resume,
+	.reset_resume = huawei_cdc_ncm_resume,
+	.supports_autosuspend = 1,
+	.disable_hub_initiated_lpm = 1,
+};
+module_usb_driver(huawei_cdc_ncm_driver);
+MODULE_AUTHOR("Enrico Mioso <mrkiko.rs@gmail.com>");
+MODULE_DESCRIPTION("USB CDC NCM host driver with encapsulated protocol support");
+MODULE_LICENSE("GPL");

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

* [PATCH 3/3] net: cdc_ncm: remove Huawei non-standard NCM device IDs
  2013-07-02 12:01 ` Bjørn Mork
                     ` (3 preceding siblings ...)
  2013-07-10 12:28   ` [PATCH 2/3] net: huawei_cdc_ncm: Introduce the " Enrico Mioso
@ 2013-07-10 12:30   ` Enrico Mioso
  4 siblings, 0 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-07-10 12:30 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: netdev

Remove device IDs of NCM-like (but not NCM-conformant) devices, that are 
handled by the huawwei_cdc_ncm driver now.

Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
---


 drivers/net/usb/cdc_ncm.c |   11 -----------
 1 file changed, 11 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 62686be..31f43f7 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1236,17 +1236,6 @@ static const struct usb_device_id cdc_devs[] = {
 	  .driver_info = (unsigned long)&wwan_info,
 	},
 
-	/* Huawei NCM devices disguised as vendor specific */
-	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x16),
-	  .driver_info = (unsigned long)&wwan_info,
-	},
-	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x46),
-	  .driver_info = (unsigned long)&wwan_info,
-	},
-	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x76),
-	  .driver_info = (unsigned long)&wwan_info,
-	},
-
 	/* Infineon(now Intel) HSPA Modem platform */
 	{ USB_DEVICE_AND_INTERFACE_INFO(0x1519, 0x0443,
 		USB_CLASS_COMM,

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

* [PATCH] net: cdc_ncm: Export cdc_ncm_{tx,rx}_fixup functions for re-use
       [not found]           ` <87bo4xx3ef.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
@ 2013-08-16 13:39             ` Enrico Mioso
  2013-08-16 13:49               ` Greg Kroah-Hartman
  0 siblings, 1 reply; 40+ messages in thread
From: Enrico Mioso @ 2013-08-16 13:39 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: Greg Kroah-Hartman, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA

Some drivers implementing NCM-like protocols, may re-use those functions, as is 
the case in the huawei_cdc_ncm driver.
Export them via EXPORT_SYMBOL_GPL.

Signed-off-by: Enrico Mioso <mrkiko.rs-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---

 drivers/net/usb/cdc_ncm.c   |    6 ++++--
 include/linux/usb/cdc_ncm.h |    3 +++
 2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 43afde8..62686be 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -858,7 +858,7 @@ static void cdc_ncm_txpath_bh(unsigned long param)
 	}
 }
 
-static struct sk_buff *
+struct sk_buff *
 cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
 {
 	struct sk_buff *skb_out;
@@ -885,6 +885,7 @@ error:
 
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(cdc_ncm_tx_fixup);
 
 /* verify NTB header and return offset of first NDP, or negative error */
 int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
@@ -965,7 +966,7 @@ error:
 }
 EXPORT_SYMBOL_GPL(cdc_ncm_rx_verify_ndp16);
 
-static int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
+int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in)
 {
 	struct sk_buff *skb;
 	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
@@ -1040,6 +1041,7 @@ err_ndp:
 error:
 	return 0;
 }
+EXPORT_SYMBOL_GPL(cdc_ncm_rx_fixup);
 
 static void
 cdc_ncm_speed_change(struct cdc_ncm_ctx *ctx,
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index cc25b70..163244b 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -133,3 +133,6 @@ extern void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
 extern struct sk_buff *cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign);
 extern int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in);
 extern int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
+struct sk_buff *
+cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags);
+int cdc_ncm_rx_fixup(struct usbnet *dev, struct sk_buff *skb_in);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH] net: huawei_cdc_ncm: Introduce new huawei_cdc_ncm driver
       [not found]         ` <87bo4xx3ef.fsf@nemi.mork.no>
       [not found]           ` <87bo4xx3ef.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
@ 2013-08-16 13:44           ` Enrico Mioso
  1 sibling, 0 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-08-16 13:44 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: Greg Kroah-Hartman, linux-kernel, linux-usb, netdev

This driver supports devices using the NCM protocol as an encapsulation layer
for other protocols, like the E3131 Huawei 3G modem.

Suggested-by: Bjorn Mork <bjorn@mork.no>
Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
---

 drivers/net/usb/Kconfig          |   11 ++
 drivers/net/usb/Makefile         |    1 +
 drivers/net/usb/huawei_cdc_ncm.c |  210 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 222 insertions(+)
 create mode 100644 drivers/net/usb/huawei_cdc_ncm.c
diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index d84bfd4..6e56751 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -242,6 +242,17 @@ config USB_NET_CDC_NCM
 	    * ST-Ericsson M343 HSPA Mobile Broadband Modem (reference design)
 	    * Ericsson F5521gw Mobile Broadband Module
 
+config USB_NET_HUAWEI_CDC_NCM
+	tristate "Huawei-style CDC NCM support"
+	depends on USB_USBNET
+	select USB_WDM
+	select USB_NET_CDC_NCM
+	help
+		This driver aims to support huawei-style NCM devices, that use ncm as a
+		transport for other protocols.
+		To compile this driver as a module, choose M here: the module will be
+		called huawei_cdc_ncm.
+
 config USB_NET_CDC_MBIM
 	tristate "CDC MBIM support"
 	depends on USB_USBNET
diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index e817178..fd0e6a7 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_USB_IPHETH)	+= ipheth.o
 obj-$(CONFIG_USB_SIERRA_NET)	+= sierra_net.o
 obj-$(CONFIG_USB_NET_CX82310_ETH)	+= cx82310_eth.o
 obj-$(CONFIG_USB_NET_CDC_NCM)	+= cdc_ncm.o
+obj-$(CONFIG_USB_NET_HUAWEI_CDC_NCM)	+= huawei_cdc_ncm.o
 obj-$(CONFIG_USB_VL600)		+= lg-vl600.o
 obj-$(CONFIG_USB_NET_QMI_WWAN)	+= qmi_wwan.o
 obj-$(CONFIG_USB_NET_CDC_MBIM)	+= cdc_mbim.o
diff --git a/drivers/net/usb/huawei_cdc_ncm.c b/drivers/net/usb/huawei_cdc_ncm.c
new file mode 100644
index 0000000..d3426b9
--- /dev/null
+++ b/drivers/net/usb/huawei_cdc_ncm.c
@@ -0,0 +1,210 @@
+/*
+ * huawei_cdc_ncm.c - handles huawei-style NCM devices.
+ * Copyright (C) 2013	 Enrico Mioso <mrkiko.rs@gmail.com>
+ * This driver handles devices resembling the CDC NCM standard, but
+ * encapsulating another protocol inside it. An example are some Huawei 3G
+ * devices, exposing an embedded AT channel where you can set up the NCM
+ * connection.
+ * This code has been heavily inspired by the cdc_mbim.c driver, which is
+ * Copyright (c) 2012  Smith Micro Software, Inc.
+ * Copyright (c) 2012  Bj??rn Mork <bjorn@mork.no>
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/ethtool.h>
+#include <linux/if_vlan.h>
+#include <linux/ip.h>
+#include <linux/mii.h>
+#include <linux/usb.h>
+#include <linux/usb/cdc.h>
+#include <linux/usb/usbnet.h>
+#include <linux/usb/cdc-wdm.h>
+#include <linux/usb/cdc_ncm.h>
+
+/* Driver data */
+struct huawei_cdc_ncm_state {
+	struct cdc_ncm_ctx *ctx;
+	atomic_t pmcount;
+	struct usb_driver *subdriver;
+	struct usb_interface *control;
+	struct usb_interface *data;
+};
+
+static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
+{
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	int rv = 0;
+
+	if ((on && atomic_add_return(1, &drvstate->pmcount) == 1) || (!on && atomic_dec_and_test(&drvstate->pmcount))) {
+		rv = usb_autopm_get_interface(usbnet_dev->intf);
+		if (rv < 0)
+			goto err;
+		usbnet_dev->intf->needs_remote_wakeup = on;
+		usb_autopm_put_interface(usbnet_dev->intf);
+	}
+err:
+	return rv;
+}
+
+static int huawei_cdc_ncm_wdm_manage_power(struct usb_interface *intf, int status)
+{
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+
+	/* can be called while disconnecting */
+	if (!usbnet_dev)
+		return 0;
+
+	return huawei_cdc_ncm_manage_power(usbnet_dev, status);
+}
+
+
+static int huawei_cdc_ncm_bind(struct usbnet *usbnet_dev, struct usb_interface *intf)
+{
+	struct cdc_ncm_ctx *ctx;
+	struct usb_driver *subdriver = ERR_PTR(-ENODEV);
+	int ret = -ENODEV;
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+
+	ret = cdc_ncm_bind_common(usbnet_dev, intf, 1); /* altsetting should be 1 for NCM devices */
+	if (ret)
+		goto err;
+
+	ctx = drvstate->ctx;
+
+	if (usbnet_dev->status)
+		subdriver = usb_cdc_wdm_register(ctx->control,
+						 &usbnet_dev->status->desc,
+						 256, /* CDC-WMC r1.1 requires wMaxCommand to be "at least 256 decimal (0x100)" */
+						 huawei_cdc_ncm_wdm_manage_power);
+	if (IS_ERR(subdriver)) {
+		ret = PTR_ERR(subdriver);
+		cdc_ncm_unbind(usbnet_dev, intf);
+		goto err;
+	}
+
+	/* Prevent usbnet from using the status descriptor */
+	usbnet_dev->status = NULL;
+
+	drvstate->subdriver = subdriver;
+
+err:
+	return ret;
+}
+
+static void huawei_cdc_ncm_unbind(struct usbnet *usbnet_dev, struct usb_interface *intf)
+{
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+
+	if (drvstate->subdriver && drvstate->subdriver->disconnect)
+		drvstate->subdriver->disconnect(ctx->control);
+	drvstate->subdriver = NULL;
+
+	cdc_ncm_unbind(usbnet_dev, intf);
+}
+
+static int huawei_cdc_ncm_suspend(struct usb_interface *intf, pm_message_t message)
+{
+	int ret = 0;
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+
+	if (ctx == NULL) {
+		ret = -1;
+		goto error;
+	}
+
+	ret = usbnet_suspend(intf, message);
+	if (ret < 0)
+		goto error;
+
+	if (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->suspend)
+		ret = drvstate->subdriver->suspend(intf, message);
+	if (ret < 0)
+		usbnet_resume(intf);
+
+error:
+	return ret;
+}
+
+static int huawei_cdc_ncm_resume(struct usb_interface *intf)
+{
+	int ret = 0;
+	struct usbnet *usbnet_dev = usb_get_intfdata(intf);
+	struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
+	struct cdc_ncm_ctx *ctx = drvstate->ctx;
+	bool callsub = (intf == ctx->control && drvstate->subdriver && drvstate->subdriver->resume); /* should we call subdriver's resume? ? */
+
+	if (callsub)
+		ret = drvstate->subdriver->resume(intf);
+	if (ret < 0)
+		goto err;
+	ret = usbnet_resume(intf);
+	if (ret < 0 && callsub && drvstate->subdriver->suspend)
+		drvstate->subdriver->suspend(intf, PMSG_SUSPEND);
+err:
+	return ret;
+}
+
+static int huawei_cdc_ncm_check_connect(struct usbnet *usbnet_dev)
+{
+	struct cdc_ncm_ctx *ctx;
+
+	ctx = (struct cdc_ncm_ctx *)usbnet_dev->data[0];
+
+	if (ctx == NULL)
+		return 1; /* disconnected */
+
+	return !ctx->connected;
+}
+
+static const struct driver_info huawei_cdc_ncm_info = {
+	.description = "Huawei CDC NCM device",
+	.flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN,
+	.bind = huawei_cdc_ncm_bind,
+	.unbind = huawei_cdc_ncm_unbind,
+	.check_connect = huawei_cdc_ncm_check_connect,
+	.manage_power = huawei_cdc_ncm_manage_power,
+	.rx_fixup = cdc_ncm_rx_fixup,
+	.tx_fixup = cdc_ncm_tx_fixup,
+};
+
+static const struct usb_device_id huawei_cdc_ncm_devs[] = {
+	/* Huawei NCM devices disguised as vendor specific */
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x16),
+	  .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x46),
+	  .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+	{ USB_VENDOR_AND_INTERFACE_INFO(0x12d1, 0xff, 0x02, 0x76),
+	  .driver_info = (unsigned long)&huawei_cdc_ncm_info,
+	},
+
+	/* Terminating entry */
+	{
+	},
+};
+MODULE_DEVICE_TABLE(usb, huawei_cdc_ncm_devs);
+
+static struct usb_driver huawei_cdc_ncm_driver = {
+	.name = "huawei_cdc_ncm",
+	.id_table = huawei_cdc_ncm_devs,
+	.probe = usbnet_probe,
+	.disconnect = usbnet_disconnect,
+	.suspend = huawei_cdc_ncm_suspend,
+	.resume = huawei_cdc_ncm_resume,
+	.reset_resume = huawei_cdc_ncm_resume,
+	.supports_autosuspend = 1,
+	.disable_hub_initiated_lpm = 1,
+};
+module_usb_driver(huawei_cdc_ncm_driver);
+MODULE_AUTHOR("Enrico Mioso <mrkiko.rs@gmail.com>");
+MODULE_DESCRIPTION("USB CDC NCM host driver with encapsulated protocol support");
+MODULE_LICENSE("GPL");

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

* Re: [PATCH] net: cdc_ncm: Export cdc_ncm_{tx,rx}_fixup functions for re-use
  2013-08-16 13:39             ` [PATCH] " Enrico Mioso
@ 2013-08-16 13:49               ` Greg Kroah-Hartman
  2013-08-16 13:51                 ` Enrico Mioso
  2013-08-16 13:52                 ` Greg Kroah-Hartman
  0 siblings, 2 replies; 40+ messages in thread
From: Greg Kroah-Hartman @ 2013-08-16 13:49 UTC (permalink / raw)
  To: Enrico Mioso; +Cc: Bjørn Mork, linux-kernel, linux-usb, netdev

On Fri, Aug 16, 2013 at 03:39:19PM +0200, Enrico Mioso wrote:
> Some drivers implementing NCM-like protocols, may re-use those functions, as is 
> the case in the huawei_cdc_ncm driver.

Where is that driver at, I don't see it in the kernel tree.

> Export them via EXPORT_SYMBOL_GPL.

Normally we don't export symbols until code that actually uses the
symbols lands in the tree at the same time.

thanks,

greg k-h

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

* Re: [PATCH] net: cdc_ncm: Export cdc_ncm_{tx,rx}_fixup functions for re-use
  2013-08-16 13:49               ` Greg Kroah-Hartman
@ 2013-08-16 13:51                 ` Enrico Mioso
  2013-08-16 13:52                 ` Greg Kroah-Hartman
  1 sibling, 0 replies; 40+ messages in thread
From: Enrico Mioso @ 2013-08-16 13:51 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Enrico Mioso, Bjørn Mork, linux-kernel, linux-usb, netdev

Yes, you're right. I'm sorry - this time I didn't apply numbering scheme to 
those 3 patches - yet they are consequential, but not depending on each other 
from a strict point of view.

Thank you for your review Greg!


On Fri, 16 Aug 2013, Greg Kroah-Hartman wrote:

==Date: Fri, 16 Aug 2013 06:49:07 -0700
==From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
==To: Enrico Mioso <mrkiko.rs@gmail.com>
==Cc: Bj?rn Mork <bjorn@mork.no>, linux-kernel@vger.kernel.org,
==    linux-usb@vger.kernel.org, netdev@vger.kernel.org
==Subject: Re: [PATCH] net: cdc_ncm: Export cdc_ncm_{tx,
==    rx}_fixup functions for re-use
==
==On Fri, Aug 16, 2013 at 03:39:19PM +0200, Enrico Mioso wrote:
==> Some drivers implementing NCM-like protocols, may re-use those functions, as is 
==> the case in the huawei_cdc_ncm driver.
==
==Where is that driver at, I don't see it in the kernel tree.
==
==> Export them via EXPORT_SYMBOL_GPL.
==
==Normally we don't export symbols until code that actually uses the
==symbols lands in the tree at the same time.
==
==thanks,
==
==greg k-h
==

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

* Re: [PATCH] net: cdc_ncm: Export cdc_ncm_{tx,rx}_fixup functions for re-use
  2013-08-16 13:49               ` Greg Kroah-Hartman
  2013-08-16 13:51                 ` Enrico Mioso
@ 2013-08-16 13:52                 ` Greg Kroah-Hartman
  1 sibling, 0 replies; 40+ messages in thread
From: Greg Kroah-Hartman @ 2013-08-16 13:52 UTC (permalink / raw)
  To: Enrico Mioso; +Cc: Bjørn Mork, linux-kernel, linux-usb, netdev

On Fri, Aug 16, 2013 at 06:49:07AM -0700, Greg Kroah-Hartman wrote:
> On Fri, Aug 16, 2013 at 03:39:19PM +0200, Enrico Mioso wrote:
> > Some drivers implementing NCM-like protocols, may re-use those functions, as is 
> > the case in the huawei_cdc_ncm driver.
> 
> Where is that driver at, I don't see it in the kernel tree.

And you sent it in a different email, sorry about that.

Normally you would number the patches (i.e. [1/3], and so on), so that
people know which order to apply them in, and that there really is 3
patches in the series...

thanks,

greg k-h

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

end of thread, other threads:[~2013-08-16 13:52 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-02 10:43 [PATCH RFC] cdc_ncm.c: make rx and tx functions exportable Enrico Mioso
2013-07-02 12:01 ` Bjørn Mork
2013-07-02 12:05   ` Enrico Mioso
2013-07-02 17:04   ` [RFC] huawei_cdc_ncm driver - status Enrico Mioso
2013-07-02 19:06     ` Enrico Mioso
2013-07-02 20:06     ` [PATCH 0/3] make cdc_ncm_tx_fixup and cdc_ncm_rx_fixup non-static functions Enrico Mioso
2013-07-02 20:10       ` [PATCH 1/3] Export cdc_ncm_{tx,rx}_fixup functions Enrico Mioso
2013-07-02 20:13         ` [PATCH RFC 2/3] Introduce huawei_cdc_ncm driver Enrico Mioso
2013-07-02 20:15           ` [PATCH RFC 3/3] huawei_cdc_ncm base skeleton Enrico Mioso
2013-07-03  6:40             ` Huaei E3131 - connected! Enrico Mioso
2013-07-03  7:11               ` Huaei E3131 - status Enrico Mioso
2013-07-03  8:15                 ` Bjørn Mork
2013-07-03 12:55                   ` [PATCH V2 1/3] usbnet: cdc_ncm: let cdc_ncm_tx_fixup and cdc_ncm_rx_fixup be non-static Enrico Mioso
2013-07-03 13:00                   ` [PATCH V2 2/3] usbnet: cdc_ncm: export cdc_ncm_tx_fixup and cdc_ncm_rx_fixup Enrico Mioso
2013-07-03 13:19                     ` Sergei Shtylyov
2013-07-03 13:22                       ` Enrico Mioso
2013-07-03 13:33                       ` [PATCH V3 1/2] Export cdc_ncm_{tx,rx]_fixup functions Enrico Mioso
2013-07-03 13:38                       ` [PATCH V3 2/2] Introduce huawei_cdc_ncm driver Enrico Mioso
2013-07-03 16:18                         ` Ben Hutchings
2013-07-03 18:11                           ` [PATCH V3 2/2 RESEND] " Enrico Mioso
2013-07-04 13:19                             ` Bjørn Mork
2013-07-04 17:03                               ` Enrico Mioso
2013-07-03 18:11                           ` [PATCH V3 2/2] " Enrico Mioso
2013-07-03 13:05                   ` [PATCH V2 3/3] usbnet: introduce " Enrico Mioso
2013-07-03 13:13                   ` [PATCH] usbnet: cdc_ncm: remove huawei devices handled by cdc_ncm_huawei.c Enrico Mioso
2013-07-03  7:13               ` Huaei E3131 - connected! Bjørn Mork
2013-07-03  7:38             ` [PATCH RFC 3/3] huawei_cdc_ncm base skeleton Bjørn Mork
2013-07-03 10:16               ` Enrico Mioso
2013-07-03  7:40           ` [PATCH RFC 2/3] Introduce huawei_cdc_ncm driver Bjørn Mork
2013-07-03  7:43         ` [PATCH 1/3] Export cdc_ncm_{tx,rx}_fixup functions Bjørn Mork
2013-07-03 10:19           ` Enrico Mioso
2013-07-03  7:45       ` [PATCH 0/3] make cdc_ncm_tx_fixup and cdc_ncm_rx_fixup non-static functions Bjørn Mork
2013-07-10 12:27   ` [PATCH 1/3] net: cdc_ncm: Export cdc_ncm_{tx,rx}_fixup functions for re-use Enrico Mioso
     [not found]     ` <0cbf7c81-182e-493c-9c28-5b78160a08f1@email.android.com>
     [not found]       ` <alpine.LNX.2.02.1307170457450.1161@eeeadesso>
     [not found]         ` <87bo4xx3ef.fsf@nemi.mork.no>
     [not found]           ` <87bo4xx3ef.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
2013-08-16 13:39             ` [PATCH] " Enrico Mioso
2013-08-16 13:49               ` Greg Kroah-Hartman
2013-08-16 13:51                 ` Enrico Mioso
2013-08-16 13:52                 ` Greg Kroah-Hartman
2013-08-16 13:44           ` [PATCH] net: huawei_cdc_ncm: Introduce new huawei_cdc_ncm driver Enrico Mioso
2013-07-10 12:28   ` [PATCH 2/3] net: huawei_cdc_ncm: Introduce the " Enrico Mioso
2013-07-10 12:30   ` [PATCH 3/3] net: cdc_ncm: remove Huawei non-standard NCM device IDs Enrico Mioso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).