Patchwork rndis_host: Poll status channel before control channel

login
register
mail settings
Submitter Ben Hutchings
Date April 19, 2010, 11:08 p.m.
Message ID <1271718508.2197.12.camel@localhost>
Download mbox | patch
Permalink /patch/50495/
State Accepted
Delegated to: David Miller
Headers show

Comments

Ben Hutchings - April 19, 2010, 11:08 p.m.
Some RNDIS devices don't respond on the control channel until polled
on the status channel.  In particular, this was reported to be the
case for the 2Wire HomePortal 1000SW.

This is roughly based on a patch by John Carr <john.carr@unrouted.co.uk>
which is reported to be needed for use with some Windows Mobile devices
and which is currently applied by Mandriva.

Reported-by: Mark Glassberg <vzeeaxwl@myfairpoint.net>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Tested-by: Mark Glassberg <vzeeaxwl@myfairpoint.net>
---
Note that this change hasn't yet been tested with any other RNDIS
devices.  John, can you confirm whether this also handles the WinMob
devices?

Ben.

 drivers/net/usb/rndis_host.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)
David Miller - April 28, 2010, 1:01 a.m.
From: Ben Hutchings <ben@decadent.org.uk>
Date: Tue, 20 Apr 2010 00:08:28 +0100

> Some RNDIS devices don't respond on the control channel until polled
> on the status channel.  In particular, this was reported to be the
> case for the 2Wire HomePortal 1000SW.
> 
> This is roughly based on a patch by John Carr <john.carr@unrouted.co.uk>
> which is reported to be needed for use with some Windows Mobile devices
> and which is currently applied by Mandriva.
> 
> Reported-by: Mark Glassberg <vzeeaxwl@myfairpoint.net>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> Tested-by: Mark Glassberg <vzeeaxwl@myfairpoint.net>
> ---
> Note that this change hasn't yet been tested with any other RNDIS
> devices.  John, can you confirm whether this also handles the WinMob
> devices?

I haven't seen any other testing for this patch and I'm real hesitant
to apply something like this in that case.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - May 13, 2010, 6:42 a.m.
From: Ben Hutchings <ben@decadent.org.uk>
Date: Tue, 20 Apr 2010 00:08:28 +0100

> Some RNDIS devices don't respond on the control channel until polled
> on the status channel.  In particular, this was reported to be the
> case for the 2Wire HomePortal 1000SW.
> 
> This is roughly based on a patch by John Carr <john.carr@unrouted.co.uk>
> which is reported to be needed for use with some Windows Mobile devices
> and which is currently applied by Mandriva.
> 
> Reported-by: Mark Glassberg <vzeeaxwl@myfairpoint.net>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> Tested-by: Mark Glassberg <vzeeaxwl@myfairpoint.net>
> ---
> Note that this change hasn't yet been tested with any other RNDIS
> devices.  John, can you confirm whether this also handles the WinMob
> devices?

Still waiting for this to get tested.  Is there really nobody in the
world with RNDIS devices who can test this patch?  If so, maybe that's
a good reason to not apply it :-))))
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings - May 15, 2010, 1:37 p.m.
On Wed, 2010-05-12 at 23:42 -0700, David Miller wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Tue, 20 Apr 2010 00:08:28 +0100
> 
> > Some RNDIS devices don't respond on the control channel until polled
> > on the status channel.  In particular, this was reported to be the
> > case for the 2Wire HomePortal 1000SW.
> > 
> > This is roughly based on a patch by John Carr <john.carr@unrouted.co.uk>
> > which is reported to be needed for use with some Windows Mobile devices
> > and which is currently applied by Mandriva.
> > 
> > Reported-by: Mark Glassberg <vzeeaxwl@myfairpoint.net>
> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > Tested-by: Mark Glassberg <vzeeaxwl@myfairpoint.net>
> > ---
> > Note that this change hasn't yet been tested with any other RNDIS
> > devices.  John, can you confirm whether this also handles the WinMob
> > devices?
> 
> Still waiting for this to get tested.  Is there really nobody in the
> world with RNDIS devices who can test this patch?  If so, maybe that's
> a good reason to not apply it :-))))

This has been in Debian unstable since 1 May and I haven't seen any
fall-out yet.  However I acknowledge that absence of evidence is not
evidence of absence.

Ben.
David Miller - May 16, 2010, 5:54 a.m.
From: Ben Hutchings <ben@decadent.org.uk>
Date: Sat, 15 May 2010 14:37:15 +0100

> On Wed, 2010-05-12 at 23:42 -0700, David Miller wrote:
>> From: Ben Hutchings <ben@decadent.org.uk>
>> Date: Tue, 20 Apr 2010 00:08:28 +0100
>> 
>> > Some RNDIS devices don't respond on the control channel until polled
>> > on the status channel.  In particular, this was reported to be the
>> > case for the 2Wire HomePortal 1000SW.
>> > 
>> > This is roughly based on a patch by John Carr <john.carr@unrouted.co.uk>
>> > which is reported to be needed for use with some Windows Mobile devices
>> > and which is currently applied by Mandriva.
>> > 
>> > Reported-by: Mark Glassberg <vzeeaxwl@myfairpoint.net>
>> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
>> > Tested-by: Mark Glassberg <vzeeaxwl@myfairpoint.net>
>> > ---
>> > Note that this change hasn't yet been tested with any other RNDIS
>> > devices.  John, can you confirm whether this also handles the WinMob
>> > devices?
>> 
>> Still waiting for this to get tested.  Is there really nobody in the
>> world with RNDIS devices who can test this patch?  If so, maybe that's
>> a good reason to not apply it :-))))
> 
> This has been in Debian unstable since 1 May and I haven't seen any
> fall-out yet.  However I acknowledge that absence of evidence is not
> evidence of absence.

I think I'll toss it into net-next-2.6 and we'll see if any monsters
come out of that.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
index f56dec6..52faca1 100644
--- a/drivers/net/usb/rndis_host.c
+++ b/drivers/net/usb/rndis_host.c
@@ -103,8 +103,10 @@  static void rndis_msg_indicate(struct usbnet *dev, struct rndis_indicate *msg,
 int rndis_command(struct usbnet *dev, struct rndis_msg_hdr *buf, int buflen)
 {
 	struct cdc_state	*info = (void *) &dev->data;
+	struct usb_cdc_notification notification;
 	int			master_ifnum;
 	int			retval;
+	int			partial;
 	unsigned		count;
 	__le32			rsp;
 	u32			xid = 0, msg_len, request_id;
@@ -132,13 +134,17 @@  int rndis_command(struct usbnet *dev, struct rndis_msg_hdr *buf, int buflen)
 	if (unlikely(retval < 0 || xid == 0))
 		return retval;
 
-	// FIXME Seems like some devices discard responses when
-	// we time out and cancel our "get response" requests...
-	// so, this is fragile.  Probably need to poll for status.
+	/* Some devices don't respond on the control channel until
+	 * polled on the status channel, so do that first. */
+	retval = usb_interrupt_msg(
+		dev->udev,
+		usb_rcvintpipe(dev->udev, dev->status->desc.bEndpointAddress),
+		&notification, sizeof(notification), &partial,
+		RNDIS_CONTROL_TIMEOUT_MS);
+	if (unlikely(retval < 0))
+		return retval;
 
-	/* ignore status endpoint, just poll the control channel;
-	 * the request probably completed immediately
-	 */
+	/* Poll the control channel; the request probably completed immediately */
 	rsp = buf->msg_type | RNDIS_MSG_COMPLETION;
 	for (count = 0; count < 10; count++) {
 		memset(buf, 0, CONTROL_BUFFER_SIZE);