mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 2.6.16] rndis_host/cdc_ether: add support for Windows Mobile 5 based devices
@ 2006-03-29 11:42 Ole André Vadla Ravnås
  2006-03-29 23:41 ` Andrew Morton
  0 siblings, 1 reply; 3+ messages in thread
From: Ole André Vadla Ravnås @ 2006-03-29 11:42 UTC (permalink / raw)
  To: linux-kernel

Windows Mobile 5 based devices speak RNDIS but lack the CDC and union
descriptors, and instead provide a couple of ACM descriptors. They
also expect the queries (RNDIS_MSG_QUERY) to have an appended buffer
of the same size as the response is expected to generate passed along
with the request. My understanding is that this is used to pass input
values, or left uninitialized when there is none, and also used for
the device to know what length the host expects. This patch fixes
these two issues and has been reported to work with several WM5 based
devices.

Signed-off-by: Ole Andre Vadla Ravnaas <oleavr@gmail.com>


diff -Nur linux-2.6.16-orig/drivers/usb/net/cdc_ether.c
linux-2.6.16/drivers/usb/net/cdc_ether.c
--- linux-2.6.16-orig/drivers/usb/net/cdc_ether.c	2006-03-20
06:53:29.000000000 +0100
+++ linux-2.6.16/drivers/usb/net/cdc_ether.c	2006-03-29 11:36:31.000000000 +0200
@@ -72,7 +72,8 @@
 	/* this assumes that if there's a non-RNDIS vendor variant
 	 * of cdc-acm, it'll fail RNDIS requests cleanly.
 	 */
-	rndis = (intf->cur_altsetting->desc.bInterfaceProtocol == 0xff);
+	rndis = (intf->cur_altsetting->desc.bInterfaceProtocol == 0xff ||
+		 intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_MISC);

 	memset(info, 0, sizeof *info);
 	info->control = intf;
@@ -172,6 +173,55 @@
 		buf += buf [0];
 	}

+	/* Windows Mobile 5 based RNDIS devices lack the CDC descriptors,
+	 * so to work around this without changing too much of the overall
+	 * logic we fake those headers here. */
+	if (intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_MISC) {
+		struct usb_cdc_header_desc *h;
+		struct usb_cdc_union_desc *u;
+
+		dev_dbg(&intf->dev, "taking WM5 workaround path\n");
+
+		/* allocate and fill in the missing structures */
+		h = kmalloc(sizeof(struct usb_cdc_header_desc), GFP_KERNEL);
+		if (!h)
+			return -ENOMEM;
+		info->header = h;
+
+		h->bLength = sizeof(struct usb_cdc_header_desc);
+		h->bDescriptorType = USB_DT_CS_INTERFACE;
+		h->bDescriptorSubType = USB_CDC_HEADER_TYPE;
+
+		h->bcdCDC = 0; /* FIXME */
+		
+		u = kmalloc(sizeof(struct usb_cdc_union_desc), GFP_KERNEL);
+		if (!u)
+			return -ENOMEM;
+		info->u = u;
+
+		u->bLength = sizeof(struct usb_cdc_union_desc);
+		u->bDescriptorType = USB_DT_CS_INTERFACE;
+		u->bDescriptorSubType = USB_CDC_UNION_TYPE;
+		
+		u->bMasterInterface0 = 0;
+		u->bSlaveInterface0 = 1;
+
+		/* initialize */
+		info->control = usb_ifnum_to_if(dev->udev,
+					info->u->bMasterInterface0);
+		info->data = usb_ifnum_to_if(dev->udev,
+					info->u->bSlaveInterface0);
+		if (!info->control || !info->data) {
+			dev_dbg(&intf->dev,
+				"master #%u/%p slave #%u/%p\n",
+				info->u->bMasterInterface0,
+				info->control,
+				info->u->bSlaveInterface0,
+				info->data);
+			goto bad_desc;
+		}
+	}
+	
 	if (!info->header || !info->u || (!rndis && !info->ether)) {
 		dev_dbg(&intf->dev, "missing cdc %s%s%sdescriptor\n",
 			info->header ? "" : "header ",
@@ -229,6 +279,15 @@
 	struct cdc_state		*info = (void *) &dev->data;
 	struct usb_driver		*driver = driver_of(intf);

+	/* clean up resources allocated by the Windows Mobile 5 RNDIS workaround */
+	if (intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_MISC) {
+		if (info->header)
+			kfree(info->header);
+
+		if (info->u)
+			kfree(info->u);
+	}
+
 	/* disconnect master --> disconnect slave */
 	if (intf == info->control && info->data) {
 		/* ensure immediate exit from usbnet_disconnect */
diff -Nur linux-2.6.16-orig/drivers/usb/net/rndis_host.c
linux-2.6.16/drivers/usb/net/rndis_host.c
--- linux-2.6.16-orig/drivers/usb/net/rndis_host.c	2006-03-20
06:53:29.000000000 +0100
+++ linux-2.6.16/drivers/usb/net/rndis_host.c	2006-03-29
11:38:49.000000000 +0200
@@ -410,10 +410,12 @@
 		1 << le32_to_cpu(u.init_c->packet_alignment));

 	/* get designated host ethernet address */
-	memset(u.get, 0, sizeof *u.get);
+	memset(u.get, 0, sizeof *u.get + 48);
 	u.get->msg_type = RNDIS_MSG_QUERY;
-	u.get->msg_len = ccpu2(sizeof *u.get);
+	u.get->msg_len = ccpu2(sizeof *u.get + 48);
 	u.get->oid = OID_802_3_PERMANENT_ADDRESS;
+	u.get->len = ccpu2(48);
+	u.get->offset = ccpu2(20);

 	retval = rndis_command(dev, u.header);
 	if (unlikely(retval < 0)) {
@@ -575,10 +577,14 @@

 /*-------------------------------------------------------------------------*/

+#define WM5_SUB_CLASS 0x01
+#define WM5_PROTOCOL  0x01
+
 static const struct usb_device_id	products [] = {
 {
 	/* RNDIS is MSFT's un-official variant of CDC ACM */
 	USB_INTERFACE_INFO(USB_CLASS_COMM, 2 /* ACM */, 0x0ff),
+	USB_INTERFACE_INFO(USB_CLASS_MISC, WM5_SUB_CLASS, WM5_PROTOCOL),
 	.driver_info = (unsigned long) &rndis_info,
 },
 	{ },		// END
diff -Nur linux-2.6.16-orig/include/linux/usb_ch9.h
linux-2.6.16/include/linux/usb_ch9.h
--- linux-2.6.16-orig/include/linux/usb_ch9.h	2006-03-20
06:53:29.000000000 +0100
+++ linux-2.6.16/include/linux/usb_ch9.h	2006-03-29 11:36:31.000000000 +0200
@@ -217,6 +217,7 @@
 #define USB_CLASS_CONTENT_SEC		0x0d	/* content security */
 #define USB_CLASS_VIDEO			0x0e
 #define USB_CLASS_WIRELESS_CONTROLLER	0xe0
+#define USB_CLASS_MISC			0xef
 #define USB_CLASS_APP_SPEC		0xfe
 #define USB_CLASS_VENDOR_SPEC		0xff

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

* Re: [PATCH 2.6.16] rndis_host/cdc_ether: add support for Windows Mobile 5 based devices
  2006-03-29 11:42 [PATCH 2.6.16] rndis_host/cdc_ether: add support for Windows Mobile 5 based devices Ole André Vadla Ravnås
@ 2006-03-29 23:41 ` Andrew Morton
  2006-03-30  3:12   ` Ole André Vadla Ravnås
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2006-03-29 23:41 UTC (permalink / raw)
  To: Ole André Vadla Ravnås ; +Cc: linux-kernel

"Ole André Vadla Ravnås" <oleavr@gmail.com> wrote:
>
> +		/* allocate and fill in the missing structures */
> +		h = kmalloc(sizeof(struct usb_cdc_header_desc), GFP_KERNEL);
> +		if (!h)
> +			return -ENOMEM;
> +		info->header = h;
> +
> +		h->bLength = sizeof(struct usb_cdc_header_desc);
> +		h->bDescriptorType = USB_DT_CS_INTERFACE;
> +		h->bDescriptorSubType = USB_CDC_HEADER_TYPE;
> +
> +		h->bcdCDC = 0; /* FIXME */
> +		
> +		u = kmalloc(sizeof(struct usb_cdc_union_desc), GFP_KERNEL);
> +		if (!u)
> +			return -ENOMEM;
> +		info->u = u;

If the second kmalloc() fails, do we leak the memory at `h'?

