On Thursday 29 July 2004 13:18, Alan Stern wrote: > David: > > Our work on the hub driver is well on its way to getting hopelessly > tangled. Should we try to coordinate the changes we've been sending in > independently?
This looked OK to me with respect to RC2 and to what's in Greg's tree (except for some "endpoint_running" detritus in Greg's tree), so I'm not quite sure what you mean. > With regard to your latest patch, I have some questions/comments: > > There's no need to check hub->quiescing in the interrupt > handler, since usb_kill_urb() will prevent the URB from > being resubmitted. Maybe not, but it can't hurt either! And I prefer having such logic be explicit, rather than implicit. > Why do you want to use locktree() in hub_events() and > usb_reset_device()? Because of what it protects: everything downstream of that port, against use of those types of signaling. And that main khubd entry point certainly uses that kind of signaling, as do usb_{reset,suspend,resume}_device(), so they've all got to use the same locking. > The comment you added to usb_disconnect() about pdev pointing > into a locked hub isn't correct when the device being > disconnected is a root hub, and the comments that follow your > change already explain what's needed. So I'll suggest you send a followup patch. But that exception for root hubs looked troublesome to me anyway; any time you can't fit a locking constraint in one line of text, it's error prone. > The FIXME comments added to hub_port_disable() and hub_port_init() > are misleading, because disconnect() would run into trouble if > called from within usb_reset_device(). At one point I considered > doing the same thing but decided against it for that reason. Point is: when is disconnect() getting called? There are several paths where devices get marked as gone, but they never actually get disconnected. I don't care when it's called, but it shouldn't be too much later ... > Rather than fretting too much about unbinding drivers that don't > support suspend(), one could improve things a lot right now by > adding suspend/resume support to the HID driver. That won't address usb-storage, or any of the other drivers, though. (I tried to suspend my Swiss Army Knife USB-key, and it misbehaved rudely during suspend since something kept probing it through SCSI. Though it came back on resume, unlike HID.) Meanwhile we **KNOW** every USB driver probe()/disconnect() cycle is basically working now, so that the USB parts of the problem have a simple solution. Thing is, I think we want to go from where we are to an environment where USB suspend behaves _without_ having to change every single last driver both in-tree and external. Implementing suspend() and resume() callbacks should be just to add value; drivers without them don't need to act broken. Plus, I don't think anyone can seriously argue that it's reasonable for the driver model PM code to self-deadlock so readily. - Dave ------------------------------------------------------- This SF.Net email is sponsored by OSTG. Have you noticed the changes on Linux.com, ITManagersJournal and NewsForge in the past few weeks? Now, one more big change to announce. We are now OSTG- Open Source Technology Group. Come see the changes on the new OSTG site. www.ostg.com _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel