All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0 / 5] move the common CDC parser
@ 2016-07-14 13:41 Oliver Neukum
  2016-07-14 13:41 ` [PATCH 1/5] usbnet: move the CDC parser into USB core Oliver Neukum
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Oliver Neukum @ 2016-07-14 13:41 UTC (permalink / raw)
  To: linux-usb, netdev, davem, gregKH, bjorn

Experience has shown that making all CDC drivers depend on usbnet
is not practical, because some of them are not network drivers.
So this patch moves the common parser from usbnet into the messages
helpers of usbcore.
The rest of the series applies it to the non-network CDC drivers.

I hope it can go through Greg's tree although it touches usbnet.

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

* [PATCH 1/5] usbnet: move the CDC parser into USB core
  2016-07-14 13:41 [PATCH 0 / 5] move the common CDC parser Oliver Neukum
@ 2016-07-14 13:41 ` Oliver Neukum
       [not found] ` <1468503694-9482-1-git-send-email-oneukum-IBi9RG/b67k@public.gmane.org>
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Oliver Neukum @ 2016-07-14 13:41 UTC (permalink / raw)
  To: linux-usb, netdev, davem, gregKH, bjorn; +Cc: Oliver Neukum, Oliver Neukum

The dependencies were impossible to handle preventing
drivers for CDC devices not which are not network drivers
from using the common parser.

Signed-off-by: Oliver Neukum <ONeukum@suse.com>
---
 drivers/net/usb/usbnet.c   | 138 ----------------------------------------
 drivers/usb/core/message.c | 153 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 153 insertions(+), 138 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 61ba464..b56d78a 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -42,7 +42,6 @@
 #include <linux/mii.h>
 #include <linux/usb.h>
 #include <linux/usb/usbnet.h>
-#include <linux/usb/cdc.h>
 #include <linux/slab.h>
 #include <linux/kernel.h>
 #include <linux/pm_runtime.h>
@@ -1968,143 +1967,6 @@ out:
 	return err;
 }
 
-int cdc_parse_cdc_header(struct usb_cdc_parsed_header *hdr,
-				struct usb_interface *intf,
-				u8 *buffer,
-				int buflen)
-{
-	/* duplicates are ignored */
-	struct usb_cdc_union_desc *union_header = NULL;
-
-	/* duplicates are not tolerated */
-	struct usb_cdc_header_desc *header = NULL;
-	struct usb_cdc_ether_desc *ether = NULL;
-	struct usb_cdc_mdlm_detail_desc *detail = NULL;
-	struct usb_cdc_mdlm_desc *desc = NULL;
-
-	unsigned int elength;
-	int cnt = 0;
-
-	memset(hdr, 0x00, sizeof(struct usb_cdc_parsed_header));
-	hdr->phonet_magic_present = false;
-	while (buflen > 0) {
-		elength = buffer[0];
-		if (!elength) {
-			dev_err(&intf->dev, "skipping garbage byte\n");
-			elength = 1;
-			goto next_desc;
-		}
-		if (buffer[1] != USB_DT_CS_INTERFACE) {
-			dev_err(&intf->dev, "skipping garbage\n");
-			goto next_desc;
-		}
-
-		switch (buffer[2]) {
-		case USB_CDC_UNION_TYPE: /* we've found it */
-			if (elength < sizeof(struct usb_cdc_union_desc))
-				goto next_desc;
-			if (union_header) {
-				dev_err(&intf->dev, "More than one union descriptor, skipping ...\n");
-				goto next_desc;
-			}
-			union_header = (struct usb_cdc_union_desc *)buffer;
-			break;
-		case USB_CDC_COUNTRY_TYPE:
-			if (elength < sizeof(struct usb_cdc_country_functional_desc))
-				goto next_desc;
-			hdr->usb_cdc_country_functional_desc =
-				(struct usb_cdc_country_functional_desc *)buffer;
-			break;
-		case USB_CDC_HEADER_TYPE:
-			if (elength != sizeof(struct usb_cdc_header_desc))
-				goto next_desc;
-			if (header)
-				return -EINVAL;
-			header = (struct usb_cdc_header_desc *)buffer;
-			break;
-		case USB_CDC_ACM_TYPE:
-			if (elength < sizeof(struct usb_cdc_acm_descriptor))
-				goto next_desc;
-			hdr->usb_cdc_acm_descriptor =
-				(struct usb_cdc_acm_descriptor *)buffer;
-			break;
-		case USB_CDC_ETHERNET_TYPE:
-			if (elength != sizeof(struct usb_cdc_ether_desc))
-				goto next_desc;
-			if (ether)
-				return -EINVAL;
-			ether = (struct usb_cdc_ether_desc *)buffer;
-			break;
-		case USB_CDC_CALL_MANAGEMENT_TYPE:
-			if (elength < sizeof(struct usb_cdc_call_mgmt_descriptor))
-				goto next_desc;
-			hdr->usb_cdc_call_mgmt_descriptor =
-				(struct usb_cdc_call_mgmt_descriptor *)buffer;
-			break;
-		case USB_CDC_DMM_TYPE:
-			if (elength < sizeof(struct usb_cdc_dmm_desc))
-				goto next_desc;
-			hdr->usb_cdc_dmm_desc =
-				(struct usb_cdc_dmm_desc *)buffer;
-			break;
-		case USB_CDC_MDLM_TYPE:
-			if (elength < sizeof(struct usb_cdc_mdlm_desc *))
-				goto next_desc;
-			if (desc)
-				return -EINVAL;
-			desc = (struct usb_cdc_mdlm_desc *)buffer;
-			break;
-		case USB_CDC_MDLM_DETAIL_TYPE:
-			if (elength < sizeof(struct usb_cdc_mdlm_detail_desc *))
-				goto next_desc;
-			if (detail)
-				return -EINVAL;
-			detail = (struct usb_cdc_mdlm_detail_desc *)buffer;
-			break;
-		case USB_CDC_NCM_TYPE:
-			if (elength < sizeof(struct usb_cdc_ncm_desc))
-				goto next_desc;
-			hdr->usb_cdc_ncm_desc = (struct usb_cdc_ncm_desc *)buffer;
-			break;
-		case USB_CDC_MBIM_TYPE:
-			if (elength < sizeof(struct usb_cdc_mbim_desc))
-				goto next_desc;
-
-			hdr->usb_cdc_mbim_desc = (struct usb_cdc_mbim_desc *)buffer;
-			break;
-		case USB_CDC_MBIM_EXTENDED_TYPE:
-			if (elength < sizeof(struct usb_cdc_mbim_extended_desc))
-				break;
-			hdr->usb_cdc_mbim_extended_desc =
-				(struct usb_cdc_mbim_extended_desc *)buffer;
-			break;
-		case CDC_PHONET_MAGIC_NUMBER:
-			hdr->phonet_magic_present = true;
-			break;
-		default:
-			/*
-			 * there are LOTS more CDC descriptors that
-			 * could legitimately be found here.
-			 */
-			dev_dbg(&intf->dev, "Ignoring descriptor: type %02x, length %ud\n",
-					buffer[2], elength);
-			goto next_desc;
-		}
-		cnt++;
-next_desc:
-		buflen -= elength;
-		buffer += elength;
-	}
-	hdr->usb_cdc_union_desc = union_header;
-	hdr->usb_cdc_header_desc = header;
-	hdr->usb_cdc_mdlm_detail_desc = detail;
-	hdr->usb_cdc_mdlm_desc = desc;
-	hdr->usb_cdc_ether_desc = ether;
-	return cnt;
-}
-
-EXPORT_SYMBOL(cdc_parse_cdc_header);
-
 /*
  * The function can't be called inside suspend/resume callback,
  * otherwise deadlock will be caused.
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index ea681f1..0406a59 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -12,6 +12,7 @@
 #include <linux/nls.h>
 #include <linux/device.h>
 #include <linux/scatterlist.h>
+#include <linux/usb/cdc.h>
 #include <linux/usb/quirks.h>
 #include <linux/usb/hcd.h>	/* for usbcore internals */
 #include <asm/byteorder.h>
@@ -2023,3 +2024,155 @@ int usb_driver_set_configuration(struct usb_device *udev, int config)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(usb_driver_set_configuration);
+
+/**
+ * cdc_parse_cdc_header - parse the extra headers present in CDC devices
+ * @hdr: the place to put the results of the parsing
+ * @intf: the interface for which parsing is requested
+ * @buffer: pointer to the extra headers to be parsed
+ * @buflen: length of the extra headers
+ *
+ * This evaluates the extra headers present in CDC devices which
+ * bind the interfaces for data and control and provide details
+ * about the capabilities of the device.
+ *
+ * Return: number of descriptors parsed or -EINVAL
+ * if the header is contradictory beyond salvage
+ */
+
+int cdc_parse_cdc_header(struct usb_cdc_parsed_header *hdr,
+				struct usb_interface *intf,
+				u8 *buffer,
+				int buflen)
+{
+	/* duplicates are ignored */
+	struct usb_cdc_union_desc *union_header = NULL;
+
+	/* duplicates are not tolerated */
+	struct usb_cdc_header_desc *header = NULL;
+	struct usb_cdc_ether_desc *ether = NULL;
+	struct usb_cdc_mdlm_detail_desc *detail = NULL;
+	struct usb_cdc_mdlm_desc *desc = NULL;
+
+	unsigned int elength;
+	int cnt = 0;
+
+	memset(hdr, 0x00, sizeof(struct usb_cdc_parsed_header));
+	hdr->phonet_magic_present = false;
+	while (buflen > 0) {
+		elength = buffer[0];
+		if (!elength) {
+			dev_err(&intf->dev, "skipping garbage byte\n");
+			elength = 1;
+			goto next_desc;
+		}
+		if (buffer[1] != USB_DT_CS_INTERFACE) {
+			dev_err(&intf->dev, "skipping garbage\n");
+			goto next_desc;
+		}
+
+		switch (buffer[2]) {
+		case USB_CDC_UNION_TYPE: /* we've found it */
+			if (elength < sizeof(struct usb_cdc_union_desc))
+				goto next_desc;
+			if (union_header) {
+				dev_err(&intf->dev, "More than one union descriptor, skipping ...\n");
+				goto next_desc;
+			}
+			union_header = (struct usb_cdc_union_desc *)buffer;
+			break;
+		case USB_CDC_COUNTRY_TYPE:
+			if (elength < sizeof(struct usb_cdc_country_functional_desc))
+				goto next_desc;
+			hdr->usb_cdc_country_functional_desc =
+				(struct usb_cdc_country_functional_desc *)buffer;
+			break;
+		case USB_CDC_HEADER_TYPE:
+			if (elength != sizeof(struct usb_cdc_header_desc))
+				goto next_desc;
+			if (header)
+				return -EINVAL;
+			header = (struct usb_cdc_header_desc *)buffer;
+			break;
+		case USB_CDC_ACM_TYPE:
+			if (elength < sizeof(struct usb_cdc_acm_descriptor))
+				goto next_desc;
+			hdr->usb_cdc_acm_descriptor =
+				(struct usb_cdc_acm_descriptor *)buffer;
+			break;
+		case USB_CDC_ETHERNET_TYPE:
+			if (elength != sizeof(struct usb_cdc_ether_desc))
+				goto next_desc;
+			if (ether)
+				return -EINVAL;
+			ether = (struct usb_cdc_ether_desc *)buffer;
+			break;
+		case USB_CDC_CALL_MANAGEMENT_TYPE:
+			if (elength < sizeof(struct usb_cdc_call_mgmt_descriptor))
+				goto next_desc;
+			hdr->usb_cdc_call_mgmt_descriptor =
+				(struct usb_cdc_call_mgmt_descriptor *)buffer;
+			break;
+		case USB_CDC_DMM_TYPE:
+			if (elength < sizeof(struct usb_cdc_dmm_desc))
+				goto next_desc;
+			hdr->usb_cdc_dmm_desc =
+				(struct usb_cdc_dmm_desc *)buffer;
+			break;
+		case USB_CDC_MDLM_TYPE:
+			if (elength < sizeof(struct usb_cdc_mdlm_desc *))
+				goto next_desc;
+			if (desc)
+				return -EINVAL;
+			desc = (struct usb_cdc_mdlm_desc *)buffer;
+			break;
+		case USB_CDC_MDLM_DETAIL_TYPE:
+			if (elength < sizeof(struct usb_cdc_mdlm_detail_desc *))
+				goto next_desc;
+			if (detail)
+				return -EINVAL;
+			detail = (struct usb_cdc_mdlm_detail_desc *)buffer;
+			break;
+		case USB_CDC_NCM_TYPE:
+			if (elength < sizeof(struct usb_cdc_ncm_desc))
+				goto next_desc;
+			hdr->usb_cdc_ncm_desc = (struct usb_cdc_ncm_desc *)buffer;
+			break;
+		case USB_CDC_MBIM_TYPE:
+			if (elength < sizeof(struct usb_cdc_mbim_desc))
+				goto next_desc;
+
+			hdr->usb_cdc_mbim_desc = (struct usb_cdc_mbim_desc *)buffer;
+			break;
+		case USB_CDC_MBIM_EXTENDED_TYPE:
+			if (elength < sizeof(struct usb_cdc_mbim_extended_desc))
+				break;
+			hdr->usb_cdc_mbim_extended_desc =
+				(struct usb_cdc_mbim_extended_desc *)buffer;
+			break;
+		case CDC_PHONET_MAGIC_NUMBER:
+			hdr->phonet_magic_present = true;
+			break;
+		default:
+			/*
+			 * there are LOTS more CDC descriptors that
+			 * could legitimately be found here.
+			 */
+			dev_dbg(&intf->dev, "Ignoring descriptor: type %02x, length %ud\n",
+					buffer[2], elength);
+			goto next_desc;
+		}
+		cnt++;
+next_desc:
+		buflen -= elength;
+		buffer += elength;
+	}
+	hdr->usb_cdc_union_desc = union_header;
+	hdr->usb_cdc_header_desc = header;
+	hdr->usb_cdc_mdlm_detail_desc = detail;
+	hdr->usb_cdc_mdlm_desc = desc;
+	hdr->usb_cdc_ether_desc = ether;
+	return cnt;
+}
+
+EXPORT_SYMBOL(cdc_parse_cdc_header);
-- 
2.1.4

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

* [PATCH 2/5] cdc-acm: use the common parser
       [not found] ` <1468503694-9482-1-git-send-email-oneukum-IBi9RG/b67k@public.gmane.org>
