[PATCH net-next 00/24] cdc_ncm: many small and mostly trivial fixes

[PATCH net-next 00/24] cdc_ncm: many small and mostly trivial fixes

[Bjørn Mork]

This series ended up longer than expected, and it is still not
complete. There is more to come when time allows...

Most changes are trivial. Notable non-trivial changes are
 - removed filtering of identical speed notifications
 - tx_max calulation is changed to count the pad byte if
   necessary, and respect the device limit as an absolute
   upper limit even if it is too low according to the spec
 - remove the bug preventing SET_MAX_DATAGRAM_SIZE from having
   any effect
 - drop the pad-to-max if ZLPs are enabled
 - the driver specific VERSION is dropped
 - dev->hard_mtu is set to tx_max instead of max_datagram_size
   causing usbnet to calculate the qlen based on the real max
   size of tx skbs

This series has been tested, along with the previously posted
cdc_mbim series, on the NCM and MBIM devices I have:
 - Ericsson F5521gw (NCM)
 - Huawei E367 (MBIM)
 - D-Link DWM-156 A7 (MBIM w/ too low dwNtb{In,Out}MaxSize bug)
 - Sierra Wireless MC7710 (MBIM w/ ZLP and CDC Union bugs)

Apart from the D-Link modem dropping a lot less oversized
frames with the fix dedicated to it, there are no end user
noticable functional changes as a result of this series.  But
all the non-trivial changes I listed above are of course
detectable by users looking at that specific area (except maybe
the removed speed notification, which requires a device sending
duplicates to be noticable - I don't have any such device).


Bjørn Mork (24):
  net: cdc_ncm: simplify and optimize frame padding
  net: cdc_ncm: add include protection to cdc_ncm.h
  net: cdc_ncm: remove redundant "intf" field
  net: cdc_ncm: remove redundant endpoint pointers
  net: cdc_ncm: remove redundant netdev field
  net: cdc_ncm: remove unused udev field
  net: cdc_ncm: remove tx_speed and rx_speed fields
  net: cdc_ncm: remove ncm_parm field
  net: cdc_ncm: fix SET_MAX_DATAGRAM_SIZE
  net: cdc_ncm: remove descriptor pointers
  net: cdc_ncm: only the control intf can be probed
  net: cdc_ncm: no point in filling up the NTBs if we send ZLPs
  net: cdc_ncm: remove probe and disconnect wrappers
  net: cdc_ncm: remove ethtool ops
  net: cdc_ncm: set correct dev->hard_mtu
  net: cdc_ncm: log the length we warn about
  net: cdc_ncm: use netif_* and dev_* instead of pr_*
  net: cdc_ncm: log signatures in hex
  net: cdc_ncm: endian convert constants instead of variables
  net: cdc_ncm: drop "extern" from header declarations
  net: cdc_ncm: refactoring cdc_ncm_setup
  net: cdc_ncm: return proper error if setup fails
  net: cdc_ncm: improve bind error debug messages
  net: cdc_ncm: no not set tx_max higher than the device supports

 drivers/net/usb/cdc_mbim.c  |    2 +-
 drivers/net/usb/cdc_ncm.c   |  490 +++++++++++++++++++------------------------
 include/linux/usb/cdc_ncm.h |   30 ++-
 3 files changed, 233 insertions(+), 289 deletions(-)

-- 
1.7.10.4

[PATCH net-next 01/24] net: cdc_ncm: simplify and optimize frame padding

[Bjørn Mork]

We can avoid the costly division for the common case where
we pad the frame to tx_max size as long as we ensure that
tx_max is either the device specified dwNtbOutMaxSize or not
a multiplum of wMaxPacketSize.

Using the preconverted 'maxpacket' field avoids converting
wMaxPacketSize to CPU endianness for every transmitted frame

And since we only will hit the one byte padding rule for short
frames, we can drop testing the skb for tailroom.

The change means that tx_max now represents the real maximum
skb size, enabling us to allocate the correct size instead of
always making room for one extra byte.

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c |   31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 43afde8..38566bf 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -506,6 +506,15 @@ advance:
 	dev->status = ctx->status_ep;
 	dev->rx_urb_size = ctx->rx_max;
 
+	/* cdc_ncm_setup will override dwNtbOutMaxSize if it is
+	 * outside the sane range. Adding a pad byte here if necessary
+	 * simplifies the handling in cdc_ncm_fill_tx_frame, making
+	 * tx_max always represent the real skb max size.
+	 */
+	if (ctx->tx_max != le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize) &&
+	    ctx->tx_max % usb_maxpacket(dev->udev, dev->out, 1) == 0)
+		ctx->tx_max++;
+
 	ctx->tx_speed = ctx->rx_speed = 0;
 	return 0;
 
@@ -664,6 +673,7 @@ static struct usb_cdc_ncm_ndp16 *cdc_ncm_ndp(struct cdc_ncm_ctx *ctx, struct sk_
 struct sk_buff *
 cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
 {
+	struct usbnet *dev = netdev_priv(ctx->netdev);
 	struct usb_cdc_ncm_nth16 *nth16;
 	struct usb_cdc_ncm_ndp16 *ndp16;
 	struct sk_buff *skb_out;
@@ -683,7 +693,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
 
 	/* allocate a new OUT skb */
 	if (!skb_out) {
-		skb_out = alloc_skb((ctx->tx_max + 1), GFP_ATOMIC);
+		skb_out = alloc_skb(ctx->tx_max, GFP_ATOMIC);
 		if (skb_out == NULL) {
 			if (skb != NULL) {
 				dev_kfree_skb_any(skb);
@@ -788,19 +798,16 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
 		/* variables will be reset at next call */
 	}
 
-	/*
-	 * If collected data size is less or equal CDC_NCM_MIN_TX_PKT bytes,
-	 * we send buffers as it is. If we get more data, it would be more
-	 * efficient for USB HS mobile device with DMA engine to receive a full
-	 * size NTB, than canceling DMA transfer and receiving a short packet.
+	/* If collected data size is less or equal CDC_NCM_MIN_TX_PKT
+	 * bytes, we send buffers as it is. If we get more data, it
+	 * would be more efficient for USB HS mobile device with DMA
+	 * engine to receive a full size NTB, than canceling DMA
+	 * transfer and receiving a short packet.
 	 */
 	if (skb_out->len > CDC_NCM_MIN_TX_PKT)
-		/* final zero padding */
-		memset(skb_put(skb_out, ctx->tx_max - skb_out->len), 0, ctx->tx_max - skb_out->len);
-
-	/* do we need to prevent a ZLP? */
-	if (((skb_out->len % le16_to_cpu(ctx->out_ep->desc.wMaxPacketSize)) == 0) &&
-	    (skb_out->len < le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize)) && skb_tailroom(skb_out))
+		memset(skb_put(skb_out, ctx->tx_max - skb_out->len), 0,
+		       ctx->tx_max - skb_out->len);
+	else if ((skb_out->len % dev->maxpacket) == 0)
 		*skb_put(skb_out, 1) = 0;	/* force short packet */
 
 	/* set final frame length */
-- 
1.7.10.4

[PATCH net-next 02/24] net: cdc_ncm: add include protection to cdc_ncm.h

[Bjørn Mork]

This makes it a lot easier to test modified versions

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 include/linux/usb/cdc_ncm.h |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index cc25b70..89f0bbc 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -36,6 +36,9 @@
  * SUCH DAMAGE.
  */
 
+#ifndef __LINUX_USB_CDC_NCM_H
+#define __LINUX_USB_CDC_NCM_H
+
 #define CDC_NCM_COMM_ALTSETTING_NCM		0
 #define CDC_NCM_COMM_ALTSETTING_MBIM		1
 
@@ -133,3 +136,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);
+
+#endif /* __LINUX_USB_CDC_NCM_H */
-- 
1.7.10.4

[PATCH net-next 03/24] net: cdc_ncm: remove redundant "intf" field

[Bjørn Mork]

This is always a duplicate of the "control" field. It causes
confusion wrt intf_data updates and cleanups.

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c   |    4 +---
 include/linux/usb/cdc_ncm.h |    1 -
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 38566bf..bfab879 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -382,7 +382,6 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 	len = intf->cur_altsetting->extralen;
 
 	ctx->udev = dev->udev;
-	ctx->intf = intf;
 
 	/* parse through descriptors associated with control interface */
 	while ((len > 0) && (buf[0] > 2) && (buf[0] <= len)) {
@@ -489,7 +488,6 @@ advance:
 
 	usb_set_intfdata(ctx->data, dev);
 	usb_set_intfdata(ctx->control, dev);
-	usb_set_intfdata(ctx->intf, dev);
 
 	if (ctx->ether_desc) {
 		temp = usbnet_get_ethernet_addr(dev, ctx->ether_desc->iMACAddress);
@@ -562,7 +560,7 @@ void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf)
 		ctx->control = NULL;
 	}
 
-	usb_set_intfdata(ctx->intf, NULL);
+	usb_set_intfdata(intf, NULL);
 	cdc_ncm_free(ctx);
 }
 EXPORT_SYMBOL_GPL(cdc_ncm_unbind);
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 89f0bbc..c14e00f 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -103,7 +103,6 @@ struct cdc_ncm_ctx {
 	struct usb_host_endpoint *in_ep;
 	struct usb_host_endpoint *out_ep;
 	struct usb_host_endpoint *status_ep;
-	struct usb_interface *intf;
 	struct usb_interface *control;
 	struct usb_interface *data;
 
-- 
1.7.10.4

[PATCH net-next 04/24] net: cdc_ncm: remove redundant endpoint pointers

[Bjørn Mork]

No need to duplicate stuff already in the common usbnet
struct.  We still need to keep our special find_endpoints
function because we need explicit control over the selected
altsetting.

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c   |   38 +++++++++++++++++++-------------------
 include/linux/usb/cdc_ncm.h |    3 ---
 2 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index bfab879..a989bd5 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -292,9 +292,9 @@ max_dgram_err:
 }
 
 static void
-cdc_ncm_find_endpoints(struct cdc_ncm_ctx *ctx, struct usb_interface *intf)
+cdc_ncm_find_endpoints(struct usbnet *dev, struct usb_interface *intf)
 {
-	struct usb_host_endpoint *e;
+	struct usb_host_endpoint *e, *in = NULL, *out = NULL;
 	u8 ep;
 
 	for (ep = 0; ep < intf->cur_altsetting->desc.bNumEndpoints; ep++) {
@@ -303,18 +303,18 @@ cdc_ncm_find_endpoints(struct cdc_ncm_ctx *ctx, struct usb_interface *intf)
 		switch (e->desc.bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) {
 		case USB_ENDPOINT_XFER_INT:
 			if (usb_endpoint_dir_in(&e->desc)) {
-				if (ctx->status_ep == NULL)
-					ctx->status_ep = e;
+				if (!dev->status)
+					dev->status = e;
 			}
 			break;
 
 		case USB_ENDPOINT_XFER_BULK:
 			if (usb_endpoint_dir_in(&e->desc)) {
-				if (ctx->in_ep == NULL)
-					ctx->in_ep = e;
+				if (!in)
+					in = e;
 			} else {
-				if (ctx->out_ep == NULL)
-					ctx->out_ep = e;
+				if (!out)
+					out = e;
 			}
 			break;
 
@@ -322,6 +322,14 @@ cdc_ncm_find_endpoints(struct cdc_ncm_ctx *ctx, struct usb_interface *intf)
 			break;
 		}
 	}
+	if (in && !dev->in)
+		dev->in = usb_rcvbulkpipe(dev->udev,
+					  in->desc.bEndpointAddress &
+					  USB_ENDPOINT_NUMBER_MASK);
+	if (out && !dev->out)
+		dev->out = usb_sndbulkpipe(dev->udev,
+					   out->desc.bEndpointAddress &
+					   USB_ENDPOINT_NUMBER_MASK);
 }
 
 static void cdc_ncm_free(struct cdc_ncm_ctx *ctx)
@@ -477,11 +485,9 @@ advance:
 	if (temp)
 		goto error2;
 
-	cdc_ncm_find_endpoints(ctx, ctx->data);
-	cdc_ncm_find_endpoints(ctx, ctx->control);
-
-	if ((ctx->in_ep == NULL) || (ctx->out_ep == NULL) ||
-	    (ctx->status_ep == NULL))
+	cdc_ncm_find_endpoints(dev, ctx->data);
+	cdc_ncm_find_endpoints(dev, ctx->control);
+	if (!dev->in || !dev->out || !dev->status)
 		goto error2;
 
 	dev->net->ethtool_ops = &cdc_ncm_ethtool_ops;
@@ -496,12 +502,6 @@ advance:
 		dev_info(&dev->udev->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
 	}
 
-
-	dev->in = usb_rcvbulkpipe(dev->udev,
-		ctx->in_ep->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK);
-	dev->out = usb_sndbulkpipe(dev->udev,
-		ctx->out_ep->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK);
-	dev->status = ctx->status_ep;
 	dev->rx_urb_size = ctx->rx_max;
 
 	/* cdc_ncm_setup will override dwNtbOutMaxSize if it is
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index c14e00f..36e1e15 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -100,9 +100,6 @@ struct cdc_ncm_ctx {
 
 	struct net_device *netdev;
 	struct usb_device *udev;
-	struct usb_host_endpoint *in_ep;
-	struct usb_host_endpoint *out_ep;
-	struct usb_host_endpoint *status_ep;
 	struct usb_interface *control;
 	struct usb_interface *data;
 
-- 
1.7.10.4

[PATCH net-next 05/24] net: cdc_ncm: remove redundant netdev field

[Bjørn Mork]

Too many pointers back and forth are likely to confuse developers,
creating subtle bugs whenever we forget to syncronize them all.

As a usbnet driver, we should stick with the standard struct
usbnet fields as much as possible.  The netdevice is one such
field.

Cc: Greg Suarez <gsuarez-AKjrjAf1O7qe8kRwQpwjMg@public.gmane.org>
Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/cdc_mbim.c  |    2 +-
 drivers/net/usb/cdc_ncm.c   |   73 ++++++++++++++++++++++---------------------
 include/linux/usb/cdc_ncm.h |    3 +-
 3 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 253472a..af76aaf 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -175,7 +175,7 @@ static struct sk_buff *cdc_mbim_tx_fixup(struct usbnet *dev, struct sk_buff *skb
 	}
 
 	spin_lock_bh(&ctx->mtx);
-	skb_out = cdc_ncm_fill_tx_frame(ctx, skb, sign);
+	skb_out = cdc_ncm_fill_tx_frame(dev, skb, sign);
 	spin_unlock_bh(&ctx->mtx);
 	return skb_out;
 
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index a989bd5..e39e767 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -80,8 +80,9 @@ cdc_ncm_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *info)
 	usb_make_path(dev->udev, info->bus_info, sizeof(info->bus_info));
 }
 
-static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
+static u8 cdc_ncm_setup(struct usbnet *dev)
 {
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
 	u32 val;
 	u8 flags;
 	u8 iface_no;
@@ -90,7 +91,6 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 	u16 ntb_fmt_supported;
 	u32 min_dgram_size;
 	u32 min_hdr_size;
-	struct usbnet *dev = netdev_priv(ctx->netdev);
 
 	iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
 
@@ -285,8 +285,8 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 	}
 
 max_dgram_err:
-	if (ctx->netdev->mtu != (ctx->max_datagram_size - eth_hlen))
-		ctx->netdev->mtu = ctx->max_datagram_size - eth_hlen;
+	if (dev->net->mtu != (ctx->max_datagram_size - eth_hlen))
+		dev->net->mtu = ctx->max_datagram_size - eth_hlen;
 
 	return 0;
 }
@@ -375,11 +375,10 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 
 	hrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 	ctx->tx_timer.function = &cdc_ncm_tx_timer_cb;
-	ctx->bh.data = (unsigned long)ctx;
+	ctx->bh.data = (unsigned long)dev;
 	ctx->bh.func = cdc_ncm_txpath_bh;
 	atomic_set(&ctx->stop, 0);
 	spin_lock_init(&ctx->mtx);
-	ctx->netdev = dev->net;
 
 	/* store ctx pointer in device data field */
 	dev->data[0] = (unsigned long)ctx;
@@ -477,7 +476,7 @@ advance:
 		goto error2;
 
 	/* initialize data interface */
-	if (cdc_ncm_setup(ctx))
+	if (cdc_ncm_setup(dev))
 		goto error2;
 
 	/* configure data interface */
@@ -669,9 +668,9 @@ static struct usb_cdc_ncm_ndp16 *cdc_ncm_ndp(struct cdc_ncm_ctx *ctx, struct sk_
 }
 
 struct sk_buff *
-cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
+cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign)
 {
-	struct usbnet *dev = netdev_priv(ctx->netdev);
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
 	struct usb_cdc_ncm_nth16 *nth16;
 	struct usb_cdc_ncm_ndp16 *ndp16;
 	struct sk_buff *skb_out;
@@ -695,7 +694,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
 		if (skb_out == NULL) {
 			if (skb != NULL) {
 				dev_kfree_skb_any(skb);
-				ctx->netdev->stats.tx_dropped++;
+				dev->net->stats.tx_dropped++;
 			}
 			goto exit_no_skb;
 		}
@@ -733,12 +732,12 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
 				/* won't fit, MTU problem? */
 				dev_kfree_skb_any(skb);
 				skb = NULL;
-				ctx->netdev->stats.tx_dropped++;
+				dev->net->stats.tx_dropped++;
 			} else {
 				/* no room for skb - store for later */
 				if (ctx->tx_rem_skb != NULL) {
 					dev_kfree_skb_any(ctx->tx_rem_skb);
-					ctx->netdev->stats.tx_dropped++;
+					dev->net->stats.tx_dropped++;
 				}
 				ctx->tx_rem_skb = skb;
 				ctx->tx_rem_sign = sign;
@@ -771,7 +770,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
 	if (skb != NULL) {
 		dev_kfree_skb_any(skb);
 		skb = NULL;
-		ctx->netdev->stats.tx_dropped++;
+		dev->net->stats.tx_dropped++;
 	}
 
 	ctx->tx_curr_frame_num = n;
@@ -814,7 +813,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb, __le32 sign)
 
 	/* return skb */
 	ctx->tx_curr_skb = NULL;
-	ctx->netdev->stats.tx_packets += ctx->tx_curr_frame_num;
+	dev->net->stats.tx_packets += ctx->tx_curr_frame_num;
 	return skb_out;
 
 exit_no_skb:
@@ -846,18 +845,19 @@ static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *timer)
 
 static void cdc_ncm_txpath_bh(unsigned long param)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)param;
+	struct usbnet *dev = (struct usbnet *)param;
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
 
 	spin_lock_bh(&ctx->mtx);
 	if (ctx->tx_timer_pending != 0) {
 		ctx->tx_timer_pending--;
 		cdc_ncm_tx_timeout_start(ctx);
 		spin_unlock_bh(&ctx->mtx);
-	} else if (ctx->netdev != NULL) {
+	} else if (dev->net != NULL) {
 		spin_unlock_bh(&ctx->mtx);
-		netif_tx_lock_bh(ctx->netdev);
-		usbnet_start_xmit(NULL, ctx->netdev);
-		netif_tx_unlock_bh(ctx->netdev);
+		netif_tx_lock_bh(dev->net);
+		usbnet_start_xmit(NULL, dev->net);
+		netif_tx_unlock_bh(dev->net);
 	} else {
 		spin_unlock_bh(&ctx->mtx);
 	}
@@ -880,7 +880,7 @@ cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
 		goto error;
 
 	spin_lock_bh(&ctx->mtx);
-	skb_out = cdc_ncm_fill_tx_frame(ctx, skb, cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN));
+	skb_out = cdc_ncm_fill_tx_frame(dev, skb, cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN));
 	spin_unlock_bh(&ctx->mtx);
 	return skb_out;
 
@@ -1047,9 +1047,10 @@ error:
 }
 
 static void
