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

Reply via email to