@ 2016-07-14 13:41   ` Oliver Neukum
  2016-07-14 13:41   ` [PATCH 3/5] cdc-acm: cleanup error handling Oliver Neukum
  2016-07-14 13:41   ` [PATCH 5/5] cdc-acm: beautify probe() Oliver Neukum
  2 siblings, 0 replies; 8+ messages in thread
From: Oliver Neukum @ 2016-07-14 13:41 UTC (permalink / raw)
  To: linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	gregKH-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, bjorn-yOkvZcmFvRU
  Cc: Oliver Neukum, Oliver Neukum

This introduces the common parser for extra CDC headers now that it no longer
depends on usbnet.

Signed-off-by: Oliver Neukum <ONeukum-IBi9RG/b67k@public.gmane.org>
---
 drivers/usb/class/cdc-acm.c | 69 +++++++--------------------------------------
 1 file changed, 10 insertions(+), 59 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 94a14f5..70bd642 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1147,6 +1147,7 @@ static int acm_probe(struct usb_interface *intf,
 {
 	struct usb_cdc_union_desc *union_header = NULL;
 	struct usb_cdc_country_functional_desc *cfd = NULL;
+	struct usb_cdc_call_mgmt_descriptor *cmd = NULL;
 	unsigned char *buffer = intf->altsetting->extra;
 	int buflen = intf->altsetting->extralen;
 	struct usb_interface *control_interface;
@@ -1155,18 +1156,16 @@ static int acm_probe(struct usb_interface *intf,
 	struct usb_endpoint_descriptor *epread = NULL;
 	struct usb_endpoint_descriptor *epwrite = NULL;
 	struct usb_device *usb_dev = interface_to_usbdev(intf);
+	struct usb_cdc_parsed_header hdr;
 	struct acm *acm;
 	int minor;
 	int ctrlsize, readsize;
 	u8 *buf;
-	u8 ac_management_function = 0;
-	u8 call_management_function = 0;
 	int call_interface_num = -1;
 	int data_interface_num = -1;
 	unsigned long quirks;
 	int num_rx_buf;
 	int i;
-	unsigned int elength = 0;
 	int combined_interfaces = 0;
 	struct device *tty_dev;
 	int rv = -ENOMEM;
@@ -1210,61 +1209,11 @@ static int acm_probe(struct usb_interface *intf,
 		}
 	}
 
-	while (buflen > 0) {
-		elength = buffer[0];
-		if (!elength) {
-			dev_err(&intf->dev, "skipping garbage byte\n");
-			elength = 1;
-			goto next_desc;
-		}
-		if (buffer[1] != USB_DT_CS_INTERFACE) {
-			dev_err(&intf->dev, "skipping garbage\n");
-			goto next_desc;
-		}
-
-		switch (buffer[2]) {
-		case USB_CDC_UNION_TYPE: /* we've found it */
-			if (elength < sizeof(struct usb_cdc_union_desc))
-				goto next_desc;
-			if (union_header) {
-				dev_err(&intf->dev, "More than one "
-					"union descriptor, skipping ...\n");
-				goto next_desc;
-			}
-			union_header = (struct usb_cdc_union_desc *)buffer;
-			break;
-		case USB_CDC_COUNTRY_TYPE: /* export through sysfs*/
-			if (elength < sizeof(struct usb_cdc_country_functional_desc))
-				goto next_desc;
-			cfd = (struct usb_cdc_country_functional_desc *)buffer;
-			break;
-		case USB_CDC_HEADER_TYPE: /* maybe check version */
-			break; /* for now we ignore it */
-		case USB_CDC_ACM_TYPE:
-			if (elength < 4)
-				goto next_desc;
-			ac_management_function = buffer[3];
-			break;
-		case USB_CDC_CALL_MANAGEMENT_TYPE:
-			if (elength < 5)
-				goto next_desc;
-			call_management_function = buffer[3];
-			call_interface_num = buffer[4];
-			break;
-		default:
-			/*
-			 * there are LOTS more CDC descriptors that
-			 * could legitimately be found here.
-			 */
-			dev_dbg(&intf->dev, "Ignoring descriptor: "
-					"type %02x, length %ud\n",
-					buffer[2], elength);
-			break;
-		}
-next_desc:
-		buflen -= elength;
-		buffer += elength;
-	}
+	cdc_parse_cdc_header(&hdr, intf, buffer, buflen);
+	union_header = hdr.usb_cdc_union_desc;
+	cmd = hdr.usb_cdc_call_mgmt_descriptor;
+	if (cmd)
+		call_interface_num = cmd->bDataInterface;
 
 	if (!union_header) {
 		if (call_interface_num > 0) {
@@ -1394,7 +1343,8 @@ made_compressed_probe:
 	acm->data = data_interface;
 	acm->minor = minor;
 	acm->dev = usb_dev;
-	acm->ctrl_caps = ac_management_function;
+	if (hdr.usb_cdc_acm_descriptor)
+		acm->ctrl_caps = hdr.usb_cdc_acm_descriptor->bmCapabilities;
 	if (quirks & NO_CAP_LINE)
 		acm->ctrl_caps &= ~USB_CDC_CAP_LINE;
 	acm->ctrlsize = ctrlsize;
@@ -1488,6 +1438,7 @@ made_compressed_probe:
 	if (i < 0)
 		goto alloc_fail7;
 
+	cfd = hdr.usb_cdc_country_functional_desc;
 	if (cfd) { /* export the country data */
 		acm->country_codes = kmalloc(cfd->bLength - 4, GFP_KERNEL);
 		if (!acm->country_codes)
-- 
2.1.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

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

* [PATCH 3/5] cdc-acm: cleanup error handling
       [not found] ` <1468503694-9482-1-git-send-email-oneukum-IBi9RG/b67k@public.gmane.org>
  2016-07-14 13:41   ` [PATCH 2/5] cdc-acm: use the common parser Oliver Neukum
@ 2016-07-14 13:41   ` Oliver Neukum
  2016-07-14 13:41   ` [PATCH 5/5] cdc-acm: beautify probe() Oliver Neukum
  2 siblings, 0 replies; 8+ messages in thread
From: Oliver Neukum @ 2016-07-14 13:41 UTC (permalink / raw)
  To: linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	gregKH-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, bjorn-yOkvZcmFvRU
  Cc: Oliver Neukum, Oliver Neukum

A small update to unify error handling during probe().

Signed-off-by: Oliver Neukum <ONeukum-IBi9RG/b67k@public.gmane.org>
---
 drivers/usb/class/cdc-acm.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 70bd642..1857fad 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1328,11 +1328,8 @@ made_compressed_probe:
 		goto alloc_fail;
 
 	minor = acm_alloc_minor(acm);
-	if (minor < 0) {
-		dev_err(&intf->dev, "no more free acm devices\n");
-		kfree(acm);
-		return -ENODEV;
-	}
+	if (minor < 0)
+		goto alloc_fail1;
 
 	ctrlsize = usb_endpoint_maxp(epctrl);
 	readsize = usb_endpoint_maxp(epread) *
@@ -1523,6 +1520,7 @@ alloc_fail4:
 	usb_free_coherent(usb_dev, ctrlsize, acm->ctrl_buffer, acm->ctrl_dma);
 alloc_fail2:
 	acm_release_minor(acm);
+alloc_fail1:
 	kfree(acm);
 alloc_fail:
 	return rv;
-- 
2.1.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

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

* [PATCH 4/5] cdc-wdm: use the common CDC parser
  2016-07-14 13:41 [PATCH 0 / 5] move the common CDC parser Oliver Neukum
  2016-07-14 13:41 ` [PATCH 1/5] usbnet: move the CDC parser into USB core Oliver Neukum
       [not found] ` <1468503694-9482-1-git-send-email-oneukum-IBi9RG/b67k@public.gmane.org>
@ 2016-07-14 13:41 ` Oliver Neukum
  2016-07-15 18:51 ` [PATCH 0 / 5] move " David Miller
  3 siblings, 0 replies; 8+ messages in thread
From: Oliver Neukum @ 2016-07-14 13:41 UTC (permalink / raw)
  To: linux-usb, netdev, davem, gregKH, bjorn; +Cc: Oliver Neukum, Oliver Neukum

Now that the common parser resides in USB core, it can
be used for CDC-WDM.

Signed-off-by: Oliver Neukum <ONeukum@suse.com>
---
 drivers/usb/class/cdc-wdm.c | 30 +++++-------------------------
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 61ea879..337948c 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -875,38 +875,18 @@ static int wdm_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	int rv = -EINVAL;
 	struct usb_host_interface *iface;
 	struct usb_endpoint_descriptor *ep;
-	struct usb_cdc_dmm_desc *dmhd;
+	struct usb_cdc_parsed_header hdr;
 	u8 *buffer = intf->altsetting->extra;
 	int buflen = intf->altsetting->extralen;
 	u16 maxcom = WDM_DEFAULT_BUFSIZE;
 
 	if (!buffer)
 		goto err;
-	while (buflen > 2) {
-		if (buffer[1] != USB_DT_CS_INTERFACE) {
-			dev_err(&intf->dev, "skipping garbage\n");
-			goto next_desc;
-		}
 
-		switch (buffer[2]) {
-		case USB_CDC_HEADER_TYPE:
-			break;
-		case USB_CDC_DMM_TYPE:
-			dmhd = (struct usb_cdc_dmm_desc *)buffer;
-			maxcom = le16_to_cpu(dmhd->wMaxCommand);
-			dev_dbg(&intf->dev,
-				"Finding maximum buffer length: %d", maxcom);
-			break;
-		default:
-			dev_err(&intf->dev,
-				"Ignoring extra header, type %d, length %d\n",
-				buffer[2], buffer[0]);
-			break;
-		}
-next_desc:
-		buflen -= buffer[0];
-		buffer += buffer[0];
-	}
+	cdc_parse_cdc_header(&hdr, intf, buffer, buflen);
+
+	if (hdr.usb_cdc_dmm_desc)
+		maxcom = le16_to_cpu(hdr.usb_cdc_dmm_desc->wMaxCommand);
 
 	iface = intf->cur_altsetting;
 	if (iface->desc.bNumEndpoints != 1)
-- 
2.1.4

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

* [PATCH 5/5] cdc-acm: beautify probe()
       [not found] ` <1468503694-9482-1-git-send-email-oneukum-IBi9RG/b67k@public.gmane.org>
  2016-07-14 13:41   ` [PATCH 2/5] cdc-acm: use the common parser Oliver Neukum
  2016-07-14 13:41   ` [PATCH 3/5] cdc-acm: cleanup error handling Oliver Neukum
