On Fri, 2016-03-11 at 11:54 -0500, Alan Stern wrote: > Perhaps I didn't do a good job of addressing the real underlying > problem. That's why I asked if this was the right approach. > > In Daniel's case, usbhid_start() never got called. Perhaps that's the > real problem? Anyway, this meant that usbhid->urbin never got set, and > consequently hid_start_in() returned an error when it was called by > hid_post_reset(). > > Does it make sense for usbhid_start() not to be called? If it depends on other modules, we can hardly guarantee it being called. > It stands out that hid_resume() checks the HID_STARTED bit before > calling hid_start_in() but hid_post_reset() doesn't. That is I think the assumption is that resets are caused by the operation of the HID interface. In practice this is almost invariably true, but strictly speaking both methods must check the flag. > inconsistent; the check should be done in both places or in neither, > but I don't know which. Is it possible for one of usbhid's devices to > be suspended or reset before usbhid_start() or after usbhid_stop()? If > it is then both places need to check -- my patch adds the missing > check. > > Finally, isn't it possible for an HID device not to have an > interrupt-IN endpoint? In which case usbhid->urbin will always be That would violate chapter 4.4 of the HID spec. > NULL, so hid_start_in() will always return an error unless it > specifically checks -- and my patch adds such a check. > > As you can see, it's a complicated and confusing situation. Maybe > you'd like to explore it in greater depth with Daniel to understand it > better. It seems to me that hid_resume_common() needs to be split into three parts a) doing the stuff for pending resets b) conditionally restarting IO c) reset the flag Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
