Mark Ellis
2010-02-07 19:39:05 UTC
Below is a patch we've had at the synce project for some time now, I was
hoping for some guidance. None of us are really kernel level developers,
but we flounder around where we can.
We'd observed a few rndis Windows Mobile devices giving -100 errors on
connection, and one of our sometime contributors, John Carr, came up
with the attached patch by comparing the kernel module with a userspace
test driver that was developed by someone that left the project some
time ago.
This patch seems to be very effective, but as I mentioned, none of us
are really device side developers, and John was never completely
convinced this was correct. Quoting him from a while back....
"The reason I consider it dirty is that it needs the
rndis_get_in_endpoint function which is called every time something
calls rndis_command.
Ideally that should run once when the driver loads for a given device,
but i felt like i was being too invasive when I started doing that and
whimped out.
The patch exists through careful comparison of the deprecated user
mode and the current kernel mode implementations of usb-rndis, the
poking of an INT IN endpoint being the only difference I found."
So we think we are on the right lines, but perhaps not fixing the
problem in the correct way. It would be great if someone could take a
look at this, and either say it's fine for submission, or point us in a
more appropriate direction.
Many thanks
Mark
---
diff -Nurp a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
--- a/drivers/net/usb/rndis_host.c 2009-09-09 23:13:59.000000000 +0100
+++ b/drivers/net/usb/rndis_host.c 2010-02-07 18:59:21.341929763 +0000
@@ -64,6 +64,25 @@ void rndis_status(struct usbnet *dev, st
}
EXPORT_SYMBOL_GPL(rndis_status);
+/* Function ripped from keyspan_remote.c */
+static struct usb_endpoint_descriptor *rndis_get_in_endpoint(struct
usb_host_interface *iface)
+{
+
+ struct usb_endpoint_descriptor *endpoint;
+ int i;
+
+ for (i = 0; i < iface->desc.bNumEndpoints; ++i) {
+ endpoint = &iface->endpoint[i].desc;
+
+ if (usb_endpoint_is_int_in(endpoint)) {
+ /* we found our interrupt in endpoint */
+ return endpoint;
+ }
+ }
+
+ return NULL;
+}
+
/*
* RPC done RNDIS-style. Caller guarantees:
* - message is properly byteswapped
@@ -77,11 +96,14 @@ EXPORT_SYMBOL_GPL(rndis_status);
int rndis_command(struct usbnet *dev, struct rndis_msg_hdr *buf, int
buflen)
{
struct cdc_state *info = (void *) &dev->data;
+ struct usb_endpoint_descriptor *endpoint;
int master_ifnum;
int retval;
unsigned count;
__le32 rsp;
u32 xid = 0, msg_len, request_id;
+ char int_buf[128];
+ int maxp, pipe, partial;
/* REVISIT when this gets called from contexts other than probe() or
* disconnect(): either serialize, or dispatch responses on xid
@@ -110,6 +132,29 @@ int rndis_command(struct usbnet *dev, st
// we time out and cancel our "get response" requests...
// so, this is fragile. Probably need to poll for status.
+ /* FIXME This feels rancid */
+ endpoint =
rndis_get_in_endpoint(info->control->cur_altsetting);
+ pipe = usb_rcvintpipe(dev->udev, endpoint->bEndpointAddress);
+ maxp = usb_maxpacket(dev->udev, pipe, usb_pipeout(pipe));
+
+ retval = usb_interrupt_msg(dev->udev,
+ pipe,
+ int_buf,
+ (maxp > 8 ? 8 : maxp),
+ &partial,
+ RNDIS_CONTROL_TIMEOUT_MS);
+
+ dev_dbg(&info->control->dev,
+ "pipe: %d, maxp: %d, partial: %d, retval: %d\n",
+ pipe,
+ maxp,
+ partial,
+ retval);
+
+ /* I /think/ usb_interrupt_msg blocks and returns < 0 for error
*/
+ if (unlikely(retval < 0))
+ return retval;
+
/* ignore status endpoint, just poll the control channel;
* the request probably completed immediately
*/
hoping for some guidance. None of us are really kernel level developers,
but we flounder around where we can.
We'd observed a few rndis Windows Mobile devices giving -100 errors on
connection, and one of our sometime contributors, John Carr, came up
with the attached patch by comparing the kernel module with a userspace
test driver that was developed by someone that left the project some
time ago.
This patch seems to be very effective, but as I mentioned, none of us
are really device side developers, and John was never completely
convinced this was correct. Quoting him from a while back....
"The reason I consider it dirty is that it needs the
rndis_get_in_endpoint function which is called every time something
calls rndis_command.
Ideally that should run once when the driver loads for a given device,
but i felt like i was being too invasive when I started doing that and
whimped out.
The patch exists through careful comparison of the deprecated user
mode and the current kernel mode implementations of usb-rndis, the
poking of an INT IN endpoint being the only difference I found."
So we think we are on the right lines, but perhaps not fixing the
problem in the correct way. It would be great if someone could take a
look at this, and either say it's fine for submission, or point us in a
more appropriate direction.
Many thanks
Mark
---
diff -Nurp a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
--- a/drivers/net/usb/rndis_host.c 2009-09-09 23:13:59.000000000 +0100
+++ b/drivers/net/usb/rndis_host.c 2010-02-07 18:59:21.341929763 +0000
@@ -64,6 +64,25 @@ void rndis_status(struct usbnet *dev, st
}
EXPORT_SYMBOL_GPL(rndis_status);
+/* Function ripped from keyspan_remote.c */
+static struct usb_endpoint_descriptor *rndis_get_in_endpoint(struct
usb_host_interface *iface)
+{
+
+ struct usb_endpoint_descriptor *endpoint;
+ int i;
+
+ for (i = 0; i < iface->desc.bNumEndpoints; ++i) {
+ endpoint = &iface->endpoint[i].desc;
+
+ if (usb_endpoint_is_int_in(endpoint)) {
+ /* we found our interrupt in endpoint */
+ return endpoint;
+ }
+ }
+
+ return NULL;
+}
+
/*
* RPC done RNDIS-style. Caller guarantees:
* - message is properly byteswapped
@@ -77,11 +96,14 @@ EXPORT_SYMBOL_GPL(rndis_status);
int rndis_command(struct usbnet *dev, struct rndis_msg_hdr *buf, int
buflen)
{
struct cdc_state *info = (void *) &dev->data;
+ struct usb_endpoint_descriptor *endpoint;
int master_ifnum;
int retval;
unsigned count;
__le32 rsp;
u32 xid = 0, msg_len, request_id;
+ char int_buf[128];
+ int maxp, pipe, partial;
/* REVISIT when this gets called from contexts other than probe() or
* disconnect(): either serialize, or dispatch responses on xid
@@ -110,6 +132,29 @@ int rndis_command(struct usbnet *dev, st
// we time out and cancel our "get response" requests...
// so, this is fragile. Probably need to poll for status.
+ /* FIXME This feels rancid */
+ endpoint =
rndis_get_in_endpoint(info->control->cur_altsetting);
+ pipe = usb_rcvintpipe(dev->udev, endpoint->bEndpointAddress);
+ maxp = usb_maxpacket(dev->udev, pipe, usb_pipeout(pipe));
+
+ retval = usb_interrupt_msg(dev->udev,
+ pipe,
+ int_buf,
+ (maxp > 8 ? 8 : maxp),
+ &partial,
+ RNDIS_CONTROL_TIMEOUT_MS);
+
+ dev_dbg(&info->control->dev,
+ "pipe: %d, maxp: %d, partial: %d, retval: %d\n",
+ pipe,
+ maxp,
+ partial,
+ retval);
+
+ /* I /think/ usb_interrupt_msg blocks and returns < 0 for error
*/
+ if (unlikely(retval < 0))
+ return retval;
+
/* ignore status endpoint, just poll the control channel;
* the request probably completed immediately
*/