And here are comments on the other (larger/less-obvious/more-intrusive) patches in that batch. The directions are IMO good, but these changes are hardly bite-sized and I think it'd be easier to not-break things if these got rolled out in smaller (and more reviewable) chunks.
- Dave On Monday 12 June 2006 12:15 pm, Alan Stern wrote: > > 4. Make usb_generic into a genuine device driver. This involves > adding a mechanism to tell apart USB interface drivers from USB > device drivers -- of which usb_generic is currently the only one. Arguably this is how "hub.c" should work. (Plus as you mentioned local proxies for remote or virtualzed usb gadgets.) But this seems somewhat incomplete, and it bothers me that the driver for a usb device is using usb_interface entities in its usbcore chitchat. Maybe a "usb_device_driver" could be a better virtualization hook too; its responsibilities will certainly be very different from those of the current usb_(interface_)_driver structure. > 8. General cleanup of the suspend/resume code. Duplicated code, > unnecessary checks, that sort of thing. Always a nice thing to have. This patch was pretty large though, maybe it should become a few smaller patches? Just to highlight the parts that aren't actually duplicated, etc. And help sort out the intermediate issues that will surely crop up. :) Glad to see usage of dev.power.power_state start to vanish. > 9. Tie together suspend/resume operations on a device and on its > interfaces. Another way to put this is that you've reverted some of the changes from patch seven-of-nine. Any chance of not actually making those changes in the first place, and thereby simplifying both patches? Or maybe just re-ordering things so this goes first. > There doesn't seem to be much need for the peculiar > intermediate state where a USB device is awake but some of its > interfaces are suspended. The patch effectively rules them out; > attempts to suspend or resume an interface won't do anything, > and attempts to suspend or resume a device will affect all its > interfaces as well as the device as a whole. Right, that's what we discussed earlier but this is probably a better way to explain it: get rid of those nasssty intermediate states. Along the lines of what we said earlier, I think the "right" approach would be to have a single sysfs file like usb3/3-1/usb_state rather than have all of usb3/3-1/power/state, usb3/3-1/3-1:1.0/power/state, and so forth. That could be defined as "suspended" or "active" (or whatever), writable from userspace, etc. So when power/state files vanish, a good solution would be in place (for USB). Comments? :) _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