-cdc_ncm_speed_change(struct cdc_ncm_ctx *ctx,
+cdc_ncm_speed_change(struct usbnet *dev,
 		     struct usb_cdc_speed_change *data)
 {
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
 	uint32_t rx_speed = le32_to_cpu(data->DLBitRRate);
 	uint32_t tx_speed = le32_to_cpu(data->ULBitRate);
 
@@ -1063,18 +1064,18 @@ cdc_ncm_speed_change(struct cdc_ncm_ctx *ctx,
 
 		if ((tx_speed > 1000000) && (rx_speed > 1000000)) {
 			printk(KERN_INFO KBUILD_MODNAME
-				": %s: %u mbit/s downlink "
-				"%u mbit/s uplink\n",
-				ctx->netdev->name,
-				(unsigned int)(rx_speed / 1000000U),
-				(unsigned int)(tx_speed / 1000000U));
+			       ": %s: %u mbit/s downlink "
+			       "%u mbit/s uplink\n",
+			       dev->net->name,
+			       (unsigned int)(rx_speed / 1000000U),
+			       (unsigned int)(tx_speed / 1000000U));
 		} else {
 			printk(KERN_INFO KBUILD_MODNAME
-				": %s: %u kbit/s downlink "
-				"%u kbit/s uplink\n",
-				ctx->netdev->name,
-				(unsigned int)(rx_speed / 1000U),
-				(unsigned int)(tx_speed / 1000U));
+			       ": %s: %u kbit/s downlink "
+			       "%u kbit/s uplink\n",
+			       dev->net->name,
+			       (unsigned int)(rx_speed / 1000U),
+			       (unsigned int)(tx_speed / 1000U));
 		}
 	}
 }
@@ -1091,7 +1092,7 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
 
 	/* test for split data in 8-byte chunks */
 	if (test_and_clear_bit(EVENT_STS_SPLIT, &dev->flags)) {
-		cdc_ncm_speed_change(ctx,
+		cdc_ncm_speed_change(dev,
 		      (struct usb_cdc_speed_change *)urb->transfer_buffer);
 		return;
 	}
@@ -1108,8 +1109,8 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
 		ctx->connected = le16_to_cpu(event->wValue);
 
 		printk(KERN_INFO KBUILD_MODNAME ": %s: network connection:"
-			" %sconnected\n",
-			ctx->netdev->name, ctx->connected ? "" : "dis");
+		       " %sconnected\n",
+		       dev->net->name, ctx->connected ? "" : "dis");
 
 		usbnet_link_change(dev, ctx->connected, 0);
 		if (!ctx->connected)
@@ -1121,8 +1122,8 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
 					sizeof(struct usb_cdc_speed_change)))
 			set_bit(EVENT_STS_SPLIT, &dev->flags);
 		else
-			cdc_ncm_speed_change(ctx,
-				(struct usb_cdc_speed_change *) &event[1]);
+			cdc_ncm_speed_change(dev,
+					     (struct usb_cdc_speed_change *)&event[1]);
 		break;
 
 	default:
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 36e1e15..5c47bd9 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -98,7 +98,6 @@ struct cdc_ncm_ctx {
 	const struct usb_cdc_union_desc *union_desc;
 	const struct usb_cdc_ether_desc *ether_desc;
 
-	struct net_device *netdev;
 	struct usb_device *udev;
 	struct usb_interface *control;
 	struct usb_interface *data;
@@ -129,7 +128,7 @@ struct cdc_ncm_ctx {
 extern u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface *intf);
 extern int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting);
 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 struct sk_buff *cdc_ncm_fill_tx_frame(struct usbnet *dev, 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);
 
-- 
1.7.10.4

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

[PATCH net-next 06/24] net: cdc_ncm: remove unused udev field

[Bjørn Mork]

We already use the usbnet udev field everywhere this could have
been used.

Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/cdc_ncm.c   |    2 --
 include/linux/usb/cdc_ncm.h |    1 -
 2 files changed, 3 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index e39e767..9cdd762 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -388,8 +388,6 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 	buf = intf->cur_altsetting->extra;
 	len = intf->cur_altsetting->extralen;
 
-	ctx->udev = dev->udev;

[PATCH net-next 07/24] net: cdc_ncm: remove tx_speed and rx_speed fields

[Bjørn Mork]

These fields are only used to prevent printing the same speeds
multiple times if we receive multiple identical speed notifications.

The value of these printk's is questionable, and even more so when
we filter out some of the notifications sent us by the firmware. If
we are going to print any of these, then we should print them all.

Removing little used fields is a bonus.

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c   |   37 ++++++++++++++-----------------------
 include/linux/usb/cdc_ncm.h |    2 --
 2 files changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 9cdd762..fc36a99 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -510,7 +510,6 @@ advance:
 	    ctx->tx_max % usb_maxpacket(dev->udev, dev->out, 1) == 0)
 		ctx->tx_max++;
 
-	ctx->tx_speed = ctx->rx_speed = 0;
 	return 0;
 
 error2:
@@ -1048,7 +1047,6 @@ static void
 cdc_ncm_speed_change(struct usbnet *dev,
 		     struct usb_cdc_speed_change *data)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
 	uint32_t rx_speed = le32_to_cpu(data->DLBitRRate);
 	uint32_t tx_speed = le32_to_cpu(data->ULBitRate);
 
@@ -1056,25 +1054,20 @@ cdc_ncm_speed_change(struct usbnet *dev,
 	 * Currently the USB-NET API does not support reporting the actual
 	 * device speed. Do print it instead.
 	 */
-	if ((tx_speed != ctx->tx_speed) || (rx_speed != ctx->rx_speed)) {
-		ctx->tx_speed = tx_speed;
-		ctx->rx_speed = rx_speed;
-
-		if ((tx_speed > 1000000) && (rx_speed > 1000000)) {
-			printk(KERN_INFO KBUILD_MODNAME
-			       ": %s: %u mbit/s downlink "
-			       "%u mbit/s uplink\n",
-			       dev->net->name,
-			       (unsigned int)(rx_speed / 1000000U),
-			       (unsigned int)(tx_speed / 1000000U));
-		} else {
-			printk(KERN_INFO KBUILD_MODNAME
-			       ": %s: %u kbit/s downlink "
-			       "%u kbit/s uplink\n",
-			       dev->net->name,
-			       (unsigned int)(rx_speed / 1000U),
-			       (unsigned int)(tx_speed / 1000U));
-		}
+	if ((tx_speed > 1000000) && (rx_speed > 1000000)) {
+		printk(KERN_INFO KBUILD_MODNAME
+		       ": %s: %u mbit/s downlink "
+		       "%u mbit/s uplink\n",
+		       dev->net->name,
+		       (unsigned int)(rx_speed / 1000000U),
+		       (unsigned int)(tx_speed / 1000000U));
+	} else {
+		printk(KERN_INFO KBUILD_MODNAME
+		       ": %s: %u kbit/s downlink "
+		       "%u kbit/s uplink\n",
+		       dev->net->name,
+		       (unsigned int)(rx_speed / 1000U),
+		       (unsigned int)(tx_speed / 1000U));
 	}
 }
 
@@ -1111,8 +1104,6 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
 		       dev->net->name, ctx->connected ? "" : "dis");
 
 		usbnet_link_change(dev, ctx->connected, 0);
-		if (!ctx->connected)
-			ctx->tx_speed = ctx->rx_speed = 0;
 		break;
 
 	case USB_CDC_NOTIFY_SPEED_CHANGE:
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 059dcc9..f14af3d 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -110,8 +110,6 @@ struct cdc_ncm_ctx {
 
 	u32 tx_timer_pending;
 	u32 tx_curr_frame_num;
-	u32 rx_speed;
-	u32 tx_speed;
 	u32 rx_max;
 	u32 tx_max;
 	u32 max_datagram_size;
-- 
1.7.10.4

[PATCH net-next 08/24] net: cdc_ncm: remove ncm_parm field

[Bjørn Mork]

Moving the call to cdc_ncm_setup() after the endpoint
setup removes the last remaining reference to ncm_parm
outside cdc_ncm_setup.

Collecting all the ncm_parm based calculations in
cdc_ncm_setup improves readability.

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c   |   46 +++++++++++++++++++++----------------------
 include/linux/usb/cdc_ncm.h |    1 -
 2 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index fc36a99..4de3a542 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -83,6 +83,7 @@ cdc_ncm_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *info)
 static u8 cdc_ncm_setup(struct usbnet *dev)
 {
 	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	struct usb_cdc_ncm_ntb_parameters ncm_parm;
 	u32 val;
 	u8 flags;
 	u8 iface_no;
@@ -97,22 +98,22 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 	err = usbnet_read_cmd(dev, USB_CDC_GET_NTB_PARAMETERS,
 			      USB_TYPE_CLASS | USB_DIR_IN
 			      |USB_RECIP_INTERFACE,
-			      0, iface_no, &ctx->ncm_parm,
-			      sizeof(ctx->ncm_parm));
+			      0, iface_no, &ncm_parm,
+			      sizeof(ncm_parm));
 	if (err < 0) {
 		pr_debug("failed GET_NTB_PARAMETERS\n");
 		return 1;
 	}
 
 	/* read correct set of parameters according to device mode */
-	ctx->rx_max = le32_to_cpu(ctx->ncm_parm.dwNtbInMaxSize);
-	ctx->tx_max = le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize);
-	ctx->tx_remainder = le16_to_cpu(ctx->ncm_parm.wNdpOutPayloadRemainder);
-	ctx->tx_modulus = le16_to_cpu(ctx->ncm_parm.wNdpOutDivisor);
-	ctx->tx_ndp_modulus = le16_to_cpu(ctx->ncm_parm.wNdpOutAlignment);
+	ctx->rx_max = le32_to_cpu(ncm_parm.dwNtbInMaxSize);
+	ctx->tx_max = le32_to_cpu(ncm_parm.dwNtbOutMaxSize);
+	ctx->tx_remainder = le16_to_cpu(ncm_parm.wNdpOutPayloadRemainder);
+	ctx->tx_modulus = le16_to_cpu(ncm_parm.wNdpOutDivisor);
+	ctx->tx_ndp_modulus = le16_to_cpu(ncm_parm.wNdpOutAlignment);
 	/* devices prior to NCM Errata shall set this field to zero */
-	ctx->tx_max_datagrams = le16_to_cpu(ctx->ncm_parm.wNtbOutMaxDatagrams);
-	ntb_fmt_supported = le16_to_cpu(ctx->ncm_parm.bmNtbFormatsSupported);
+	ctx->tx_max_datagrams = le16_to_cpu(ncm_parm.wNtbOutMaxDatagrams);
+	ntb_fmt_supported = le16_to_cpu(ncm_parm.bmNtbFormatsSupported);
 
 	eth_hlen = ETH_HLEN;
 	min_dgram_size = CDC_NCM_MIN_DATAGRAM_SIZE;
