On Friday 08 February 2008, Jean-Christophe Dubois wrote:
> 
> When the rndis_host bind() function is called (by usbnet_probe()) the 
> dev->maxpacket is not yet set (It will be set later in usbnet_probe()). 
> As it is 0 the computed dev->rx_urb_size is also 0 and therefore the 
> "max_transfer_size" sent to the device in the RNDIS_MSG_INIT message 
> is also 0. I am not sure how the device will react to this but obviously 
> this is a bogus value.

Right, this is a problem only for the RNDIS host code.  Good catch!


> So it seems dev->maxpacket needs to be set before calling the bind() 
> function (it makes some sense as rndis_bind() is using it). This means 
> we need to move up the call to usb_maxpacket() in the  usbnet_probe()
> function. But the call to usb_maxpacket() is requiring dev->out to be set 
> which in turn requires this computation to be moved above the call to 
> usb_maxpacket() 

A less intrusive fix would make rndis_host.c set dev->maxpacket before
using it, and after calling usbnet_generic_cdc_bind().  That would
be a one-line fix, affecting only the relevant driver ... much safer!

The problem I have with how you approached this is that you're changing
core code, and in a way that suggests all the minidriver bind() code
should be changed (to rely on this having been done already).  Now,
there are a fair number of devices that will "obviously" work fine
that way.  But for ones that split their data and control interfaces,
it's not quite obvious.  Which means all true CDC devices ... and even
the real RNDIS devices, as opposed to the ActiveSync version.

So I'd rather see a one-line fix to the RNDIS host code, instead of
this more elaborate approach.  Could you give that a try?

- Dave

 




-
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

Reply via email to