@ 2016-07-14 13:41   ` Oliver Neukum
  2 siblings, 0 replies; 8+ messages in thread
From: Oliver Neukum @ 2016-07-14 13:41 UTC (permalink / raw)
  To: linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	gregKH-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, bjorn-yOkvZcmFvRU
  Cc: Oliver Neukum

This removes some overly long lines by renaming variables and giving
them local scope.

Signed-off-by: Oliver Neukum <oneukum-IBi9RG/b67k@public.gmane.org>
---
 drivers/usb/class/cdc-acm.c | 44 ++++++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 1857fad..369bde0 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1146,8 +1146,7 @@ static int acm_probe(struct usb_interface *intf,
 		     const struct usb_device_id *id)
 {
 	struct usb_cdc_union_desc *union_header = NULL;
-	struct usb_cdc_country_functional_desc *cfd = NULL;
-	struct usb_cdc_call_mgmt_descriptor *cmd = NULL;
+	struct usb_cdc_call_mgmt_descriptor *cmgmd = NULL;
 	unsigned char *buffer = intf->altsetting->extra;
 	int buflen = intf->altsetting->extralen;
 	struct usb_interface *control_interface;
@@ -1156,13 +1155,13 @@ static int acm_probe(struct usb_interface *intf,
 	struct usb_endpoint_descriptor *epread = NULL;
 	struct usb_endpoint_descriptor *epwrite = NULL;
 	struct usb_device *usb_dev = interface_to_usbdev(intf);
-	struct usb_cdc_parsed_header hdr;
+	struct usb_cdc_parsed_header h;
 	struct acm *acm;
 	int minor;
 	int ctrlsize, readsize;
 	u8 *buf;
-	int call_interface_num = -1;
-	int data_interface_num = -1;
+	int call_intf_num = -1;
+	int data_intf_num = -1;
 	unsigned long quirks;
 	int num_rx_buf;
 	int i;
@@ -1209,20 +1208,22 @@ static int acm_probe(struct usb_interface *intf,
 		}
 	}
 
-	cdc_parse_cdc_header(&hdr, intf, buffer, buflen);
-	union_header = hdr.usb_cdc_union_desc;
-	cmd = hdr.usb_cdc_call_mgmt_descriptor;
-	if (cmd)
-		call_interface_num = cmd->bDataInterface;
+	cdc_parse_cdc_header(&h, intf, buffer, buflen);
+	union_header = h.usb_cdc_union_desc;
+	cmgmd = h.usb_cdc_call_mgmt_descriptor;
+	if (cmgmd)
+		call_intf_num = cmgmd->bDataInterface;
 
 	if (!union_header) {
-		if (call_interface_num > 0) {
+		if (call_intf_num > 0) {
 			dev_dbg(&intf->dev, "No union descriptor, using call management descriptor\n");
 			/* quirks for Droids MuIn LCD */
-			if (quirks & NO_DATA_INTERFACE)
+			if (quirks & NO_DATA_INTERFACE) {
 				data_interface = usb_ifnum_to_if(usb_dev, 0);
-			else
-				data_interface = usb_ifnum_to_if(usb_dev, (data_interface_num = call_interface_num));
+			} else {
+				data_intf_num = call_intf_num;
+				data_interface = usb_ifnum_to_if(usb_dev, data_intf_num);
+			}
 			control_interface = intf;
 		} else {
 			if (intf->cur_altsetting->desc.bNumEndpoints != 3) {
@@ -1236,8 +1237,9 @@ static int acm_probe(struct usb_interface *intf,
 			}
 		}
 	} else {
+		data_intf_num = union_header->bSlaveInterface0;
 		control_interface = usb_ifnum_to_if(usb_dev, union_header->bMasterInterface0);
-		data_interface = usb_ifnum_to_if(usb_dev, (data_interface_num = union_header->bSlaveInterface0));
+		data_interface = usb_ifnum_to_if(usb_dev, data_intf_num);
 	}
 
 	if (!control_interface || !data_interface) {
@@ -1245,7 +1247,7 @@ static int acm_probe(struct usb_interface *intf,
 		return -ENODEV;
 	}
 
-	if (data_interface_num != call_interface_num)
+	if (data_intf_num != call_intf_num)
 		dev_dbg(&intf->dev, "Separate call control interface. That is not fully supported.\n");
 
 	if (control_interface == data_interface) {
@@ -1340,8 +1342,8 @@ made_compressed_probe:
 	acm->data = data_interface;
 	acm->minor = minor;
 	acm->dev = usb_dev;
-	if (hdr.usb_cdc_acm_descriptor)
-		acm->ctrl_caps = hdr.usb_cdc_acm_descriptor->bmCapabilities;
+	if (h.usb_cdc_acm_descriptor)
+		acm->ctrl_caps = h.usb_cdc_acm_descriptor->bmCapabilities;
 	if (quirks & NO_CAP_LINE)
 		acm->ctrl_caps &= ~USB_CDC_CAP_LINE;
 	acm->ctrlsize = ctrlsize;
@@ -1435,8 +1437,10 @@ made_compressed_probe:
 	if (i < 0)
 		goto alloc_fail7;
 
-	cfd = hdr.usb_cdc_country_functional_desc;
-	if (cfd) { /* export the country data */
+	if (h.usb_cdc_country_functional_desc) { /* export the country data */
+		struct usb_cdc_country_functional_desc * cfd =
+					h.usb_cdc_country_functional_desc;
+
 		acm->country_codes = kmalloc(cfd->bLength - 4, GFP_KERNEL);
 		if (!acm->country_codes)
 			goto skip_countries;
-- 
2.1.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

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

* Re: [PATCH 0 / 5] move the common CDC parser
  2016-07-14 13:41 [PATCH 0 / 5] move the common CDC parser Oliver Neukum
                   ` (2 preceding siblings ...)
  2016-07-14 13:41 ` [PATCH 4/5] cdc-wdm: use the common CDC parser Oliver Neukum
@ 2016-07-15 18:51 ` David Miller
       [not found]   ` <20160715.115147.1896139966807241395.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
  3 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2016-07-15 18:51 UTC (permalink / raw)
  To: oneukum; +Cc: linux-usb, netdev, gregKH, bjorn

From: Oliver Neukum <oneukum@suse.com>
Date: Thu, 14 Jul 2016 15:41:29 +0200

> Experience has shown that making all CDC drivers depend on usbnet
> is not practical, because some of them are not network drivers.
> So this patch moves the common parser from usbnet into the messages
> helpers of usbcore.
> The rest of the series applies it to the non-network CDC drivers.
> 
> I hope it can go through Greg's tree although it touches usbnet.

I'm fine with Greg taking this series, sure.

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

* Re: [PATCH 0 / 5] move the common CDC parser
       [not found]   ` <20160715.115147.1896139966807241395.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
