> Are we talking about the same thing here? In my original message I was > describing the usb_device_altered() function, which we expect a driver to > call after it has called usb_device_reset(). The reset() call rebinds all > the drivers to the interfaces they had before the reset, explicitly > assuming that the descriptors have _not_ changed. (That will be the > common case for things like error recovery.) Altered() reads the new > descriptors, unbinds the drivers, and binds the new interfaces.
OK, yes I see. You're right. Nevertheless I'd like a version that does a full reset with enumeration for error recovery. > Okay, so what about errors like a failure to accept firmware? As you say, > the device is screwed. I don't know what you mean by a "full reset" -- > usb_device_reset() really does reset the device and usb_device_altered() > will re-read the descriptors and do reprobing. If reading the descriptors > fails, altered() will disable the device, just as in the case of a > newly-connected device. What else remains to be done? Reset again. If a device has a failure somewhere during firmware load, it's probably really screwed. [..] > > > another way, if a firmware download is needed for the device to work > > > properly, why wouldn't that download be done first thing, during > > > probe()? > > > > Right, but there it's not the core's lock, but the driver model's lock. > > And neither is the problem specific to USB. Wouldn't it be more elegant > > to just return -ERESTARTSYS and let the driver model reprobe? > > No. We need a usbcore-specific lock. It's not enough to rely on the > driver model's locking. Doing that would leave open the possibility, for That's not a question of reliance. The generic lock is there. We need to deal with it. Furthermore we can always live with coarser locking. > example, of a driver newly bound to one interface doing a > set_configuration() at the same time that a driver for another interface > is being probed. To avoid this, set_config and probing must use the > _same_ lock. Yes, but it need not be per device. Per bus would do as well. Per interface is wrong. > The real problem is that the driver model is only aware of separate > individual interfaces, whereas usbcore realizes that several interfaces > are part of the same device. We need to lock the whole device, not just > a single interface. > > > Sorry, I was unclear. Shouldn't robust error handling look like: > > disable port -> reset hub -- retry disable -> powerdown port -> > > reset hub -- retry power down port -> power down hub -> recursion > > Would that really work? That is, if you reset the hub, won't that disable > _all_ its ports? We don't want to do that if powering down a single port > is possible. Powering down means that the port will no longer work. From disable we can recover. > > > I agree partially. Probing while the device is not in sync wouldn't be > > > a disaster, but it would be nice to avoid it. Perhaps the > > > usb_device_altered() routine could set some flag which would tell the > > > core to abort further probing until after the new descriptors have been > > > read. This would not require exporting the locking to the drivers. > > > > Too late. Submitting URBs to nonexistant interfaces is a very bad idea. > > Let's be more precise. URBs aren't submitted to interfaces, they're > submitted to endpoints. Submitting an URB to a nonexistent endpoint isn't > very bad, it merely causes an error code to be sent to the URB's Nonexistent is survivable. Endpoints going away while the URB is processed are another matter. > completion routine. Besides, who's going to be sending these URBs? Not > the driver that just altered the device's state. And if further probing > is aborted, no later drivers will get the chance. Maybe a driver that was > bound earlier. Right. > Anyway, how will allowing the drivers to lock out further probing prevent > these bad URBs from being sent? Lock against probe -> Kick off other drivers -> reset/config change > One way around this might be to have a combined reset_altered() > function. That would make sense; it's hard to think of a case where you > would want to call device_altered() without calling device_reset() > immediately beforehand. Doing things this way would avoid having any > period in which the core is out-of-sync with the device. Firmware download. > > > 2. I don't see how we can have a version without locking. Setting a > > > new configuration necessarily involves unbinding the existing > > > interfaces and binding the new ones, and that requires holding the > > > device lock. > > > > Which is held during probe, where these cases arise. > > I don't like that idea. It might lead to unbounded stack usage. You're > saying that a driver's probe() function could tell the core to set a > configuration and restart probing from the beginning, without waiting for > the original probe() to return. It goes against my grain. No, by returning an error. There's no recursion. The loop is justed started anew. > > > helper to do the job. But the existing code would probably still need > > > to be rewritten. The correct way for a driver to handle this in its > > > probe() routine is: See if the proper configuration is already loaded. > > > If yes then proceed normally; if no then call usb_set_configuration() > > > and return a failure code (i.e., don't bind). > > > > Then you can fail regardless. Could you explain? I don't see the logic? > > I don't understand what you mean by "you can fail regardless"... but I'll > explain the earlier statement. First, it's clear that if the correct > configuration is already loaded then the driver doesn't need to ask for it > to be loaded again. The driver can simply go on about its job. > > Second, if the wrong configuration is loaded, the driver _does_ need to > call set_configuration(). Doing so will ultimately cause a new set of > interfaces to be bound, and it will also cause the driver to be unbound > from the interface it is currently probing should the probe() return > success. So returning success is pointless; the driver might as well just > return a failure code. Even more strongly: if the configuration is wrong > then the driver has no business binding to this interface in the first > place, so it should return a failure code regardless, even if > set_configuration() fails. Probe() should have three possible returns. 1. can deal with the device 2. cannot deal with the device 3. can deal in a new form with firmware now in place > But assuming everything goes well and set_configuration() succeeds, the > core will at some point probe the interfaces in the new configuration. > One of them presumably will match the driver, so probe() will be called > again, this time with the correct configuration in place. Then the driver > will be happy and things will work as expected. Right. Regards Oliver ------------------------------------------------------- This SF.net email is sponsored by: eBay Get office equipment for less on eBay! http://adfarm.mediaplex.com/ad/ck/711-11697-6916-5 _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel