Re: [PATCH 2/4] usb: musb: Fix sleeping function called from invalid context for hdrc glue

[Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

On Tue, Nov 08, 2016 at 10:34:13AM -0700, Tony Lindgren wrote:
> * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161108 10:09]:
> > On Mon, Nov 07, 2016 at 02:50:18PM -0700, Tony Lindgren wrote:
> > > @@ -2604,6 +2669,9 @@ static int musb_resume(struct device *dev)
> > >  	mask = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_FSDEV | MUSB_DEVCTL_LSDEV;
> > >  	if ((devctl & mask) != (musb->context.devctl & mask))
> > >  		musb->port1_status = 0;
> > > +
> > > +	schedule_delayed_work(&musb->pending_resume_work, 0);
> > > +
> > 
> > The interactions with system suspend looks a bit suspicious. It seems
> > you need to drain any pending resume work on suspend for example.
> > 
> > And then the above should not be needed, right?
> 
> Hmm that's an interesting idea. I think that would allow us to get
> rid of the delayed work if we check the list both on resume and
> suspend :)

Note that I was referring to draining the runtime-resume work on system
suspend above. I think we still want the delayed work considering that
we could stay active for long periods of time.

> > In fact, the dsps timer must also be cancelled on suspend, or you could
> > end up calling dsps_check_status() while suspended (it is currently not
> > cancelled until the parent device is suspended, which could be too
> > late).
> 
> And then this should no longer be an issue either.

It would still be an issue as a system-suspending device could already
have been runtime-resumed so that dsps_check_status() would be called
directly from the timer function.

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