Am Donnerstag, 10. Juni 2010 16:22:54 schrieb Sjur BRENDELAND: > Hi Oliver. > > Oliver Neukum wrote: > >> We need a timeout when to send out the USB frame, because we are > >> accumulating Ethernet packets and don't know when we have got the last > >> one in the outgoing network queue. The cdc_ncm_tx_timeout_start() > >> function is called at every TX, but we don't reset the timer at every > >> TX, which means we have to check timer_pending() to avoid re-starting > >> the timer before it has elapsed. > > You can use mod_timer(). > > > Technically we could use mod_timer(). The only problem I see is that using > mod_timer() at every ethernet TX frame would significantly extend the > timeout before the actual TX, if the outgoing Ethernet packets comes every 0.8ms > while the timeout is 1.0ms. Currently 32-ethernet packets can be accumulated, > which means that with mod_timer() we could have an effective delay of 32*1.0ms. Yes, I see. > > The HCD must not become idle. To accomplish that you use two buffers. > > As the first buffer is finished, the HCD starts transfering the second > > buffer > > and does a callback on the first buffer. > > Upon that callback you must submit a new buffer (or technically its > > URB). > > To do so the best way is to have a third buffer which you fill while > > two > > URBs are in flight and submit as the first buffer is finished. Then you > > start filling the old first buffer. > > > > No timer and optimal performance. > > > The USB-net layer which the CDC-NCM driver is using, will do the kind of buffering you are referring to. We have tried to keep our changes isolated to the CDC-NCM module. > > In the future we could consider move the Ethernet packet accumulation functionality into the USB-net module, which is one layer up. This is a good idea, but it doesn't affect the algorithm. You cannot get optimal performance without multiple buffers. If you submit a new buffer from the callback you will miss a frame. > > You tell usbcore to use autosuspend but you include no support for > > detecting > > idleness in the driver. As you hook into usbnet you can use the > > framework > > usbnet provides. But if you do that you must implement the > > "manage_power" > > callback like cdc-ether does. You do neither. So how do you expect this > > to work? > > We cannot find any references to "manage_power" under drivers/usb? And we cannot see that the cdc-ether.c driver does more than setting the "udriver->supports_autosuspend" flag. Could you please be a little bit more specific, perhaps showing us fragments of some example code that utilize USB power save on Linux? The implementation of cdc-ether is minimal. The hook is intended for devices which need a specific control sequence. You can probably get away with using the implementation of cdc-ether in cdc-ncm. It is simple but essential. I guess you get away with it in your tests is that you also have a cdc-wdm interface on the device. And please directly call usbnet_suspend/resume Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
