Skip to contentSkip to conversation

cdc_ncm: fixes for big-endian architecture / MIPS

PATCH Applied Giuseppe Scrivano 4 participants 6 messages Aug 3, 2011 – Aug 4, 2011 3 messages not in the archive

Subsystems (4)
Networking Network drivers USB networking drivers USB cdc Ethernet driver
lore
Share & tools
lore

Giuseppe Scrivano

author
#0 lore raw

Hello,

I have reworked the original patch I have submitted to Alexey.

Regards,
Giuseppe

Quoted 1 line

From 8bd65735b4f0db5b6213f59a443c21d0d55dba8e Mon Sep 17 00:00:00 2001

From: Giuseppe Scrivano <[email protected]>
Date: Fri, 15 Jul 2011 15:34:14 +0200
Subject: [PATCH] cdc_ncm: fix endianess problem.

Signed-off-by Giuseppe Scrivano <[email protected]>
---
 drivers/net/usb/cdc_ncm.c |   65 +++++++++++++++++++++++++--------------------
 1 files changed, 36 insertions(+), 29 deletions(-)
drivers/net/usb/cdc_ncm.c +36 -29
index f33ca6a..cd5d819 100644
@@ -54,7 +54,7 @@
 #include <linux/usb/usbnet.h>
 #include <linux/usb/cdc.h>
 
-#define	DRIVER_VERSION				"01-June-2011"
+#define	DRIVER_VERSION				"03-Aug-2011"
 
 /* CDC NCM subclass 3.2.1 */
 #define USB_CDC_NCM_NDP16_LENGTH_MIN		0x10

@@ -136,6 +136,14 @@ struct cdc_ncm_ctx {
 	u16 connected;
 };
 
+struct cdc_ncm_request {
+	u8	bRequest;
+	u8	bmRequestType;
+	u16	wValue;
+	u16	wIndex;
+	u16	wLength;
+} __packed;
+
 static void cdc_ncm_tx_timeout(unsigned long arg);
 static const struct driver_info cdc_ncm_info;
 static struct usb_driver cdc_ncm_driver;

@@ -165,7 +173,7 @@ cdc_ncm_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *info)
 }
 
 static int
