On Tue, Mar 08, 2005 at 09:22:20AM -0800, David Brownell wrote: > On Tuesday 08 March 2005 9:05 am, Alan Stern wrote: > > On Mon, 7 Mar 2005, Nishanth Aravamudan wrote: > > > > > - int maxretry = 50; > > > + unsigned long timeout = 500; > > > > _Please_ don't set timeout to 500 jiffies! Use msecs_to_jiffies(500) > > instead. > > Hmm, somewhere in the queue are patches changing all timeouts to > milliseconds ... but not for the primitive used in this patch.
Well, I did that for usb_bulk_msg() and usb_ctrl_msg(). It's a much more daunting effort (but will be done!) to convert all timeouts everywhere, not just in USB, to sane time units. I'm working on it, though :) > > Note that schedule_timeout can return before the timeout has elapsed, even > > though the condition you're waiting for (URB completed) hasn't happened > > yet. People use a loop to catch these things, and you definitely need a > > way to test for URB completed. But seeing if ep->urb->status is equal to > > -EINPROGRESS is not the correct way to do it. I see that the completion > > handler, usb_write_callback, doesn't do anything but wake up the > > waitqueue. That's not good enough; it needs to set a flag in the ep > > structure for you to test. > > All true. Seems like if there were an interruptible version of > usb_bulk_msg(), that's what this driver should use ... and lacking > that, it's pretty much re-inventing that logic. Wouldn't a better > fix just be a new usb_bulk_msg_interruptible(), which uses one of > the interruptible variants of wait_for_completion() ? After all, > one of the longstanding API glitches in usb_bulk_msg() has been > exactly that it's not interruptible ... hence inappropriate for > use from syscalls including read() and write(). This is something for USB developers to figure out if they want, but I support it. Thanks, Nish ------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