@@ -153,7 +154,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 	}
 
 	/* inform device about NTB input size changes */
-	if (ctx->rx_max != le32_to_cpu(ctx->ncm_parm.dwNtbInMaxSize)) {
+	if (ctx->rx_max != le32_to_cpu(ncm_parm.dwNtbInMaxSize)) {
 		__le32 dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
 
 		err = usbnet_write_cmd(dev, USB_CDC_SET_NTB_INPUT_SIZE,
@@ -171,6 +172,14 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 		pr_debug("Using default maximum transmit length=%d\n",
 						CDC_NCM_NTB_MAX_SIZE_TX);
 		ctx->tx_max = CDC_NCM_NTB_MAX_SIZE_TX;
+
+		/* Adding a pad byte here simplifies the handling in
+		 * cdc_ncm_fill_tx_frame, by making tx_max always
+		 * represent the real skb max size.
+		 */
+		if (ctx->tx_max % usb_maxpacket(dev->udev, dev->out, 1) == 0)
+			ctx->tx_max++;
+
 	}
 
 	/*
@@ -473,10 +482,6 @@ advance:
 	if (temp)
 		goto error2;
 
-	/* initialize data interface */
-	if (cdc_ncm_setup(dev))
-		goto error2;
-
 	/* configure data interface */
 	temp = usb_set_interface(dev->udev, iface_no, data_altsetting);
 	if (temp)
@@ -487,6 +492,10 @@ advance:
 	if (!dev->in || !dev->out || !dev->status)
 		goto error2;
 
+	/* initialize data interface */
+	if (cdc_ncm_setup(dev))
+		goto error2;
+
 	dev->net->ethtool_ops = &cdc_ncm_ethtool_ops;
 
 	usb_set_intfdata(ctx->data, dev);
@@ -501,15 +510,6 @@ advance:
 
 	dev->rx_urb_size = ctx->rx_max;
 
-	/* cdc_ncm_setup will override dwNtbOutMaxSize if it is
-	 * outside the sane range. Adding a pad byte here if necessary
-	 * simplifies the handling in cdc_ncm_fill_tx_frame, making
-	 * tx_max always represent the real skb max size.
-	 */
-	if (ctx->tx_max != le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize) &&
-	    ctx->tx_max % usb_maxpacket(dev->udev, dev->out, 1) == 0)
-		ctx->tx_max++;
-
 	return 0;
 
 error2:
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index f14af3d..89b52a0 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -88,7 +88,6 @@
 #define cdc_ncm_data_intf_is_mbim(x)  ((x)->desc.bInterfaceProtocol == USB_CDC_MBIM_PROTO_NTB)
 
 struct cdc_ncm_ctx {
-	struct usb_cdc_ncm_ntb_parameters ncm_parm;
 	struct hrtimer tx_timer;
 	struct tasklet_struct bh;
 
-- 
1.7.10.4

[PATCH net-next 09/24] net: cdc_ncm: fix SET_MAX_DATAGRAM_SIZE

[Bjørn Mork]

We need to inform the device about the *new* value, not the
old one.

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4de3a542..b8de8dd 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -280,6 +280,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 			/* if value changed, update device */
 			if (ctx->max_datagram_size !=
 					le16_to_cpu(max_datagram_size)) {
+				max_datagram_size = cpu_to_le16(ctx->max_datagram_size);
 				err = usbnet_write_cmd(dev,
 						USB_CDC_SET_MAX_DATAGRAM_SIZE,
 						USB_TYPE_CLASS | USB_DIR_OUT
-- 
1.7.10.4

[PATCH net-next 10/24] net: cdc_ncm: remove descriptor pointers

[Bjørn Mork]

header_desc was completely unused and union_desc was never used
outside cdc_ncm_bind_common.

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c   |   12 ++++++------
 include/linux/usb/cdc_ncm.h |    4 +---
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index b8de8dd..435fcc7 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -372,6 +372,7 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = {
 
 int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting)
 {
+	const struct usb_cdc_union_desc *union_desc = NULL;
 	struct cdc_ncm_ctx *ctx;
 	struct usb_driver *driver;
 	u8 *buf;
@@ -406,16 +407,15 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 
 		switch (buf[2]) {
 		case USB_CDC_UNION_TYPE:
-			if (buf[0] < sizeof(*(ctx->union_desc)))
+			if (buf[0] < sizeof(*union_desc))
 				break;
 
-			ctx->union_desc =
-					(const struct usb_cdc_union_desc *)buf;
+			union_desc = (const struct usb_cdc_union_desc *)buf;
 
 			ctx->control = usb_ifnum_to_if(dev->udev,
-					ctx->union_desc->bMasterInterface0);
+						       union_desc->bMasterInterface0);
 			ctx->data = usb_ifnum_to_if(dev->udev,
-					ctx->union_desc->bSlaveInterface0);
+						    union_desc->bSlaveInterface0);
 			break;
 
 		case USB_CDC_ETHERNET_TYPE:
@@ -458,7 +458,7 @@ advance:
 	}
 
 	/* some buggy devices have an IAD but no CDC Union */
-	if (!ctx->union_desc && intf->intf_assoc && intf->intf_assoc->bInterfaceCount == 2) {
+	if (!union_desc && intf->intf_assoc && intf->intf_assoc->bInterfaceCount == 2) {
 		ctx->control = intf;
 		ctx->data = usb_ifnum_to_if(dev->udev, intf->cur_altsetting->desc.bInterfaceNumber + 1);
 		dev_dbg(&intf->dev, "CDC Union missing - got slave from IAD\n");
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 89b52a0..cad54ad 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -92,9 +92,7 @@ struct cdc_ncm_ctx {
 	struct tasklet_struct bh;
 
 	const struct usb_cdc_ncm_desc *func_desc;
-	const struct usb_cdc_mbim_desc   *mbim_desc;
-	const struct usb_cdc_header_desc *header_desc;
-	const struct usb_cdc_union_desc *union_desc;
+	const struct usb_cdc_mbim_desc *mbim_desc;
 	const struct usb_cdc_ether_desc *ether_desc;
 
 	struct usb_interface *control;
-- 
1.7.10.4

[PATCH net-next 11/24] net: cdc_ncm: only the control intf can be probed

[Bjørn Mork]

The probed interface must be the master/control interface of the
function.  Make this explicit and simplify redundant tests.

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c |   14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 435fcc7..5aa3e60 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -394,6 +394,9 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 	/* store ctx pointer in device data field */
 	dev->data[0] = (unsigned long)ctx;
 
+	/* only the control interface can be successfully probed */
+	ctx->control = intf;
+
 	/* get some pointers */
 	driver = driver_of(intf);
 	buf = intf->cur_altsetting->extra;
@@ -411,9 +414,10 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 				break;
 
 			union_desc = (const struct usb_cdc_union_desc *)buf;
-
-			ctx->control = usb_ifnum_to_if(dev->udev,
-						       union_desc->bMasterInterface0);
+			/* the master must be the interface we are probing */
+			if (intf->cur_altsetting->desc.bInterfaceNumber !=
+			    union_desc->bMasterInterface0)
+				goto error;
 			ctx->data = usb_ifnum_to_if(dev->udev,
 						    union_desc->bSlaveInterface0);
 			break;
@@ -459,14 +463,12 @@ advance:
 
 	/* some buggy devices have an IAD but no CDC Union */
 	if (!union_desc && intf->intf_assoc && intf->intf_assoc->bInterfaceCount == 2) {
-		ctx->control = intf;
 		ctx->data = usb_ifnum_to_if(dev->udev, intf->cur_altsetting->desc.bInterfaceNumber + 1);
 		dev_dbg(&intf->dev, "CDC Union missing - got slave from IAD\n");
 	}
 
 	/* check if we got everything */
-	if ((ctx->control == NULL) || (ctx->data == NULL) ||
-	    ((!ctx->mbim_desc) && ((ctx->ether_desc == NULL) || (ctx->control != intf))))
+	if (!ctx->data || (!ctx->mbim_desc && !ctx->ether_desc))
 		goto error;
 
 	/* claim data interface, if different from control */
-- 
1.7.10.4

[PATCH net-next 12/24] net: cdc_ncm: no point in filling up the NTBs if we send ZLPs

[Bjørn Mork]

Padding NTBs to max size is part of the support for devices
optimizing their DMA transfers. This optimization depends on
max sized NTBs not being ZLP terminated. So we are much better
off dropping the padding if we are going to send a ZLP anyway.

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 5aa3e60..42c8620 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -800,8 +800,12 @@ cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign)
 	 * would be more efficient for USB HS mobile device with DMA
 	 * engine to receive a full size NTB, than canceling DMA
 	 * transfer and receiving a short packet.
+	 *
+	 * This optimization support is pointless if we end up sending
+	 * a ZLP after full sized NTBs.
 	 */
-	if (skb_out->len > CDC_NCM_MIN_TX_PKT)
+	if (!(dev->driver_info->flags & FLAG_SEND_ZLP) &&
+	    skb_out->len > CDC_NCM_MIN_TX_PKT)
 		memset(skb_put(skb_out, ctx->tx_max - skb_out->len), 0,
 		       ctx->tx_max - skb_out->len);
 	else if ((skb_out->len % dev->maxpacket) == 0)
-- 
1.7.10.4

[PATCH net-next 13/24] net: cdc_ncm: remove probe and disconnect wrappers

[Bjørn Mork]

These functions were merely wrappers around the usbnet
variants.  Remove them.

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c |   20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 42c8620..c90d843 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1141,22 +1141,6 @@ static int cdc_ncm_check_connect(struct usbnet *dev)
 	return !ctx->connected;
 }
 
-static int
-cdc_ncm_probe(struct usb_interface *udev, const struct usb_device_id *prod)
-{
-	return usbnet_probe(udev, prod);
-}
-
-static void cdc_ncm_disconnect(struct usb_interface *intf)
-{
-	struct usbnet *dev = usb_get_intfdata(intf);
-
-	if (dev == NULL)
-		return;		/* already disconnected */
-
-	usbnet_disconnect(intf);
-}
-
 static const struct driver_info cdc_ncm_info = {
 	.description = "CDC NCM",
 	.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET,
@@ -1267,8 +1251,8 @@ MODULE_DEVICE_TABLE(usb, cdc_devs);
 static struct usb_driver cdc_ncm_driver = {
 	.name = "cdc_ncm",
 	.id_table = cdc_devs,
-	.probe = cdc_ncm_probe,
-	.disconnect = cdc_ncm_disconnect,
+	.probe = usbnet_probe,
+	.disconnect = usbnet_disconnect,
 	.suspend = usbnet_suspend,
 	.resume = usbnet_resume,
 	.reset_resume =	usbnet_resume,
-- 
1.7.10.4

[PATCH net-next 14/24] net: cdc_ncm: remove ethtool ops

[Bjørn Mork]

No need to keep this code duplicated from usbnet.

Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/cdc_ncm.c |   26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index c90d843..8fc1a06 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -53,8 +53,6 @@
 #include <linux/usb/cdc.h>
 #include <linux/usb/cdc_ncm.h>
 
-#define	DRIVER_VERSION				"14-Mar-2012"
-
 #if IS_ENABLED(CONFIG_USB_NET_CDC_MBIM)
 static bool prefer_mbim = true;
 #else
@@ -68,18 +66,6 @@ static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
 static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
 static struct usb_driver cdc_ncm_driver;
 
-static void
-cdc_ncm_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *info)
-{
-	struct usbnet *dev = netdev_priv(net);
-
-	strlcpy(info->driver, dev->driver_name, sizeof(info->driver));
-	strlcpy(info->version, DRIVER_VERSION, sizeof(info->version));
-	strlcpy(info->fw_version, dev->driver_info->description,
-		sizeof(info->fw_version));
-	usb_make_path(dev->udev, info->bus_info, sizeof(info->bus_info));
-}
-
 static u8 cdc_ncm_setup(struct usbnet *dev)
 {
 	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
@@ -360,16 +346,6 @@ static void cdc_ncm_free(struct cdc_ncm_ctx *ctx)
 	kfree(ctx);
 }
 
-static const struct ethtool_ops cdc_ncm_ethtool_ops = {
-	.get_drvinfo = cdc_ncm_get_drvinfo,
-	.get_link = usbnet_get_link,
-	.get_msglevel = usbnet_get_msglevel,
-	.set_msglevel = usbnet_set_msglevel,
-	.get_settings = usbnet_get_settings,
-	.set_settings = usbnet_set_settings,
-	.nway_reset = usbnet_nway_reset,
-};
-
 int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting)
 {
 	const struct usb_cdc_union_desc *union_desc = NULL;
@@ -499,8 +475,6 @@ advance:
 	if (cdc_ncm_setup(dev))
 		goto error2;
 
-	dev->net->ethtool_ops = &cdc_ncm_ethtool_ops;

[PATCH net-next 15/24] net: cdc_ncm: set correct dev->hard_mtu

[Bjørn Mork]

usbnet use the hard_mtu value for sizing the tx queue and nothing
else.  We will be transmitting buffers of up to tx_max size, so
that's the proper value to give usbnet.

The individual datagram size is completely irrelevant here.

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 8fc1a06..c40f742 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -404,13 +404,6 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 
 			ctx->ether_desc =
 					(const struct usb_cdc_ether_desc *)buf;
-			dev->hard_mtu =
-				le16_to_cpu(ctx->ether_desc->wMaxSegmentSize);
-
-			if (dev->hard_mtu < CDC_NCM_MIN_DATAGRAM_SIZE)
-				dev->hard_mtu =	CDC_NCM_MIN_DATAGRAM_SIZE;
-			else if (dev->hard_mtu > CDC_NCM_MAX_DATAGRAM_SIZE)
-				dev->hard_mtu =	CDC_NCM_MAX_DATAGRAM_SIZE;
 			break;
 
 		case USB_CDC_NCM_TYPE:
@@ -485,6 +478,8 @@ advance:
 		dev_info(&dev->udev->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
 	}
 
+	/* usbnet use these values for sizing tx/rx queues */
+	dev->hard_mtu = ctx->tx_max;
 	dev->rx_urb_size = ctx->rx_max;
 
 	return 0;
-- 
1.7.10.4

[PATCH net-next 16/24] net: cdc_ncm: log the length we warn about

[Bjørn Mork]

Fix cut'n'paste typo.  Log the bogus length and not the
irrelevant signature.

Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/cdc_ncm.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index c40f742..b5fdf8f 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -923,7 +923,7 @@ int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset)
 
 	if (le16_to_cpu(ndp16->wLength) < USB_CDC_NCM_NDP16_LENGTH_MIN) {
 		pr_debug("invalid DPT16 length <%u>\n",
-					le32_to_cpu(ndp16->dwSignature));
+			 le16_to_cpu(ndp16->wLength));
 		goto error;
 	}
 
-- 
1.7.10.4

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

[PATCH net-next 17/24] net: cdc_ncm: use netif_* and dev_* instead of pr_*

[Bjørn Mork]

Take advantage of standard device name prefixing and
netdevice msglvl control where possible.

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c |   98 ++++++++++++++++++++++-----------------------
 1 file changed, 48 insertions(+), 50 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index b5fdf8f..73faf05 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -87,7 +87,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 			      0, iface_no, &ncm_parm,
 			      sizeof(ncm_parm));
 	if (err < 0) {
-		pr_debug("failed GET_NTB_PARAMETERS\n");
+		dev_dbg(&dev->intf->dev, "failed GET_NTB_PARAMETERS\n");
 		return 1;
 	}
 
@@ -115,11 +115,10 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 		flags = 0;
 	}
 
-	pr_debug("dwNtbInMaxSize=%u dwNtbOutMaxSize=%u "
-		 "wNdpOutPayloadRemainder=%u wNdpOutDivisor=%u "
-		 "wNdpOutAlignment=%u wNtbOutMaxDatagrams=%u flags=0x%x\n",
-		 ctx->rx_max, ctx->tx_max, ctx->tx_remainder, ctx->tx_modulus,
-		 ctx->tx_ndp_modulus, ctx->tx_max_datagrams, flags);
+	dev_dbg(&dev->intf->dev,
+		"dwNtbInMaxSize=%u dwNtbOutMaxSize=%u wNdpOutPayloadRemainder=%u wNdpOutDivisor=%u wNdpOutAlignment=%u wNtbOutMaxDatagrams=%u flags=0x%x\n",
+		ctx->rx_max, ctx->tx_max, ctx->tx_remainder, ctx->tx_modulus,
+		ctx->tx_ndp_modulus, ctx->tx_max_datagrams, flags);
 
 	/* max count of tx datagrams */
 	if ((ctx->tx_max_datagrams == 0) ||
@@ -128,14 +127,14 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 
 	/* verify maximum size of received NTB in bytes */
 	if (ctx->rx_max < USB_CDC_NCM_NTB_MIN_IN_SIZE) {
-		pr_debug("Using min receive length=%d\n",
-						USB_CDC_NCM_NTB_MIN_IN_SIZE);
+		dev_dbg(&dev->intf->dev, "Using min receive length=%d\n",
+			USB_CDC_NCM_NTB_MIN_IN_SIZE);
 		ctx->rx_max = USB_CDC_NCM_NTB_MIN_IN_SIZE;
 	}
 
 	if (ctx->rx_max > CDC_NCM_NTB_MAX_SIZE_RX) {
-		pr_debug("Using default maximum receive length=%d\n",
-						CDC_NCM_NTB_MAX_SIZE_RX);
+		dev_dbg(&dev->intf->dev, "Using default maximum receive length=%d\n",
+			CDC_NCM_NTB_MAX_SIZE_RX);
 		ctx->rx_max = CDC_NCM_NTB_MAX_SIZE_RX;
 	}
 
@@ -148,15 +147,15 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 				       | USB_RECIP_INTERFACE,
 				       0, iface_no, &dwNtbInMaxSize, 4);
 		if (err < 0)
-			pr_debug("Setting NTB Input Size failed\n");
+			dev_dbg(&dev->intf->dev, "Setting NTB Input Size failed\n");
 	}
 
 	/* verify maximum size of transmitted NTB in bytes */
 	if ((ctx->tx_max <
 	    (min_hdr_size + min_dgram_size)) ||
 	    (ctx->tx_max > CDC_NCM_NTB_MAX_SIZE_TX)) {
-		pr_debug("Using default maximum transmit length=%d\n",
-						CDC_NCM_NTB_MAX_SIZE_TX);
+		dev_dbg(&dev->intf->dev, "Using default maximum transmit length=%d\n",
+			CDC_NCM_NTB_MAX_SIZE_TX);
 		ctx->tx_max = CDC_NCM_NTB_MAX_SIZE_TX;
 
 		/* Adding a pad byte here simplifies the handling in
@@ -178,7 +177,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 
 	if ((val < USB_CDC_NCM_NDP_ALIGN_MIN_SIZE) ||
 	    (val != ((-val) & val)) || (val >= ctx->tx_max)) {
-		pr_debug("Using default alignment: 4 bytes\n");
+		dev_dbg(&dev->intf->dev, "Using default alignment: 4 bytes\n");
 		ctx->tx_ndp_modulus = USB_CDC_NCM_NDP_ALIGN_MIN_SIZE;
 	}
 
@@ -192,13 +191,13 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 
 	if ((val < USB_CDC_NCM_NDP_ALIGN_MIN_SIZE) ||
 	    (val != ((-val) & val)) || (val >= ctx->tx_max)) {
-		pr_debug("Using default transmit modulus: 4 bytes\n");
+		dev_dbg(&dev->intf->dev, "Using default transmit modulus: 4 bytes\n");
 		ctx->tx_modulus = USB_CDC_NCM_NDP_ALIGN_MIN_SIZE;
 	}
 
 	/* verify the payload remainder */
 	if (ctx->tx_remainder >= ctx->tx_modulus) {
-		pr_debug("Using default transmit remainder: 0 bytes\n");
+		dev_dbg(&dev->intf->dev, "Using default transmit remainder: 0 bytes\n");
 		ctx->tx_remainder = 0;
 	}
 
@@ -216,7 +215,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 				       USB_CDC_NCM_CRC_NOT_APPENDED,
 				       iface_no, NULL, 0);
 		if (err < 0)
