On Fri, 11 Mar 2016, Jiri Kosina wrote: > On Wed, 2 Mar 2016, Alan Stern wrote: > > [ ... snip ... ] > > The patch that fixed Daniel's problem is repeated below for your > > convenience. Is this the right approach? If you think it is, I'll > > submit it officially. > > I've been thinking about this, and while I don't really like such bandaids > being added to the generic code, I haven't been able to come up with > anything better. > > So the aproach is fine with me ... perhaps a short comment before testing > HID_STARTED wouldn't hurt as well, as it has a potential of making code > reader's head spin. 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? It stands out that hid_resume() checks the HID_STARTED bit before calling hid_start_in() but hid_post_reset() doesn't. That is 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 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. What do you suggest? Alan Stern -- 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
