synce-devel...

Didn't hear back from Ole André, so I thought I'd see what the kernel
usb guys said.

On Sun, 2010-02-07 at 20:39 +0100, Mark Ellis wrote:
> 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
>        */
> 

Attachment: signature.asc
Description: This is a digitally signed message part

------------------------------------------------------------------------------
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com
_______________________________________________
SynCE-Devel mailing list
SynCE-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/synce-devel

Reply via email to