-			pr_debug("Setting CRC mode off failed\n");
+			dev_dbg(&dev->intf->dev, "Setting CRC mode off failed\n");
 	}
 
 	/* set NTB format, if both formats are supported */
@@ -227,7 +226,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 				       USB_CDC_NCM_NTB16_FORMAT,
 				       iface_no, NULL, 0);
 		if (err < 0)
-			pr_debug("Setting NTB format to 16-bit failed\n");
+			dev_dbg(&dev->intf->dev, "Setting NTB format to 16-bit failed\n");
 	}
 
 	ctx->max_datagram_size = min_dgram_size;
@@ -248,8 +247,8 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 				      | USB_RECIP_INTERFACE,
 				      0, iface_no, &max_datagram_size, 2);
 		if (err < 0) {
-			pr_debug("GET_MAX_DATAGRAM_SIZE failed, use size=%u\n",
-				 min_dgram_size);
+			dev_dbg(&dev->intf->dev, "GET_MAX_DATAGRAM_SIZE failed, use size=%u\n",
+				min_dgram_size);
 		} else {
 			ctx->max_datagram_size =
 				le16_to_cpu(max_datagram_size);
@@ -275,7 +274,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 						iface_no, &max_datagram_size,
 						2);
 				if (err < 0)
-					pr_debug("SET_MAX_DGRAM_SIZE failed\n");
+					dev_dbg(&dev->intf->dev, "SET_MAX_DGRAM_SIZE failed\n");
 			}
 		}
 	}
