As discussed earlier, we need a way for a driver to inform the core that a device's descriptors have changed (following a firmware download, for example). Let's call this new function usb_device_altered(). It will queue a request to be handled in another thread ...
I'm not really clear how usb_device_altered() would really need to be different from usb_device_reset() ... except
(a) reset() tries to preserve device address. Why bother?
(b) reset() tries to preserve driver bindings -- unless the
descriptors happened to change, when it clearly won't.
Why shouldn't altered() do that too, in the exceptional
case that the descriptors _didn't_ change? (c) reset() tries to be synchronous ... but implementing
it that way causes problems, like adding a bad lock
visibility to the API. Why shouldn't it be async,
with maybe a completion callback?In short, I don't see value in having two API calls.
Other than that synchony issue, I suspect that making the hub driver just reuse dev->children[port-1], instead of reallocation, would simplify a number of problems: both types of code can reuse the standard "used every day" enumeration logic. (And both should use the standard "device is gone" disconnect logic, "used every day", in some mode, as both you and Oliver have noted in other contexts.)
We really don't have much choice about this. Oliver pointed out earlier that involved maneuvers like firmware downloads may involve several sequential steps, and the device may not have any well-defined state in the intermediate portions. If the core tried to interfere in any way, this sort of reconfiguration would be impossible for a driver to carry out.
There aren't so many Linux drivers that load firmware after all. We can arrange to support their state transitions explicitly. For example:
- EZ-USB device "renumeration" has a final device-initiated reset ... the hub driver will always notice a USB_STATE_* transition from ADDRESS to POWERED even in the absense of a usb_device_altered() call. That's part of why it's so easy for fxload to do it from userspace.
- The "USB Device Firmware Update" (DFU) spec has a final host-initated reset ... enter DFU configuration, load firmware, issue DFU_DETACH, then host resets CONFIGURED back to POWERED and the hub driver takes over. Likewise, the usb_device_altered() call is not needed.
- I recall mention of a handful of purely proprietary models too ... but the ones I remember also transitioned to POWERED. Which one of those wants usb_device_altered()?
The basic model to use for firmware loading is that the original device disappears.
All right then, but what if through an error or bad writing the driver doesn't call usb_device_altered()? Depending on how large the change was, the device may now work only partially, or even not at all. But that's exactly what happens anyway when a driver does something wrong or contains a bug! In this situation the problem clearly lies in the driver, so the driver would need to be fixed, not the core. In the meantime, the kernel will struggle along with incorrect host-side configuration records. That's not so terrible, and if need be the user can always unplug the device and then re-attach it.
Aarg -- incorrect data could oops. Scrub out state in all cases. If that means the driver bugs must be fixed faster -- fine.
On a different tack... The operation of setting a new configuration will need to be handled by a separate thread (separate from the calling device driver, I mean), just like usb_device_altered(). Fitting a proper API for this may require some effort; I notice that several drivers already call usb_set_configuration() synchronously. Some set the config to its already-existing value, others blindly set it to 1. This will have to be examined closely and in some cases changed. Trying to set a new value synchronously will almost certainly result in deadlock. Even trying to reset the current value, which normally should be innocuous, might cause deadlock if the call failed.
There are few drivers that set configurations now.
Near as I can tell, that dates from an era when usbcore didn't; and anyway, usb_set_configuration() has only recently begun to behave correctly outside of "device not yet used, and it's already in that configuration" stages of configuration (where the fact that usbcore didn't reset all the relevant device state couldn't matter).
Concerning parallelization of the operations involved in connect/reset/disconnect processing, David writes:
I suspect a per-hub "struct work_struct" would suffice,
though it's TBD exactly what work would be done there.
Note that there operations require a separate thread or work-queue equivalent, because they have to take place in process context. I can think of several other possible arrangements:
(A) One work-queue entry per request, created on demand. This would involve creating lots of little kernel processes and would make it
difficult to handle requests in the correct order.
There aren't so many cases where an order is required though. Disconnecting a hub with children is probably the only case worth looking at.
(B) One persistent kernel thread per bus. This could result in a
large number of largely dormant threads. On the plus side, this is a natural level for handling address-0 serialization.
(C) One single kernel thread to do everything -- i.e., khubd. That's what we've got now.
I don't know what's best; maybe something not listed here.
There's also what I had mentioned:
(0) one work_struct per hub, scheduled as needed by the hub
status change URB callback, usb_device_reset(), etc. It _could_ handle config change events too, which would
make it responsible for "new usb inteface" paths through
driver binding. (Not "new usb driver" paths.)Parallelization is not needed for performance reasons, only for reliability.
Right. Just like host-initiated resets, unless they're part of a firmware change cycle (as in DFU and maybe others).
- Dave
------------------------------------------------------- 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