-cdc_ncm_do_request(struct cdc_ncm_ctx *ctx, struct usb_cdc_notification *req,
+cdc_ncm_do_request(struct cdc_ncm_ctx *ctx, struct cdc_ncm_request *req,
 		   void *data, u16 flags, u16 *actlen, u16 timeout)
 {
 	int err;

@@ -173,7 +181,7 @@ cdc_ncm_do_request(struct cdc_ncm_ctx *ctx, struct usb_cdc_notification *req,
 	err = usb_control_msg(ctx->udev, (req->bmRequestType & USB_DIR_IN) ?
 				usb_rcvctrlpipe(ctx->udev, 0) :
 				usb_sndctrlpipe(ctx->udev, 0),
-				req->bNotificationType, req->bmRequestType,
+				req->bRequest, req->bmRequestType,
 				req->wValue,
 				req->wIndex, data,
 				req->wLength, timeout);

@@ -192,7 +200,7 @@ cdc_ncm_do_request(struct cdc_ncm_ctx *ctx, struct usb_cdc_notification *req,
 
 static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 {
-	struct usb_cdc_notification req;
+	struct cdc_ncm_request req;
 	u32 val;
 	u8 flags;
 	u8 iface_no;

@@ -202,10 +210,10 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 	iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
 
 	req.bmRequestType = USB_TYPE_CLASS | USB_DIR_IN | USB_RECIP_INTERFACE;
-	req.bNotificationType = USB_CDC_GET_NTB_PARAMETERS;
+	req.bRequest = USB_CDC_GET_NTB_PARAMETERS;
 	req.wValue = 0;
-	req.wIndex = cpu_to_le16(iface_no);
-	req.wLength = cpu_to_le16(sizeof(ctx->ncm_parm));
+	req.wIndex = iface_no;
+	req.wLength = sizeof(ctx->ncm_parm);
 
 	err = cdc_ncm_do_request(ctx, &req, &ctx->ncm_parm, 0, NULL, 1000);
 	if (err) {

@@ -256,9 +264,9 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 	if (ctx->rx_max != le32_to_cpu(ctx->ncm_parm.dwNtbInMaxSize)) {
 		req.bmRequestType = USB_TYPE_CLASS | USB_DIR_OUT |
 							USB_RECIP_INTERFACE;
-		req.bNotificationType = USB_CDC_SET_NTB_INPUT_SIZE;
+		req.bRequest = USB_CDC_SET_NTB_INPUT_SIZE;
 		req.wValue = 0;
-		req.wIndex = cpu_to_le16(iface_no);
+		req.wIndex = iface_no;
 
 		if (flags & USB_CDC_NCM_NCAP_NTB_INPUT_SIZE) {
 			struct usb_cdc_ncm_ndp_input_size ndp_in_sz;

@@ -335,9 +343,9 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 	if (flags & USB_CDC_NCM_NCAP_CRC_MODE) {
 		req.bmRequestType = USB_TYPE_CLASS | USB_DIR_OUT |
 							USB_RECIP_INTERFACE;
-		req.bNotificationType = USB_CDC_SET_CRC_MODE;
-		req.wValue = cpu_to_le16(USB_CDC_NCM_CRC_NOT_APPENDED);
-		req.wIndex = cpu_to_le16(iface_no);
+		req.bRequest = USB_CDC_SET_CRC_MODE;
+		req.wValue = USB_CDC_NCM_CRC_NOT_APPENDED;
+		req.wIndex = iface_no;
 		req.wLength = 0;
 
 		err = cdc_ncm_do_request(ctx, &req, NULL, 0, NULL, 1000);

@@ -349,9 +357,9 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 	if (ntb_fmt_supported & USB_CDC_NCM_NTH32_SIGN) {
 		req.bmRequestType = USB_TYPE_CLASS | USB_DIR_OUT |
 							USB_RECIP_INTERFACE;
-		req.bNotificationType = USB_CDC_SET_NTB_FORMAT;
-		req.wValue = cpu_to_le16(USB_CDC_NCM_NTB16_FORMAT);
-		req.wIndex = cpu_to_le16(iface_no);
+		req.bRequest = USB_CDC_SET_NTB_FORMAT;
+		req.wValue = USB_CDC_NCM_NTB16_FORMAT;
+		req.wIndex = iface_no;
 		req.wLength = 0;
 
 		err = cdc_ncm_do_request(ctx, &req, NULL, 0, NULL, 1000);

@@ -368,10 +376,10 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 
 		req.bmRequestType = USB_TYPE_CLASS | USB_DIR_IN |
 							USB_RECIP_INTERFACE;
-		req.bNotificationType = USB_CDC_GET_MAX_DATAGRAM_SIZE;
+		req.bRequest = USB_CDC_GET_MAX_DATAGRAM_SIZE;
 		req.wValue = 0;
-		req.wIndex = cpu_to_le16(iface_no);
-		req.wLength = cpu_to_le16(2);
+		req.wIndex = iface_no;
+		req.wLength = 2;
 
 		err = cdc_ncm_do_request(ctx, &req, &max_datagram_size, 0, NULL,
 									1000);

@@ -398,9 +406,9 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 			/* if value changed, update device */
 			req.bmRequestType = USB_TYPE_CLASS | USB_DIR_OUT |
 							USB_RECIP_INTERFACE;
-			req.bNotificationType = USB_CDC_SET_MAX_DATAGRAM_SIZE;
+			req.bRequest = USB_CDC_SET_MAX_DATAGRAM_SIZE;
 			req.wValue = 0;
-			req.wIndex = cpu_to_le16(iface_no);
+			req.wIndex = iface_no;
 			req.wLength = 2;
 			max_datagram_size = cpu_to_le16(ctx->max_datagram_size);
 

@@ -672,7 +680,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 	u32 rem;
 	u32 offset;
 	u32 last_offset;
-	u16 n = 0;
+	u16 n = 0, index;
 	u8 ready2send = 0;
 
 	/* if there is a remaining skb, it gets priority */

@@ -860,8 +868,8 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 					cpu_to_le16(sizeof(ctx->tx_ncm.nth16));
 	ctx->tx_ncm.nth16.wSequence = cpu_to_le16(ctx->tx_seq);
 	ctx->tx_ncm.nth16.wBlockLength = cpu_to_le16(last_offset);
-	ctx->tx_ncm.nth16.wNdpIndex = ALIGN(sizeof(struct usb_cdc_ncm_nth16),
-							ctx->tx_ndp_modulus);
+	index = ALIGN(sizeof(struct usb_cdc_ncm_nth16), ctx->tx_ndp_modulus);
+	ctx->tx_ncm.nth16.wNdpIndex = cpu_to_le16(index);
 
 	memcpy(skb_out->data, &(ctx->tx_ncm.nth16), sizeof(ctx->tx_ncm.nth16));
 	ctx->tx_seq++;

@@ -874,12 +882,11 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 	ctx->tx_ncm.ndp16.wLength = cpu_to_le16(rem);
 	ctx->tx_ncm.ndp16.wNextNdpIndex = 0; /* reserved */
 
-	memcpy(((u8 *)skb_out->data) + ctx->tx_ncm.nth16.wNdpIndex,
+	memcpy(((u8 *)skb_out->data) + index,
 						&(ctx->tx_ncm.ndp16),
 						sizeof(ctx->tx_ncm.ndp16));
 
-	memcpy(((u8 *)skb_out->data) + ctx->tx_ncm.nth16.wNdpIndex +
-					sizeof(ctx->tx_ncm.ndp16),
+	memcpy(((u8 *)skb_out->data) + index + sizeof(ctx->tx_ncm.ndp16),
 					&(ctx->tx_ncm.dpe16),
 					(ctx->tx_curr_frame_num + 1) *
 					sizeof(struct usb_cdc_ncm_dpe16));

@@ -1129,7 +1136,7 @@ cdc_ncm_speed_change(struct cdc_ncm_ctx *ctx,
 static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
 {
 	struct cdc_ncm_ctx *ctx;
-	struct usb_cdc_notification *event;
+	struct cdc_ncm_request *event;
 
 	ctx = (struct cdc_ncm_ctx *)dev->data[0];
 

@@ -1145,7 +1152,7 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
 
 	event = urb->transfer_buffer;
 
-	switch (event->bNotificationType) {
+	switch (event->bRequest) {
 	case USB_CDC_NOTIFY_NETWORK_CONNECTION:
 		/*
 		 * According to the CDC NCM specification ch.7.1

@@ -1177,7 +1184,7 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
 
 	default:
 		dev_err(&dev->udev->dev, "NCM: unexpected "
-			"notification 0x%02x!\n", event->bNotificationType);
+			"notification 0x%02x!\n", event->bRequest);
 		break;
 	}
 }
Signature · 1 line
1.7.5.4

A message that is not in the archive (sent off-list or not yet received): 87hb5yock6.fsf-mXXj517/[email protected]

#2 lore raw
Quoted 8 lines

From 8bd65735b4f0db5b6213f59a443c21d0d55dba8e Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <[email protected]>
Date: Fri, 15 Jul 2011 15:34:14 +0200
Subject: [PATCH] cdc_ncm: fix endianess problem.

Signed-off-by Giuseppe Scrivano <[email protected]>
---
 drivers/net/usb/cdc_ncm.c |   65 +++++++++++++++++++++++++-------------------

drivers/net/usb/cdc_ncm.c struct cdc_ncm_ctx { · line 145

Quoted 9 lines
drivers/net/usb/cdc_ncm.c +7 -1
-
 
+struct cdc_ncm_request {
+	u8	bRequest;
+	u8	bmRequestType;
+	u16	wValue;
+	u16	wIndex;
+	u16	wLength;
+} __packed;

Since it was incorrect to use struct usb_cdc_notification I would rather remove cdc_ncm_do_request() function and provide u16 parameters directly to usb_control_msg without creating yet additional structure for usb control request.

alexey
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 2AC7D4AD8BA1C640B4C60C61C8E520153E3DEF6C26-8ZTw5gFVCTjVH5byLeRTJxkTb7+GphCuwzqs5ZKRSiY@public.gmane.org

#4 lore raw
Quoted 3 lines · Alexey ORISHKO

Since it was incorrect to use struct usb_cdc_notification I would rather remove cdc_ncm_do_request() function and provide u16 parameters directly to usb_control_msg without creating yet additional structure for usb control request.

here another version, I have removed cdc_ncm_do_request and call directly usb_control_msg. Tested on mips.

Cheers,
Giuseppe

Quoted 1 line

From 7ba49d858103acb2ce4043127e3512ea29dff307 Mon Sep 17 00:00:00 2001

From: Giuseppe Scrivano <[email protected]>
Date: Fri, 15 Jul 2011 15:34:14 +0200
Subject: [PATCH] cdc_ncm: fix endianess problem.

Signed-off-by Giuseppe Scrivano <[email protected]>
---
 drivers/net/usb/cdc_ncm.c |  156 ++++++++++++++++-----------------------------
 1 files changed, 56 insertions(+), 100 deletions(-)
drivers/net/usb/cdc_ncm.c +56 -100
index f33ca6a..9c37d54 100644
@@ -54,7 +54,7 @@
 #include <linux/usb/usbnet.h>
 #include <linux/usb/cdc.h>
 
-#define	DRIVER_VERSION				"01-June-2011"
+#define	DRIVER_VERSION				"03-Aug-2011"
 
 /* CDC NCM subclass 3.2.1 */
 #define USB_CDC_NCM_NDP16_LENGTH_MIN		0x10

@@ -164,35 +164,8 @@ 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 int
-cdc_ncm_do_request(struct cdc_ncm_ctx *ctx, struct usb_cdc_notification *req,
-		   void *data, u16 flags, u16 *actlen, u16 timeout)
-{
-	int err;
-
-	err = usb_control_msg(ctx->udev, (req->bmRequestType & USB_DIR_IN) ?
-				usb_rcvctrlpipe(ctx->udev, 0) :
-				usb_sndctrlpipe(ctx->udev, 0),
-				req->bNotificationType, req->bmRequestType,
-				req->wValue,
-				req->wIndex, data,
-				req->wLength, timeout);
-
-	if (err < 0) {
-		if (actlen)
-			*actlen = 0;
-		return err;
-	}
-
-	if (actlen)
-		*actlen = err;
-
-	return 0;
-}
-
 static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 {
-	struct usb_cdc_notification req;
 	u32 val;
 	u8 flags;
 	u8 iface_no;

@@ -201,14 +174,14 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 
 	iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
 
-	req.bmRequestType = USB_TYPE_CLASS | USB_DIR_IN | USB_RECIP_INTERFACE;
-	req.bNotificationType = USB_CDC_GET_NTB_PARAMETERS;
-	req.wValue = 0;
-	req.wIndex = cpu_to_le16(iface_no);
-	req.wLength = cpu_to_le16(sizeof(ctx->ncm_parm));
-
-	err = cdc_ncm_do_request(ctx, &req, &ctx->ncm_parm, 0, NULL, 1000);
-	if (err) {
+	err = usb_control_msg(ctx->udev,
+				usb_rcvctrlpipe(ctx->udev, 0),
+				USB_CDC_GET_NTB_PARAMETERS,
+				USB_TYPE_CLASS | USB_DIR_IN
+				 | USB_RECIP_INTERFACE,
+				0, iface_no, &ctx->ncm_parm,
+				sizeof(ctx->ncm_parm), 10000);
+	if (err < 0) {
 		pr_debug("failed GET_NTB_PARAMETERS\n");
 		return 1;
 	}

@@ -254,31 +227,26 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 
 	/* inform device about NTB input size changes */
 	if (ctx->rx_max != le32_to_cpu(ctx->ncm_parm.dwNtbInMaxSize)) {
-		req.bmRequestType = USB_TYPE_CLASS | USB_DIR_OUT |
-							USB_RECIP_INTERFACE;
-		req.bNotificationType = USB_CDC_SET_NTB_INPUT_SIZE;
-		req.wValue = 0;
-		req.wIndex = cpu_to_le16(iface_no);
 
 		if (flags & USB_CDC_NCM_NCAP_NTB_INPUT_SIZE) {
 			struct usb_cdc_ncm_ndp_input_size ndp_in_sz;
-
-			req.wLength = 8;
-			ndp_in_sz.dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
-			ndp_in_sz.wNtbInMaxDatagrams =
-					cpu_to_le16(CDC_NCM_DPT_DATAGRAMS_MAX);
-			ndp_in_sz.wReserved = 0;
-			err = cdc_ncm_do_request(ctx, &req, &ndp_in_sz, 0, NULL,
-									1000);
+			err = usb_control_msg(ctx->udev,
+					usb_sndctrlpipe(ctx->udev, 0),
+					USB_CDC_SET_NTB_INPUT_SIZE,
+					USB_TYPE_CLASS | USB_DIR_OUT
+					 | USB_RECIP_INTERFACE,
+					0, iface_no, &ndp_in_sz, 8, 1000);
 		} else {
 			__le32 dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
-
-			req.wLength = 4;
-			err = cdc_ncm_do_request(ctx, &req, &dwNtbInMaxSize, 0,
-								NULL, 1000);
+			err = usb_control_msg(ctx->udev,
+					usb_sndctrlpipe(ctx->udev, 0),
+					USB_CDC_SET_NTB_INPUT_SIZE,
+					USB_TYPE_CLASS | USB_DIR_OUT
+					 | USB_RECIP_INTERFACE,
+					0, iface_no, &dwNtbInMaxSize, 4, 1000);
 		}
 
-		if (err)
+		if (err < 0)
 			pr_debug("Setting NTB Input Size failed\n");
 	}
 

@@ -333,29 +301,24 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 
 	/* set CRC Mode */
 	if (flags & USB_CDC_NCM_NCAP_CRC_MODE) {
-		req.bmRequestType = USB_TYPE_CLASS | USB_DIR_OUT |
-							USB_RECIP_INTERFACE;
-		req.bNotificationType = USB_CDC_SET_CRC_MODE;
-		req.wValue = cpu_to_le16(USB_CDC_NCM_CRC_NOT_APPENDED);
-		req.wIndex = cpu_to_le16(iface_no);
-		req.wLength = 0;
-
-		err = cdc_ncm_do_request(ctx, &req, NULL, 0, NULL, 1000);
-		if (err)
+		err = usb_control_msg(ctx->udev, usb_sndctrlpipe(ctx->udev, 0),
+				USB_CDC_SET_CRC_MODE,
+				USB_TYPE_CLASS | USB_DIR_OUT
+				 | USB_RECIP_INTERFACE,
+				USB_CDC_NCM_CRC_NOT_APPENDED,
+				iface_no, NULL, 0, 1000);
+		if (err < 0)
 			pr_debug("Setting CRC mode off failed\n");
 	}
 
 	/* set NTB format, if both formats are supported */
 	if (ntb_fmt_supported & USB_CDC_NCM_NTH32_SIGN) {
-		req.bmRequestType = USB_TYPE_CLASS | USB_DIR_OUT |
-							USB_RECIP_INTERFACE;
-		req.bNotificationType = USB_CDC_SET_NTB_FORMAT;
-		req.wValue = cpu_to_le16(USB_CDC_NCM_NTB16_FORMAT);
-		req.wIndex = cpu_to_le16(iface_no);
-		req.wLength = 0;
-
-		err = cdc_ncm_do_request(ctx, &req, NULL, 0, NULL, 1000);
-		if (err)
+		err = usb_control_msg(ctx->udev, usb_sndctrlpipe(ctx->udev, 0),
+				USB_CDC_SET_NTB_FORMAT, USB_TYPE_CLASS
+				 | USB_DIR_OUT | USB_RECIP_INTERFACE,
+				USB_CDC_NCM_NTB16_FORMAT,
+				iface_no, NULL, 0, 1000);
+		if (err < 0)
 			pr_debug("Setting NTB format to 16-bit failed\n");
 	}
 

@@ -365,17 +328,13 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 	if (flags & USB_CDC_NCM_NCAP_MAX_DATAGRAM_SIZE) {
 		__le16 max_datagram_size;
 		u16 eth_max_sz = le16_to_cpu(ctx->ether_desc->wMaxSegmentSize);
-
-		req.bmRequestType = USB_TYPE_CLASS | USB_DIR_IN |
-							USB_RECIP_INTERFACE;
-		req.bNotificationType = USB_CDC_GET_MAX_DATAGRAM_SIZE;
-		req.wValue = 0;
-		req.wIndex = cpu_to_le16(iface_no);
-		req.wLength = cpu_to_le16(2);
-
-		err = cdc_ncm_do_request(ctx, &req, &max_datagram_size, 0, NULL,
-									1000);
-		if (err) {
+		err = usb_control_msg(ctx->udev, usb_rcvctrlpipe(ctx->udev, 0),
+				USB_CDC_GET_MAX_DATAGRAM_SIZE,
+				USB_TYPE_CLASS | USB_DIR_IN
+				 | USB_RECIP_INTERFACE,
+				0, iface_no, &max_datagram_size,
+				2, 1000);
+		if (err < 0) {
 			pr_debug("GET_MAX_DATAGRAM_SIZE failed, use size=%u\n",
 						CDC_NCM_MIN_DATAGRAM_SIZE);
 		} else {

@@ -396,17 +355,15 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 					CDC_NCM_MIN_DATAGRAM_SIZE;
 
 			/* if value changed, update device */
-			req.bmRequestType = USB_TYPE_CLASS | USB_DIR_OUT |
-							USB_RECIP_INTERFACE;
-			req.bNotificationType = USB_CDC_SET_MAX_DATAGRAM_SIZE;
-			req.wValue = 0;
-			req.wIndex = cpu_to_le16(iface_no);
-			req.wLength = 2;
-			max_datagram_size = cpu_to_le16(ctx->max_datagram_size);
-
-			err = cdc_ncm_do_request(ctx, &req, &max_datagram_size,
-								0, NULL, 1000);
-			if (err)
+			err = usb_control_msg(ctx->udev,
+						usb_sndctrlpipe(ctx->udev, 0),
+						USB_CDC_SET_MAX_DATAGRAM_SIZE,
+						USB_TYPE_CLASS | USB_DIR_OUT
+						 | USB_RECIP_INTERFACE,
+						0,
+						iface_no, &max_datagram_size,
+						2, 1000);
+			if (err < 0)
 				pr_debug("SET_MAX_DATAGRAM_SIZE failed\n");
 		}
 

@@ -672,7 +629,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 	u32 rem;
 	u32 offset;
 	u32 last_offset;
-	u16 n = 0;
+	u16 n = 0, index;
 	u8 ready2send = 0;
 
 	/* if there is a remaining skb, it gets priority */

@@ -860,8 +817,8 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 					cpu_to_le16(sizeof(ctx->tx_ncm.nth16));
 	ctx->tx_ncm.nth16.wSequence = cpu_to_le16(ctx->tx_seq);
 	ctx->tx_ncm.nth16.wBlockLength = cpu_to_le16(last_offset);
-	ctx->tx_ncm.nth16.wNdpIndex = ALIGN(sizeof(struct usb_cdc_ncm_nth16),
-							ctx->tx_ndp_modulus);
+	index = ALIGN(sizeof(struct usb_cdc_ncm_nth16), ctx->tx_ndp_modulus);
+	ctx->tx_ncm.nth16.wNdpIndex = cpu_to_le16(index);
 
 	memcpy(skb_out->data, &(ctx->tx_ncm.nth16), sizeof(ctx->tx_ncm.nth16));
 	ctx->tx_seq++;

@@ -874,12 +831,11 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 	ctx->tx_ncm.ndp16.wLength = cpu_to_le16(rem);
 	ctx->tx_ncm.ndp16.wNextNdpIndex = 0; /* reserved */
 
-	memcpy(((u8 *)skb_out->data) + ctx->tx_ncm.nth16.wNdpIndex,
+	memcpy(((u8 *)skb_out->data) + index,
 						&(ctx->tx_ncm.ndp16),
 						sizeof(ctx->tx_ncm.ndp16));
 
-	memcpy(((u8 *)skb_out->data) + ctx->tx_ncm.nth16.wNdpIndex +
-					sizeof(ctx->tx_ncm.ndp16),
+	memcpy(((u8 *)skb_out->data) + index + sizeof(ctx->tx_ncm.ndp16),
 					&(ctx->tx_ncm.dpe16),
 					(ctx->tx_curr_frame_num + 1) *
 					sizeof(struct usb_cdc_ncm_dpe16));
Signature · 6 lines
1.7.5.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
maintainer
#5 lore raw

From: Giuseppe Scrivano <[email protected]>
Date: Wed, 03 Aug 2011 17:46:43 +0200

Quoted 6 lines
Quoted 1 line

From 7ba49d858103acb2ce4043127e3512ea29dff307 Mon Sep 17 00:00:00 2001

From: Giuseppe Scrivano <[email protected]>
Date: Fri, 15 Jul 2011 15:34:14 +0200
Subject: [PATCH] cdc_ncm: fix endianess problem.

Signed-off-by Giuseppe Scrivano <[email protected]>

The patch looks great, but please repost this fresh with a more vebose commit log message explaining all the things we discussed to get the change to it's current form.

Thanks!

A message that is not in the archive (sent off-list or not yet received): 20110803.191141.888495095889424492.davem-fT/PcQaiUtIeIZ0/[email protected]

#7 lore raw

Hello David,

Quoted 3 lines · David Miller

The patch looks great, but please repost this fresh with a more vebose commit log message explaining all the things we discussed to get the change to it's current form.

thanks for your review! I hope I was enough verbose this time :-)

Regards,
Giuseppe

Quoted 1 line

From 2ad27dac918eea4d39f3cc10796f4f08ead7210b Mon Sep 17 00:00:00 2001

From: Giuseppe Scrivano <[email protected]>
Date: Fri, 15 Jul 2011 15:34:14 +0200
Subject: [PATCH] cdc_ncm: fix endianness problem.

Fix a misusage of the struct usb_cdc_notification to pass arguments to the usb_control_msg function. The usb_control_msg function expects host endian arguments but usb_cdc_notification stores these values as little endian.

Now usb_control_msg is directly invoked with host endian values.

Signed-off-by Giuseppe Scrivano <[email protected]>
---
 drivers/net/usb/cdc_ncm.c |  156 ++++++++++++++++-----------------------------
 1 files changed, 56 insertions(+), 100 deletions(-)
drivers/net/usb/cdc_ncm.c +56 -100
index f33ca6a..d3b9e95 100644
@@ -54,7 +54,7 @@
 #include <linux/usb/usbnet.h>
 #include <linux/usb/cdc.h>
 
-#define	DRIVER_VERSION				"01-June-2011"
+#define	DRIVER_VERSION				"04-Aug-2011"
 
 /* CDC NCM subclass 3.2.1 */
 #define USB_CDC_NCM_NDP16_LENGTH_MIN		0x10

@@ -164,35 +164,8 @@ 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 int
-cdc_ncm_do_request(struct cdc_ncm_ctx *ctx, struct usb_cdc_notification *req,
-		   void *data, u16 flags, u16 *actlen, u16 timeout)
-{
-	int err;
-
-	err = usb_control_msg(ctx->udev, (req->bmRequestType & USB_DIR_IN) ?
-				usb_rcvctrlpipe(ctx->udev, 0) :
-				usb_sndctrlpipe(ctx->udev, 0),
-				req->bNotificationType, req->bmRequestType,
-				req->wValue,
-				req->wIndex, data,
-				req->wLength, timeout);
-
-	if (err < 0) {
-		if (actlen)
-			*actlen = 0;
-		return err;
-	}
-
-	if (actlen)
-		*actlen = err;
-
-	return 0;
-}
-
 static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 {
-	struct usb_cdc_notification req;
 	u32 val;
 	u8 flags;
 	u8 iface_no;

@@ -201,14 +174,14 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 
 	iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
 
-	req.bmRequestType = USB_TYPE_CLASS | USB_DIR_IN | USB_RECIP_INTERFACE;
-	req.bNotificationType = USB_CDC_GET_NTB_PARAMETERS;
-	req.wValue = 0;
-	req.wIndex = cpu_to_le16(iface_no);
-	req.wLength = cpu_to_le16(sizeof(ctx->ncm_parm));
-
-	err = cdc_ncm_do_request(ctx, &req, &ctx->ncm_parm, 0, NULL, 1000);
-	if (err) {
+	err = usb_control_msg(ctx->udev,
+				usb_rcvctrlpipe(ctx->udev, 0),
+				USB_CDC_GET_NTB_PARAMETERS,
+				USB_TYPE_CLASS | USB_DIR_IN
+				 | USB_RECIP_INTERFACE,
+				0, iface_no, &ctx->ncm_parm,
+				sizeof(ctx->ncm_parm), 10000);
+	if (err < 0) {
 		pr_debug("failed GET_NTB_PARAMETERS\n");
 		return 1;
 	}

@@ -254,31 +227,26 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 
 	/* inform device about NTB input size changes */
 	if (ctx->rx_max != le32_to_cpu(ctx->ncm_parm.dwNtbInMaxSize)) {
-		req.bmRequestType = USB_TYPE_CLASS | USB_DIR_OUT |
-							USB_RECIP_INTERFACE;
-		req.bNotificationType = USB_CDC_SET_NTB_INPUT_SIZE;
-		req.wValue = 0;
-		req.wIndex = cpu_to_le16(iface_no);
 
 		if (flags & USB_CDC_NCM_NCAP_NTB_INPUT_SIZE) {
 			struct usb_cdc_ncm_ndp_input_size ndp_in_sz;
-
-			req.wLength = 8;
-			ndp_in_sz.dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
-			ndp_in_sz.wNtbInMaxDatagrams =
-					cpu_to_le16(CDC_NCM_DPT_DATAGRAMS_MAX);
-			ndp_in_sz.wReserved = 0;
-			err = cdc_ncm_do_request(ctx, &req, &ndp_in_sz, 0, NULL,
-									1000);
+			err = usb_control_msg(ctx->udev,
+					usb_sndctrlpipe(ctx->udev, 0),
+					USB_CDC_SET_NTB_INPUT_SIZE,
+					USB_TYPE_CLASS | USB_DIR_OUT
+					 | USB_RECIP_INTERFACE,
+					0, iface_no, &ndp_in_sz, 8, 1000);
 		} else {
 			__le32 dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
-
-			req.wLength = 4;
-			err = cdc_ncm_do_request(ctx, &req, &dwNtbInMaxSize, 0,
-								NULL, 1000);
+			err = usb_control_msg(ctx->udev,
+					usb_sndctrlpipe(ctx->udev, 0),
+					USB_CDC_SET_NTB_INPUT_SIZE,
+					USB_TYPE_CLASS | USB_DIR_OUT
+					 | USB_RECIP_INTERFACE,
+					0, iface_no, &dwNtbInMaxSize, 4, 1000);
 		}
 
-		if (err)
+		if (err < 0)
 			pr_debug("Setting NTB Input Size failed\n");
 	}
 

@@ -333,29 +301,24 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 
 	/* set CRC Mode */
 	if (flags & USB_CDC_NCM_NCAP_CRC_MODE) {
-		req.bmRequestType = USB_TYPE_CLASS | USB_DIR_OUT |
-							USB_RECIP_INTERFACE;
-		req.bNotificationType = USB_CDC_SET_CRC_MODE;
-		req.wValue = cpu_to_le16(USB_CDC_NCM_CRC_NOT_APPENDED);
-		req.wIndex = cpu_to_le16(iface_no);
-		req.wLength = 0;
-
-		err = cdc_ncm_do_request(ctx, &req, NULL, 0, NULL, 1000);
-		if (err)
+		err = usb_control_msg(ctx->udev, usb_sndctrlpipe(ctx->udev, 0),
+				USB_CDC_SET_CRC_MODE,
+				USB_TYPE_CLASS | USB_DIR_OUT
+				 | USB_RECIP_INTERFACE,
+				USB_CDC_NCM_CRC_NOT_APPENDED,
+				iface_no, NULL, 0, 1000);
+		if (err < 0)
 			pr_debug("Setting CRC mode off failed\n");
 	}
 
 	/* set NTB format, if both formats are supported */
 	if (ntb_fmt_supported & USB_CDC_NCM_NTH32_SIGN) {
-		req.bmRequestType = USB_TYPE_CLASS | USB_DIR_OUT |
-							USB_RECIP_INTERFACE;
-		req.bNotificationType = USB_CDC_SET_NTB_FORMAT;
-		req.wValue = cpu_to_le16(USB_CDC_NCM_NTB16_FORMAT);
-		req.wIndex = cpu_to_le16(iface_no);
-		req.wLength = 0;
-
-		err = cdc_ncm_do_request(ctx, &req, NULL, 0, NULL, 1000);
-		if (err)
+		err = usb_control_msg(ctx->udev, usb_sndctrlpipe(ctx->udev, 0),
+				USB_CDC_SET_NTB_FORMAT, USB_TYPE_CLASS
+				 | USB_DIR_OUT | USB_RECIP_INTERFACE,
+				USB_CDC_NCM_NTB16_FORMAT,
+				iface_no, NULL, 0, 1000);
+		if (err < 0)
 			pr_debug("Setting NTB format to 16-bit failed\n");
 	}
 

@@ -365,17 +328,13 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 	if (flags & USB_CDC_NCM_NCAP_MAX_DATAGRAM_SIZE) {
 		__le16 max_datagram_size;
 		u16 eth_max_sz = le16_to_cpu(ctx->ether_desc->wMaxSegmentSize);
-
-		req.bmRequestType = USB_TYPE_CLASS | USB_DIR_IN |
-							USB_RECIP_INTERFACE;
-		req.bNotificationType = USB_CDC_GET_MAX_DATAGRAM_SIZE;
-		req.wValue = 0;
-		req.wIndex = cpu_to_le16(iface_no);
-		req.wLength = cpu_to_le16(2);
-
-		err = cdc_ncm_do_request(ctx, &req, &max_datagram_size, 0, NULL,
-									1000);
-		if (err) {
+		err = usb_control_msg(ctx->udev, usb_rcvctrlpipe(ctx->udev, 0),
+				USB_CDC_GET_MAX_DATAGRAM_SIZE,
+				USB_TYPE_CLASS | USB_DIR_IN
+				 | USB_RECIP_INTERFACE,
+				0, iface_no, &max_datagram_size,
+				2, 1000);
+		if (err < 0) {
 			pr_debug("GET_MAX_DATAGRAM_SIZE failed, use size=%u\n",
 						CDC_NCM_MIN_DATAGRAM_SIZE);
 		} else {

@@ -396,17 +355,15 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 					CDC_NCM_MIN_DATAGRAM_SIZE;
 
 			/* if value changed, update device */
-			req.bmRequestType = USB_TYPE_CLASS | USB_DIR_OUT |
-							USB_RECIP_INTERFACE;
-			req.bNotificationType = USB_CDC_SET_MAX_DATAGRAM_SIZE;
-			req.wValue = 0;
-			req.wIndex = cpu_to_le16(iface_no);
-			req.wLength = 2;
-			max_datagram_size = cpu_to_le16(ctx->max_datagram_size);
-
-			err = cdc_ncm_do_request(ctx, &req, &max_datagram_size,
-								0, NULL, 1000);
-			if (err)
+			err = usb_control_msg(ctx->udev,
+						usb_sndctrlpipe(ctx->udev, 0),
+						USB_CDC_SET_MAX_DATAGRAM_SIZE,
+						USB_TYPE_CLASS | USB_DIR_OUT
+						 | USB_RECIP_INTERFACE,
+						0,
+						iface_no, &max_datagram_size,
+						2, 1000);
+			if (err < 0)
 				pr_debug("SET_MAX_DATAGRAM_SIZE failed\n");
 		}
 

@@ -672,7 +629,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 	u32 rem;
 	u32 offset;
 	u32 last_offset;
-	u16 n = 0;
+	u16 n = 0, index;
 	u8 ready2send = 0;
 
 	/* if there is a remaining skb, it gets priority */

@@ -860,8 +817,8 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 					cpu_to_le16(sizeof(ctx->tx_ncm.nth16));
 	ctx->tx_ncm.nth16.wSequence = cpu_to_le16(ctx->tx_seq);
 	ctx->tx_ncm.nth16.wBlockLength = cpu_to_le16(last_offset);
-	ctx->tx_ncm.nth16.wNdpIndex = ALIGN(sizeof(struct usb_cdc_ncm_nth16),
-							ctx->tx_ndp_modulus);
+	index = ALIGN(sizeof(struct usb_cdc_ncm_nth16), ctx->tx_ndp_modulus);
+	ctx->tx_ncm.nth16.wNdpIndex = cpu_to_le16(index);
 
 	memcpy(skb_out->data, &(ctx->tx_ncm.nth16), sizeof(ctx->tx_ncm.nth16));
 	ctx->tx_seq++;

@@ -874,12 +831,11 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 	ctx->tx_ncm.ndp16.wLength = cpu_to_le16(rem);
 	ctx->tx_ncm.ndp16.wNextNdpIndex = 0; /* reserved */
 
-	memcpy(((u8 *)skb_out->data) + ctx->tx_ncm.nth16.wNdpIndex,
+	memcpy(((u8 *)skb_out->data) + index,
 						&(ctx->tx_ncm.ndp16),
 						sizeof(ctx->tx_ncm.ndp16));
 
-	memcpy(((u8 *)skb_out->data) + ctx->tx_ncm.nth16.wNdpIndex +
-					sizeof(ctx->tx_ncm.ndp16),
+	memcpy(((u8 *)skb_out->data) + index + sizeof(ctx->tx_ncm.ndp16),
 					&(ctx->tx_ncm.dpe16),
 					(ctx->tx_curr_frame_num + 1) *
 					sizeof(struct usb_cdc_ncm_dpe16));
Signature · 5 lines
1.7.5.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
maintainer Applied
#8 lore raw

From: Giuseppe Scrivano <[email protected]>
Date: Thu, 04 Aug 2011 10:10:29 +0200

Quoted 7 lines

cdc_ncm: fix endianness problem.

Fix a misusage of the struct usb_cdc_notification to pass arguments to the usb_control_msg function. The usb_control_msg function expects host endian arguments but usb_cdc_notification stores these values as little endian.

Now usb_control_msg is directly invoked with host endian values.

Applied.