@ 2016-07-15 21:57     ` Greg KH
  0 siblings, 0 replies; 8+ messages in thread
From: Greg KH @ 2016-07-15 21:57 UTC (permalink / raw)
  To: David Miller
  Cc: oneukum-IBi9RG/b67k, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, bjorn-yOkvZcmFvRU

On Fri, Jul 15, 2016 at 11:51:47AM -0700, David Miller wrote:
> From: Oliver Neukum <oneukum-IBi9RG/b67k@public.gmane.org>
> Date: Thu, 14 Jul 2016 15:41:29 +0200
> 
> > Experience has shown that making all CDC drivers depend on usbnet
> > is not practical, because some of them are not network drivers.
> > So this patch moves the common parser from usbnet into the messages
> > helpers of usbcore.
> > The rest of the series applies it to the non-network CDC drivers.
> > 
> > I hope it can go through Greg's tree although it touches usbnet.
> 
> I'm fine with Greg taking this series, sure.

Ok, I'll take it, thanks.

greg k-h
--
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	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-07-15 21:57 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-14 13:41 [PATCH 0 / 5] move the common CDC parser Oliver Neukum
2016-07-14 13:41 ` [PATCH 1/5] usbnet: move the CDC parser into USB core Oliver Neukum
     [not found] ` <1468503694-9482-1-git-send-email-oneukum-IBi9RG/b67k@public.gmane.org>
2016-07-14 13:41   ` [PATCH 2/5] cdc-acm: use the common parser Oliver Neukum
2016-07-14 13:41   ` [PATCH 3/5] cdc-acm: cleanup error handling Oliver Neukum
2016-07-14 13:41   ` [PATCH 5/5] cdc-acm: beautify probe() Oliver Neukum
2016-07-14 13:41 ` [PATCH 4/5] cdc-wdm: use the common CDC parser Oliver Neukum
2016-07-15 18:51 ` [PATCH 0 / 5] move " David Miller
     [not found]   ` <20160715.115147.1896139966807241395.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2016-07-15 21:57     ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.