On Saturday 09 February 2008 10:51:06 David Brownell wrote: > 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?
Well, I could certainly give it a try. However I'd like to point out a few thing. 1) in the actual usbnet code, dev->out and dev->in are never set in case of a bind() driver. As a result the computed dev->maxpacket is suspicious. 2) I could set the dev->maxpacket in the rndis_host driver but you should notice that whatever I set it to, it will be overwritten by usbnet afterward (and based on a bogus dev->out). 3) doing the change in the bind() function (without touching usbnet) would mean that any bind() driver (I don't know if there are some other ones) will have to do the same. Is this a contract that we can force on any bind() driver? In the end it seems to me that dev->in, dev->out and dev->maxpacket are generic enough values to be computed in one place whatever driver (bind() or not) attach to usbnet. bind() could then recompute them if need be but is not forced to. In any case it seems to be the usbnet job to set these values (as it is doing it now but late) and computing them before bind() seems cleaner to me. Regards JC - 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
