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.
Yep -- it does! Part of why it's a win to reuse that code path, with device morph causing both disconnect and reconnect processing. We know it works, and it's less error-prone to stick to one simple model.
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.
That could work too. I thought the other way looked like less code; maybe not, it depends how the parts get assembled. I also liked the idea of just _knowing_ there's can't be old state lying around.
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.
That's a part that needs investigation. If we can't notice the end of a reset sequence using current khubd code, surely it can be taught to have non-hub devices in its queue!
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().
True. Which is why I want reset_device() to be orthogonal to config change processing, so locks won't collide as readily.
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.
I must have missed that discussion. Why? We already know drivers "shall" handle device unplug at any such awkward instants; and it's not as if any driver bound at that instant is ever going to succeed in another i/o call to that device until it "re-enumerates". If they can clean up after that, they can clean up post-morphing too.
Why have two almost-identical faults expect different cleanup? Plus, remember how complex the disconnect processing is already. An "even more immediate" cleanup mode wouldn't simplify...
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.
We already do disconnect/unbind in multiple contexts. "rmmod uhci-hcd" will do that as nicely as khubd; and sometimes PM cleanup will too. Only one of them gets the lock at a time.
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.
Such answers can be found without excessive pain ... :)
- Dave
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