On Wed, 17 Sep 2003, David Brownell wrote:
> I see them as different in the following way: switching configurations
> retains the same "struct usb_device". Morphing the device, after a
> DFU style firmware update (host-initiated reset) or an EZ-USB style
> one ("Renumeration(tm)", device initiated reset), should in both cases
> involve a new usb_device and complete re-enumeration.
I don't necessarily agree. The EZ-USB style reset yes; the device
simulates a disconnect so the normal disconnect processing will handle it.
That probably works even now.
But for the other "device-morphed" case we don't need to do all that
stuff. There's no need to get rid of the current struct usb_device and
allocate a new one; just re-use the existing one. Re-enumeration can pick
up with the device already in the ADDRESS state.
> It's much simpler if you think of the considerations as different ... :)
>
> If the reset succeeds without the device changing, then the current
> code is pretty much correct -- except that the code doesn't use the
> device semaphore. (I seem to recall leaving all such issues for your
> rework of the reset code.)
>
> If the reset fails somehow, or succeeds with descriptors changing,
> that should be handled by forcing khubd to perform disconnect processing
> for the device. That means khubd disconnecting this driver (assuming it
> successfully probed) and putting the port into a state where it'll
> start enumerating again. Isn't there sume hub port feature we can
> set/clear that'll automatically do that?
There isn't, unfortunately. There is a hub port feature that would do it,
but it can't be set, only cleared. We will have to invent some way of
queueing such a request.
> > The real issue is ownership of usb_dev->serialize. set_config() -- and
> > reset_device() when something goes wrong -- must possess the mutex. But
> > if they're called during probe() then the thread they belong to already
> > owns it; they don't need to acquire it again. So there has to be some way
> > for the caller to say "Don't try to grab the serialize mutex because you
> > already own it somewhere earlier in the call stack". Yes it's awkward,
> > but I don't see how else to do it.
>
> Well, usb_set_configuration() shouldn't be called from driver probe();
> leaving usb_reset_device(). And given what I described above, I don't
> see why that would have a problem. But some of those paths don't need
> to keep the device lock, since they're protected by the bus rwsem; so
> they might drop it before outcalls (and grab it on return), if needed.
Not having thought this through very carefully, I don't want to say too
much. But I don't think we can rely on the bus rw-semaphore, because that
might or might not be held during usb_reset_device().
Consider the "device morphed" case. When that happens we have to wind
everything back to the ADDRESS state, which means unbinding all the
drivers and destroying all the interfaces. The question is, should the
unbinding be done right away (within the usb_reset_device() routine) or
should it be delayed until khubd or keventd or whoever gets around to
seeing the queued request? Oliver has said, and I agree, that the drivers
should be unbound immediately.
But unbinding requires owning dev->serialize. Up to now that hasn't been
an issue; binding and unbinding were pretty much done only in khubd, which
automatically made them serial (usbfs being the main exception). With
multiple pathways for unbinding, it's more important to make sure that
only one thread tries to manage a device at a time.
So the problem still exists.
> > One way around this problem is simply to outlaw calling either routine
> > from within a probe. But I think that may be going too far. For example,
> > usb-storage uses usb_reset_device() as a last-ditch error recovery method,
> > and it doesn't distinguish whether the error occurred during probe or
> > during normal operation.
>
> See above. I think usb_reset_device() should be nearly OK, except for
> locking and the "now kick khubd" path (which has never worked). There's
> no interaction with the usb_set_configuration() code except indirectly,
> in that new "kick khubd" path.
Okay. Since a driver that _really_ needs to change a configuration during
probe() can always schedule a work_queue item to do the job, I don't mind
outlawing direct calls to usb_set_configuration() within probe. I'm just
concerned about this dev->serialize question.
Alan Stern
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