@@ -867,6 +866,7 @@ error:
 /* 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)
 {
+	struct usbnet *dev = netdev_priv(skb_in->dev);
 	struct usb_cdc_ncm_nth16 *nth16;
 	int len;
 	int ret = -EINVAL;
@@ -876,7 +876,7 @@ int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
 
 	if (skb_in->len < (sizeof(struct usb_cdc_ncm_nth16) +
 					sizeof(struct usb_cdc_ncm_ndp16))) {
-		pr_debug("frame too short\n");
+		netif_dbg(dev, rx_err, dev->net, "frame too short\n");
 		goto error;
 	}
 
@@ -890,16 +890,18 @@ int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
 
 	len = le16_to_cpu(nth16->wBlockLength);
 	if (len > ctx->rx_max) {
-		pr_debug("unsupported NTB block length %u/%u\n", len,
-								ctx->rx_max);
+		netif_dbg(dev, rx_err, dev->net,
+			  "unsupported NTB block length %u/%u\n", len,
+			  ctx->rx_max);
 		goto error;
 	}
 
 	if ((ctx->rx_seq + 1) != le16_to_cpu(nth16->wSequence) &&
-		(ctx->rx_seq || le16_to_cpu(nth16->wSequence)) &&
-		!((ctx->rx_seq == 0xffff) && !le16_to_cpu(nth16->wSequence))) {
-		pr_debug("sequence number glitch prev=%d curr=%d\n",
-				ctx->rx_seq, le16_to_cpu(nth16->wSequence));
+	    (ctx->rx_seq || le16_to_cpu(nth16->wSequence)) &&
+	    !((ctx->rx_seq == 0xffff) && !le16_to_cpu(nth16->wSequence))) {
+		netif_dbg(dev, rx_err, dev->net,
+			  "sequence number glitch prev=%d curr=%d\n",
+			  ctx->rx_seq, le16_to_cpu(nth16->wSequence));
 	}
 	ctx->rx_seq = le16_to_cpu(nth16->wSequence);
 
@@ -912,18 +914,20 @@ EXPORT_SYMBOL_GPL(cdc_ncm_rx_verify_nth16);
 /* verify NDP header and return number of datagrams, or negative error */
 int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset)
 {
+	struct usbnet *dev = netdev_priv(skb_in->dev);
 	struct usb_cdc_ncm_ndp16 *ndp16;
 	int ret = -EINVAL;
 
 	if ((ndpoffset + sizeof(struct usb_cdc_ncm_ndp16)) > skb_in->len) {
-		pr_debug("invalid NDP offset  <%u>\n", ndpoffset);
+		netif_dbg(dev, rx_err, dev->net, "invalid NDP offset  <%u>\n",
+			  ndpoffset);
 		goto error;
 	}
 	ndp16 = (struct usb_cdc_ncm_ndp16 *)(skb_in->data + ndpoffset);
 
 	if (le16_to_cpu(ndp16->wLength) < USB_CDC_NCM_NDP16_LENGTH_MIN) {
-		pr_debug("invalid DPT16 length <%u>\n",
-			 le16_to_cpu(ndp16->wLength));
+		netif_dbg(dev, rx_err, dev->net, "invalid DPT16 length <%u>\n",
+			  le16_to_cpu(ndp16->wLength));
 		goto error;
 	}
 
@@ -932,9 +936,9 @@ int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset)
 					sizeof(struct usb_cdc_ncm_dpe16));
 	ret--; /* we process NDP entries except for the last one */
 
-	if ((sizeof(struct usb_cdc_ncm_ndp16) + ret * (sizeof(struct usb_cdc_ncm_dpe16))) >
-								skb_in->len) {
-		pr_debug("Invalid nframes = %d\n", ret);
+	if ((sizeof(struct usb_cdc_ncm_ndp16) +
+	     ret * (sizeof(struct usb_cdc_ncm_dpe16))) > skb_in->len) {
+		netif_dbg(dev, rx_err, dev->net, "Invalid nframes = %d\n", ret);
 		ret = -EINVAL;
 	}
 
@@ -991,9 +995,9 @@ next_ndp:
 		/* sanity checking */
 		if (((offset + len) > skb_in->len) ||
 				(len > ctx->rx_max) || (len < ETH_HLEN)) {
-			pr_debug("invalid frame detected (ignored)"
-					"offset[%u]=%u, length=%u, skb=%p\n",
-					x, offset, len, skb_in);
+			netif_dbg(dev, rx_err, dev->net,
+				  "invalid frame detected (ignored) offset[%u]=%u, length=%u, skb=%p\n",
+				  x, offset, len, skb_in);
 			if (!x)
 				goto err_ndp;
 			break;
@@ -1031,17 +1035,13 @@ cdc_ncm_speed_change(struct usbnet *dev,
 	 * device speed. Do print it instead.
 	 */
 	if ((tx_speed > 1000000) && (rx_speed > 1000000)) {
-		printk(KERN_INFO KBUILD_MODNAME
-		       ": %s: %u mbit/s downlink "
-		       "%u mbit/s uplink\n",
-		       dev->net->name,
+		netif_info(dev, link, dev->net,
+		       "%u mbit/s downlink %u mbit/s uplink\n",
 		       (unsigned int)(rx_speed / 1000000U),
 		       (unsigned int)(tx_speed / 1000000U));
 	} else {
-		printk(KERN_INFO KBUILD_MODNAME
-		       ": %s: %u kbit/s downlink "
-		       "%u kbit/s uplink\n",
-		       dev->net->name,
+		netif_info(dev, link, dev->net,
+		       "%u kbit/s downlink %u kbit/s uplink\n",
 		       (unsigned int)(rx_speed / 1000U),
 		       (unsigned int)(tx_speed / 1000U));
 	}
@@ -1074,11 +1074,9 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
 		 * sent by device after USB_CDC_NOTIFY_SPEED_CHANGE.
 		 */
 		ctx->connected = le16_to_cpu(event->wValue);
-
-		printk(KERN_INFO KBUILD_MODNAME ": %s: network connection:"
-		       " %sconnected\n",
-		       dev->net->name, ctx->connected ? "" : "dis");
-
+		netif_info(dev, link, dev->net,
+			   "network connection: %sconnected\n",
+			   ctx->connected ? "" : "dis");
 		usbnet_link_change(dev, ctx->connected, 0);
 		break;
 
-- 
1.7.10.4

[PATCH net-next 18/24] net: cdc_ncm: log signatures in hex

[Bjørn Mork]

These signatures are well known bit patterns, mostly made up
of ascii characters.  Mentally parsing works best if they
are printed in hex.

Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/cdc_ncm.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 73faf05..d49e4c5 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -883,8 +883,9 @@ int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
 	nth16 = (struct usb_cdc_ncm_nth16 *)skb_in->data;
 
 	if (le32_to_cpu(nth16->dwSignature) != USB_CDC_NCM_NTH16_SIGN) {
-		pr_debug("invalid NTH16 signature <%u>\n",
-					le32_to_cpu(nth16->dwSignature));
+		netif_dbg(dev, rx_err, dev->net,
+			  "invalid NTH16 signature <%#010x>\n",
+			  le32_to_cpu(nth16->dwSignature));
 		goto error;
 	}
 
@@ -972,8 +973,9 @@ next_ndp:
 	ndp16 = (struct usb_cdc_ncm_ndp16 *)(skb_in->data + ndpoffset);
 
 	if (le32_to_cpu(ndp16->dwSignature) != USB_CDC_NCM_NDP16_NOCRC_SIGN) {
-		pr_debug("invalid DPT16 signature <%u>\n",
-			 le32_to_cpu(ndp16->dwSignature));
+		netif_dbg(dev, rx_err, dev->net,
+			  "invalid DPT16 signature <%#010x>\n",
+			  le32_to_cpu(ndp16->dwSignature));
 		goto err_ndp;
 	}
 	dpe16 = ndp16->dpe16;
-- 
1.7.10.4

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

[PATCH net-next 19/24] net: cdc_ncm: endian convert constants instead of variables

[Bjørn Mork]

Converting the constants used in these comparisons at build
time instead of converting the variables for every received
frame at run time.

Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/cdc_ncm.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index d49e4c5..83e6d5b 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -882,7 +882,7 @@ int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in)
 
 	nth16 = (struct usb_cdc_ncm_nth16 *)skb_in->data;
 
-	if (le32_to_cpu(nth16->dwSignature) != USB_CDC_NCM_NTH16_SIGN) {
+	if (nth16->dwSignature != cpu_to_le32(USB_CDC_NCM_NTH16_SIGN)) {
 		netif_dbg(dev, rx_err, dev->net,
 			  "invalid NTH16 signature <%#010x>\n",
 			  le32_to_cpu(nth16->dwSignature));
@@ -972,7 +972,7 @@ next_ndp:
 
 	ndp16 = (struct usb_cdc_ncm_ndp16 *)(skb_in->data + ndpoffset);
 
-	if (le32_to_cpu(ndp16->dwSignature) != USB_CDC_NCM_NDP16_NOCRC_SIGN) {
+	if (ndp16->dwSignature != cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN)) {
 		netif_dbg(dev, rx_err, dev->net,
 			  "invalid DPT16 signature <%#010x>\n",
 			  le32_to_cpu(ndp16->dwSignature));
-- 
1.7.10.4

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

[PATCH net-next 20/24] net: cdc_ncm: drop "extern" from header declarations

[Bjørn Mork]

Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 include/linux/usb/cdc_ncm.h |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index cad54ad..2300f74 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -119,11 +119,11 @@ struct cdc_ncm_ctx {
 	u16 connected;
 };
 
-extern u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface *intf);
-extern int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting);
-extern void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
-extern struct sk_buff *cdc_ncm_fill_tx_frame(struct usbnet *dev, 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);
+u8 cdc_ncm_select_altsetting(struct usbnet *dev, struct usb_interface *intf);
+int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_altsetting);
+void cdc_ncm_unbind(struct usbnet *dev, struct usb_interface *intf);
+struct sk_buff *cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign);
+int cdc_ncm_rx_verify_nth16(struct cdc_ncm_ctx *ctx, struct sk_buff *skb_in);
+int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset);
 
 #endif /* __LINUX_USB_CDC_NCM_H */
-- 
1.7.10.4

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

[PATCH net-next 21/24] net: cdc_ncm: refactoring cdc_ncm_setup

[Bjørn Mork]

Rewriting the "set max datagram" part of dc_ncm_setup to
separate the selection and validatation of the size from
the code which optionally informs the device of this
value.  This ensures that we use the correct value
regardless of device support for the get and set commands.

Removing some of the many indent levels while doing this
to make the code more readable.

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c |  108 ++++++++++++++++++---------------------------
 1 file changed, 44 insertions(+), 64 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 83e6d5b..62dcb2e 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -76,8 +76,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 	int err;
 	int eth_hlen;
 	u16 ntb_fmt_supported;
-	u32 min_dgram_size;
-	u32 min_hdr_size;
+	__le16 max_datagram_size;
 
 	iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
 
@@ -101,20 +100,29 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 	ctx->tx_max_datagrams = le16_to_cpu(ncm_parm.wNtbOutMaxDatagrams);
 	ntb_fmt_supported = le16_to_cpu(ncm_parm.bmNtbFormatsSupported);
 
-	eth_hlen = ETH_HLEN;
-	min_dgram_size = CDC_NCM_MIN_DATAGRAM_SIZE;
-	min_hdr_size = CDC_NCM_MIN_HDR_SIZE;
-	if (ctx->mbim_desc != NULL) {
-		flags = ctx->mbim_desc->bmNetworkCapabilities;
+	/* there are some minor differences in NCM and MBIM defaults */
+	if (cdc_ncm_comm_intf_is_mbim(ctx->control->cur_altsetting)) {
+		if (!ctx->mbim_desc)
+			return -EINVAL;
 		eth_hlen = 0;
-		min_dgram_size = CDC_MBIM_MIN_DATAGRAM_SIZE;
-		min_hdr_size = 0;
-	} else if (ctx->func_desc != NULL) {
-		flags = ctx->func_desc->bmNetworkCapabilities;
+		flags = ctx->mbim_desc->bmNetworkCapabilities;
+		ctx->max_datagram_size = le16_to_cpu(ctx->mbim_desc->wMaxSegmentSize);
+		if (ctx->max_datagram_size < CDC_MBIM_MIN_DATAGRAM_SIZE)
+			ctx->max_datagram_size = CDC_MBIM_MIN_DATAGRAM_SIZE;
 	} else {
-		flags = 0;
+		if (!ctx->func_desc)
+			return -EINVAL;
+		eth_hlen = ETH_HLEN;
+		flags = ctx->func_desc->bmNetworkCapabilities;
+		ctx->max_datagram_size = le16_to_cpu(ctx->ether_desc->wMaxSegmentSize);
+		if (ctx->max_datagram_size < CDC_NCM_MIN_DATAGRAM_SIZE)
+			ctx->max_datagram_size = CDC_NCM_MIN_DATAGRAM_SIZE;
 	}
 
+	/* common absolute max for NCM and MBIM */
+	if (ctx->max_datagram_size > CDC_NCM_MAX_DATAGRAM_SIZE)
+		ctx->max_datagram_size = CDC_NCM_MAX_DATAGRAM_SIZE;
+
 	dev_dbg(&dev->intf->dev,
 		"dwNtbInMaxSize=%u dwNtbOutMaxSize=%u wNdpOutPayloadRemainder=%u wNdpOutDivisor=%u wNdpOutAlignment=%u wNtbOutMaxDatagrams=%u flags=0x%x\n",
 		ctx->rx_max, ctx->tx_max, ctx->tx_remainder, ctx->tx_modulus,
@@ -151,8 +159,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 	}
 
 	/* verify maximum size of transmitted NTB in bytes */
-	if ((ctx->tx_max <
-	    (min_hdr_size + min_dgram_size)) ||
+	if ((ctx->tx_max < (CDC_NCM_MIN_HDR_SIZE + ctx->max_datagram_size)) ||
 	    (ctx->tx_max > CDC_NCM_NTB_MAX_SIZE_TX)) {
 		dev_dbg(&dev->intf->dev, "Using default maximum transmit length=%d\n",
 			CDC_NCM_NTB_MAX_SIZE_TX);
@@ -229,60 +236,33 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 			dev_dbg(&dev->intf->dev, "Setting NTB format to 16-bit failed\n");
 	}
 
-	ctx->max_datagram_size = min_dgram_size;
+	/* inform the device about the selected Max Datagram Size */
+	if (!(flags & USB_CDC_NCM_NCAP_MAX_DATAGRAM_SIZE))
+		goto out;
 
-	/* set Max Datagram Size (MTU) */
-	if (flags & USB_CDC_NCM_NCAP_MAX_DATAGRAM_SIZE) {
-		__le16 max_datagram_size;
-		u16 eth_max_sz;
-		if (ctx->ether_desc != NULL)
-			eth_max_sz = le16_to_cpu(ctx->ether_desc->wMaxSegmentSize);
-		else if (ctx->mbim_desc != NULL)
-			eth_max_sz = le16_to_cpu(ctx->mbim_desc->wMaxSegmentSize);
-		else
-			goto max_dgram_err;
-
-		err = usbnet_read_cmd(dev, USB_CDC_GET_MAX_DATAGRAM_SIZE,
-				      USB_TYPE_CLASS | USB_DIR_IN
-				      | USB_RECIP_INTERFACE,
-				      0, iface_no, &max_datagram_size, 2);
-		if (err < 0) {
-			dev_dbg(&dev->intf->dev, "GET_MAX_DATAGRAM_SIZE failed, use size=%u\n",
-				min_dgram_size);
-		} else {
-			ctx->max_datagram_size =
-				le16_to_cpu(max_datagram_size);
-			/* Check Eth descriptor value */
-			if (ctx->max_datagram_size > eth_max_sz)
-					ctx->max_datagram_size = eth_max_sz;
-
-			if (ctx->max_datagram_size > CDC_NCM_MAX_DATAGRAM_SIZE)
-				ctx->max_datagram_size = CDC_NCM_MAX_DATAGRAM_SIZE;
-
-			if (ctx->max_datagram_size < min_dgram_size)
-				ctx->max_datagram_size = min_dgram_size;
-
-			/* if value changed, update device */
-			if (ctx->max_datagram_size !=
-					le16_to_cpu(max_datagram_size)) {
-				max_datagram_size = cpu_to_le16(ctx->max_datagram_size);
-				err = usbnet_write_cmd(dev,
-						USB_CDC_SET_MAX_DATAGRAM_SIZE,
-						USB_TYPE_CLASS | USB_DIR_OUT
-						 | USB_RECIP_INTERFACE,
-						0,
-						iface_no, &max_datagram_size,
-						2);
-				if (err < 0)
-					dev_dbg(&dev->intf->dev, "SET_MAX_DGRAM_SIZE failed\n");
-			}
-		}
+	/* read current mtu value from device */
+	err = usbnet_read_cmd(dev, USB_CDC_GET_MAX_DATAGRAM_SIZE,
+			      USB_TYPE_CLASS | USB_DIR_IN | USB_RECIP_INTERFACE,
+			      0, iface_no, &max_datagram_size, 2);
+	if (err < 0) {
+		dev_dbg(&dev->intf->dev, "GET_MAX_DATAGRAM_SIZE failed\n");
+		goto out;
 	}
 
-max_dgram_err:
-	if (dev->net->mtu != (ctx->max_datagram_size - eth_hlen))
-		dev->net->mtu = ctx->max_datagram_size - eth_hlen;
+	if (le16_to_cpu(max_datagram_size) == ctx->max_datagram_size)
+		goto out;
+
+	max_datagram_size = cpu_to_le16(ctx->max_datagram_size);
+	err = usbnet_write_cmd(dev, USB_CDC_SET_MAX_DATAGRAM_SIZE,
+			       USB_TYPE_CLASS | USB_DIR_OUT | USB_RECIP_INTERFACE,
+			       0, iface_no, &max_datagram_size, 2);
+	if (err < 0)
+		dev_dbg(&dev->intf->dev, "SET_MAX_DATAGRAM_SIZE failed\n");
 
+out:
+	/* set MTU to max supported by the device if necessary */
+	if (dev->net->mtu > ctx->max_datagram_size - eth_hlen)
+		dev->net->mtu = ctx->max_datagram_size - eth_hlen;
 	return 0;
 }
 
-- 
1.7.10.4

[PATCH net-next 22/24] net: cdc_ncm: return proper error if setup fails

[Bjørn Mork]

Most setup errors are ignored to ensure maximum firmware
compatibilty.  But GET_NTB_PARAMETERS and the functional
descriptors are required.  Use proper error codes and
log level if these fail.

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 62dcb2e..f168bc8 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -86,8 +86,8 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 			      0, iface_no, &ncm_parm,
 			      sizeof(ncm_parm));
 	if (err < 0) {
-		dev_dbg(&dev->intf->dev, "failed GET_NTB_PARAMETERS\n");
-		return 1;
+		dev_err(&dev->intf->dev, "failed GET_NTB_PARAMETERS\n");
+		return err; /* GET_NTB_PARAMETERS is required */
 	}
 
 	/* read correct set of parameters according to device mode */
-- 
1.7.10.4

[PATCH net-next 23/24] net: cdc_ncm: improve bind error debug messages

[Bjørn Mork]

Make it a bit easier for users to figure out what goes
wrong when bind fails.

Cc: Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
---
 drivers/net/usb/cdc_ncm.c |   36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index f168bc8..4531f38 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -371,8 +371,10 @@ int cdc_ncm_bind_common(struct usbnet *dev, struct usb_interface *intf, u8 data_
 			union_desc = (const struct usb_cdc_union_desc *)buf;
 			/* the master must be the interface we are probing */
 			if (intf->cur_altsetting->desc.bInterfaceNumber !=
-			    union_desc->bMasterInterface0)
+			    union_desc->bMasterInterface0) {
+				dev_dbg(&intf->dev, "bogus CDC Union\n");
 				goto error;
+			}
 			ctx->data = usb_ifnum_to_if(dev->udev,
 						    union_desc->bSlaveInterface0);
 			break;
@@ -416,45 +418,59 @@ advance:
 	}
 
 	/* check if we got everything */
-	if (!ctx->data || (!ctx->mbim_desc && !ctx->ether_desc))
+	if (!ctx->data || (!ctx->mbim_desc && !ctx->ether_desc)) {
+		dev_dbg(&intf->dev, "CDC descriptors missing\n");
 		goto error;
+	}
 
 	/* claim data interface, if different from control */
 	if (ctx->data != ctx->control) {
 		temp = usb_driver_claim_interface(driver, ctx->data, dev);
-		if (temp)
+		if (temp) {
+			dev_dbg(&intf->dev, "failed to claim data intf\n");
 			goto error;
+		}
 	}
 
 	iface_no = ctx->data->cur_altsetting->desc.bInterfaceNumber;
 
 	/* reset data interface */
 	temp = usb_set_interface(dev->udev, iface_no, 0);
-	if (temp)
+	if (temp) {
+		dev_dbg(&intf->dev, "set interface failed\n");
 		goto error2;
+	}
 
 	/* configure data interface */
 	temp = usb_set_interface(dev->udev, iface_no, data_altsetting);
-	if (temp)
+	if (temp) {
+		dev_dbg(&intf->dev, "set interface failed\n");
 		goto error2;
+	}
 
 	cdc_ncm_find_endpoints(dev, ctx->data);
 	cdc_ncm_find_endpoints(dev, ctx->control);
-	if (!dev->in || !dev->out || !dev->status)
+	if (!dev->in || !dev->out || !dev->status) {
+		dev_dbg(&intf->dev, "failed to collect endpoints\n");
 		goto error2;
+	}
 
 	/* initialize data interface */
-	if (cdc_ncm_setup(dev))
+	if (cdc_ncm_setup(dev))	{
+		dev_dbg(&intf->dev, "cdc_ncm_setup() failed\n");
 		goto error2;
+	}
 
 	usb_set_intfdata(ctx->data, dev);
 	usb_set_intfdata(ctx->control, dev);
 
 	if (ctx->ether_desc) {
 		temp = usbnet_get_ethernet_addr(dev, ctx->ether_desc->iMACAddress);
-		if (temp)
+		if (temp) {
+			dev_dbg(&intf->dev, "failed to get mac address\n");
 			goto error2;
-		dev_info(&dev->udev->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
+		}
+		dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
 	}
 
 	/* usbnet use these values for sizing tx/rx queues */
@@ -471,7 +487,7 @@ error2:
 error:
 	cdc_ncm_free((struct cdc_ncm_ctx *)dev->data[0]);
 	dev->data[0] = 0;
-	dev_info(&dev->udev->dev, "bind() failure\n");
+	dev_info(&intf->dev, "bind() failure\n");
 	return -ENODEV;
 }
 EXPORT_SYMBOL_GPL(cdc_ncm_bind_common);
-- 
1.7.10.4

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

[PATCH net-next 24/24] net: cdc_ncm: no not set tx_max higher than the device supports

[Bjørn Mork]

There are MBIM devices out there reporting

  dwNtbInMaxSize=2048 dwNtbOutMaxSize=2048

and since the spec require a datagram max size of at least
2048, this means that a full sized datagram will never fit.

Still, sending larger NTBs than the device supports is not
going to help.  We do not have any other options than either
 a) refusing to bindi, or
 b) respect the insanely low value.

