On Sat, 9 Aug 2003, David Brownell wrote: > Alan Stern wrote: > > I've got a plan for reorganizing usb_reset_device(), > > usb_set_configuration(), and parts of the hub driver and core. .. > > > > Before doing any real programming, I wanted to get some feedback to see if > > this plan makes any sense. > > Well, please let me submit some chunks of that set_configuration() patch > I posted a while back. 2.5.67 (?) wasn't ready for it, but with all > the endpoint_disable() related cleanups, I think likely 2.6.0-test3 is > ready for chunks of it.
Certainly. I wasn't planning on doing this right away. > > The idea is that when usb_reset_device() sees a change in the descriptors > > after the reset, or when usb_set_configuration() succeeds, they set the > > state_change_pending flag and post a request to the khubd queue. This > > I like this: a clean way to tell usbcore to look at the device again. > Except ... does it have to be khubd in all cases? > > We need per-bus synchronization when something is resetting a device, > and pushing it up to the DEFAULT state. That's a reasonable thing to > reserve to a "khubd" role, even if it over-synchronizes (it doesn't > allow different devices to enumerate concurrently). > > But once a device is in the ADDRESS state, there's no reason to be > even vaguely related to khubd. Config change events, including > both SET_CONFIGURATION (possibly to zero) and SET_INTERFACE (affects > fewer endpoints), can/should be initiated by arbitrary tasks. They > don't need synchronization between devices, just a per-device mutex. > > So ... why not have a work_struct that gets scheduled, and have these > devices on a list that's processed when keventd has a moment? Eventually > I'd rather see khubd like that too (no dedicated task), but these config > changes are things that seem to need deferring more. I agree with most of your points. However, this is going to be a sizeable change just as it is. Redoing the way khubd works is another big change. There's no reason we can't do one first and the other later. I don't even care what the order is -- but I can see my way clear to programming this reorganization and I'm not so sure about all the details of changing khubd. > > request will include the device and the old configuration. When the hub > > thread processes the request, it will unbind all the interfaces for that > > configuration and then restart the device initialization procedure at the > > spot corresponding to the device's current state. If the state is > > DISCONNECTED, the device structure is deleted in the usual way; if the > > state is DEFAULT, an address is assigned; if the state is ADDRESS, a > > configuration is selected; if the state is CONFIGURED, all the interfaces > > are probed. > > Seems to me there will be two kinds of requests to process here: > > - immediate ones, which shouldn't be queued. > --> Example: echo 3 > /sys/bus/usb/devices/xxxx/bConfigurationValue > --> Example: usb_set_interface() in some driver > - deferred ones, which must be deferred. > --> Example: usb_reset_device() "device morphed" cases > > And things like initial usb_set_configuration() are invisible; they > can be in either context ... but today the're hidden in khubd, along > with the hub port reset logic (should share with usb_reset_device). I'm not sure about your first example. Configuration changes would take place immediately under my scheme, but unbinding the old interface drivers and probing the new interfaces would be deferred. Is anything wrong with that? When the config change is initiated through usbfs the deferral is unnecessary, but when it's initiated by a driver the deferral is essential. It's not clear to me how to distinguish the two cases within usb_set_configuration(), or if we even need to. Your second example, usb_set_interface() would remain practically unchanged. It doesn't require unbinding anything, so it doesn't have to do anything fancy. The only alteration is that I would like to separate out the actual message-sending -- usb_set_interface_physical() -- so that it could be shared by usb_reset_device() in the device-didn't-morph path. Actually, that suggests an unanswered question. What should we do if the device didn't morph, and we can restore the prior configuration, but for some reason we can't restore an interface's prior altsetting? > Another example of a deferred update would be recovery after HCD failure. > This ought to involve reset and complete re-initialization. We don't > do anything like that today; eventually we probably should. A job for the future. > > Only one state_change request may be queued for a device, and if > > usb_device_reset() or usb_set_configuration() see that one is already > > pending they will refuse to do anything (just return -EBUSY). Also, to > > prevent races against hardware disconnects, the hub disconnect handler > > will check to see if state_change_pending is set before doing anything. > > If it is set, all the disconnect handler needs to do is change the state > > to USB_STATE_DISCONNECTED. > > Or just use this work queue approach as the normal disconnect path.. Sure. > > After usb_device_reset() and usb_set_configuration() carry out their > > physical actions, they merely need to disable all the endpoints on the > > device (except maybe ep0), set the state to the correct value, and post > > the state_change request. The hub thread will handle all the rest. > > Again, I'd rather move away from the model of a dedicated khubd thread. > Far better to start limiting its duties, and making sure that any > thread which needs to do these other things can do so. I'd like to see that too. The connect/probe and disconnect paths are complex and subject to driver failures. The lower-level hub control logic shouldn't have to contend with stuff like that. Not long ago I tracked down a mysterious problem, wherein the UHCI controller would alternately suspend and wakeup, over and over. It turned out that the hub thread had hung or died during a disconnect(), before the port state had been fully reset. The hub thread should manage _hubs_, without worrying about all the other USB drivers. Alan Stern ------------------------------------------------------- This SF.Net email sponsored by: Free pre-built ASP.NET sites including Data Reports, E-commerce, Portals, and Forums are available now. Download today and enter to win an XBOX or Visual Studio .NET. http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01 _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