Please don't sprinkle `return' statements deep inside large and complex
functions.  It makes the code hard to audit and to modify and it invites
resource leaks and locking errors.  Instead, use `goto suitable_unwind_point;', thanks.



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

* Re: [PATCH 2.6.16] rndis_host/cdc_ether: add support for Windows Mobile 5 based devices
  2006-03-29 23:41 ` Andrew Morton
@ 2006-03-30  3:12   ` Ole André Vadla Ravnås
  0 siblings, 0 replies; 3+ messages in thread
From: Ole André Vadla Ravnås @ 2006-03-30  3:12 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

On 3/30/06, Andrew Morton <akpm@osdl.org> wrote:
> "Ole André Vadla Ravnås" <oleavr@gmail.com> wrote:
> >
> > +             /* allocate and fill in the missing structures */
> > +             h = kmalloc(sizeof(struct usb_cdc_header_desc), GFP_KERNEL);
> > +             if (!h)
> > +                     return -ENOMEM;
> > +             info->header = h;
> > +
> > +             h->bLength = sizeof(struct usb_cdc_header_desc);
> > +             h->bDescriptorType = USB_DT_CS_INTERFACE;
> > +             h->bDescriptorSubType = USB_CDC_HEADER_TYPE;
> > +
> > +             h->bcdCDC = 0; /* FIXME */
> > +
> > +             u = kmalloc(sizeof(struct usb_cdc_union_desc), GFP_KERNEL);
> > +             if (!u)
> > +                     return -ENOMEM;
> > +             info->u = u;
>
> If the second kmalloc() fails, do we leak the memory at `h'?
>
> Please don't sprinkle `return' statements deep inside large and complex
> functions.  It makes the code hard to audit and to modify and it invites
> resource leaks and locking errors.  Instead, use `goto suitable_unwind_point;', thanks.

Ahh, very good point there! Thanks for pointing it out!  I'll fix it,
wait for a comment from David, and then resubmit.

Cheers,
Ole Andre

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

end of thread, other threads:[~2006-03-30  3:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-03-29 11:42 [PATCH 2.6.16] rndis_host/cdc_ether: add support for Windows Mobile 5 based devices Ole André Vadla Ravnås
2006-03-29 23:41 ` Andrew Morton
2006-03-30  3:12   ` Ole André Vadla Ravnås

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®