Alternative b will at least make these devices work, so go
for it.

Cc: Alexey Orishko <alexey.orishko@gmail.com>
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4531f38..11c7033 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -159,8 +159,7 @@ static u8 cdc_ncm_setup(struct usbnet *dev)
 	}
 
 	/* verify maximum size of transmitted NTB in bytes */
-	if ((ctx->tx_max < (CDC_NCM_MIN_HDR_SIZE + ctx->max_datagram_size)) ||
-	    (ctx->tx_max > CDC_NCM_NTB_MAX_SIZE_TX)) {
+	if (ctx->tx_max > CDC_NCM_NTB_MAX_SIZE_TX) {
 		dev_dbg(&dev->intf->dev, "Using default maximum transmit length=%d\n",
 			CDC_NCM_NTB_MAX_SIZE_TX);
 		ctx->tx_max = CDC_NCM_NTB_MAX_SIZE_TX;
-- 
1.7.10.4

Re: [PATCH net-next 17/24] net: cdc_ncm: use netif_* and dev_* instead of pr_*

[Joe Perches]

On Fri, 2013-11-01 at 11:16 +0100, Bjørn Mork wrote:
> Take advantage of standard device name prefixing and
> netdevice msglvl control where possible.

Nice, thanks.

You did most all the multi-line statement
alignment perfectly but missed a couple.

Maybe in a follow-on patch.

> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
[]
> @@ -1031,17 +1035,13 @@ cdc_ncm_speed_change(struct usbnet *dev,
>  	 * device speed. Do print it instead.
>  	 */
>  	if ((tx_speed > 1000000) && (rx_speed > 1000000)) {
> -		printk(KERN_INFO KBUILD_MODNAME
> -		       ": %s: %u mbit/s downlink "
> -		       "%u mbit/s uplink\n",
> -		       dev->net->name,
> +		netif_info(dev, link, dev->net,
> +		       "%u mbit/s downlink %u mbit/s uplink\n",
>  		       (unsigned int)(rx_speed / 1000000U),
>  		       (unsigned int)(tx_speed / 1000000U));
>  	} else {
> -		printk(KERN_INFO KBUILD_MODNAME
> -		       ": %s: %u kbit/s downlink "
> -		       "%u kbit/s uplink\n",
> -		       dev->net->name,
> +		netif_info(dev, link, dev->net,
> +		       "%u kbit/s downlink %u kbit/s uplink\n",
>  		       (unsigned int)(rx_speed / 1000U),
>  		       (unsigned int)(tx_speed / 1000U));
>  	}

Re: [PATCH net-next 00/24] cdc_ncm: many small and mostly trivial fixes

[David Miller]

From: Bjørn Mork <bjorn@mork.no>
Date: Fri,  1 Nov 2013 11:16:37 +0100

> This series ended up longer than expected, and it is still not
> complete. There is more to come when time allows...
> 
> Most changes are trivial. Notable non-trivial changes are
>  - removed filtering of identical speed notifications
>  - tx_max calulation is changed to count the pad byte if
>    necessary, and respect the device limit as an absolute
>    upper limit even if it is too low according to the spec
>  - remove the bug preventing SET_MAX_DATAGRAM_SIZE from having
>    any effect
>  - drop the pad-to-max if ZLPs are enabled
>  - the driver specific VERSION is dropped
>  - dev->hard_mtu is set to tx_max instead of max_datagram_size
>    causing usbnet to calculate the qlen based on the real max
>    size of tx skbs
> 
> This series has been tested, along with the previously posted
> cdc_mbim series, on the NCM and MBIM devices I have:
>  - Ericsson F5521gw (NCM)
>  - Huawei E367 (MBIM)
>  - D-Link DWM-156 A7 (MBIM w/ too low dwNtb{In,Out}MaxSize bug)
>  - Sierra Wireless MC7710 (MBIM w/ ZLP and CDC Union bugs)
> 
> Apart from the D-Link modem dropping a lot less oversized
> frames with the fix dedicated to it, there are no end user
> noticable functional changes as a result of this series.  But
> all the non-trivial changes I listed above are of course
> detectable by users looking at that specific area (except maybe
> the removed speed notification, which requires a device sending
> duplicates to be noticable - I don't have any such device).

Looks good, series applied, thanks!

Re: [PATCH net-next 17/24] net: cdc_ncm: use netif_* and dev_* instead of pr_*

[Bjørn Mork]

Joe Perches <joe@perches.com> writes:

> You did most all the multi-line statement
> alignment perfectly but missed a couple.

Damn! OK, that will teach me to do "checkpatch --strict" before
submitting.

> Maybe in a follow-on patch.

Yes, I'll fix this when I get around to the next series for this driver.

Thanks.


Bjørn

>> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> []
>> @@ -1031,17 +1035,13 @@ cdc_ncm_speed_change(struct usbnet *dev,
>>  	 * device speed. Do print it instead.
>>  	 */
>>  	if ((tx_speed > 1000000) && (rx_speed > 1000000)) {
>> -		printk(KERN_INFO KBUILD_MODNAME
>> -		       ": %s: %u mbit/s downlink "
>> -		       "%u mbit/s uplink\n",
>> -		       dev->net->name,
>> +		netif_info(dev, link, dev->net,
>> +		       "%u mbit/s downlink %u mbit/s uplink\n",
>>  		       (unsigned int)(rx_speed / 1000000U),
>>  		       (unsigned int)(tx_speed / 1000000U));
>>  	} else {
>> -		printk(KERN_INFO KBUILD_MODNAME
>> -		       ": %s: %u kbit/s downlink "
>> -		       "%u kbit/s uplink\n",
>> -		       dev->net->name,
>> +		netif_info(dev, link, dev->net,
>> +		       "%u kbit/s downlink %u kbit/s uplink\n",
>>  		       (unsigned int)(rx_speed / 1000U),
>>  		       (unsigned int)(tx_speed / 1000U));
>>  	}