On Mon, 14 Mar 2016, Oliver Neukum wrote:

> 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.

Okay.

> > 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,

Not at all.  For one thing, a reset may be started by a driver for
another interface on the same device.  For another, a reset-resume can
occur at any time, if the device is attached through a host controller
that loses power during suspend (which is what happened to Daniel).

> but strictly speaking both methods must check the flag.

So that does need to be added.

> 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 

Daniel's problem involved hid_post_reset(), not hid_resume_common().  
Is that what you meant?  Maybe I'm wrong, but it looks like 
hid_post_reset() just needs to check whether I/O should be started 
before trying to restart it.

(It also looks like hid_resume() needs to clear the HID_SUSPENDED flag 
even when HID_STARTED isn't set.)

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to