musb: fix remote wakeup racing with suspend

[Bin Liu <b-liu@ti.com>]

On Wed, May 02, 2018 at 03:12:16AM +0200, Daniel Glöckner wrote:
> Hi,
> 
> On Tue, May 01, 2018 at 08:48:11AM -0500, Bin Liu wrote:
> > On Fri, Apr 27, 2018 at 03:00:05PM +0200, Daniel Glöckner wrote:
> > > It has been observed that writing 0xF2 to the power register while it
> > > reads as 0xF4 results in the register having the value 0xF0, i.e. clearing
> > > RESUME and setting SUSPENDM in one go does not work. It might also violate
> > > the USB spec to transition directly from resume to suspend, especially
> > > when not taking T_DRSMDN into account. But this is what happens when a
> > > remote wakeup occurs between SetPortFeature USB_PORT_FEAT_SUSPEND on the
> > > root hub and musb_bus_suspend being called.
> > > 
> > > This commit returns -EBUSY when musb_bus_suspend is called while remote
> > > wakeup is signalled and thus avoids to reset the RESUME bit. Remember that
> > > resume can be signalled only when the bus was already suspended, so it is
> > > safe to skip the following steps even when musb_hub_control ignores the
> > 
> > what do you mean by "skip the following steps"?
> 
> Setting USB_PORT_STAT_SUSPEND in musb->port1_status, changing
> musb->xceiv->otg->state, setting musb->is_active, etc.

Okay, but I am not sure how "even musb_hub_control ignores the error..."
is relevant in this statement. The steps are safely skipped in
musb_port_suspend() when MUSB_POWER_RESUME bit is set, no matter how
musb_hub_control() to deal with the return code from
musb_port_suspend(). Did I misunderstand anything?

> 
> > > error returned by musb_port_suspend.
> > > 
> > > The resume part of musb_port_suspend is modified to also accept a pending
> > > remote wakeup, to bring it to the end after T_DRSMDN has passed.
> > 
> > Can you please explain more here? I am not sure when musb_port_suspend()
> > is involved in resume by remote wakeup, and what case is a 'pending'
> > remote wakeup?
> 
> With 'pending' I was referring to the situation when MUSB_POWER_RESUME
> has been set by the controller in the power register as a result of
> of a detected remote wakeup.

Okay.

> 
> I see... finish_resume_work is already scheduled by musb_stage0_irq.

Yes.

> So the condition of the if statement probably does not need to be

I think so too. BTY, I don't think this pending remote wakeup situation
will trigger ClearPortFeature, which is the only case that invokes that
if statement you changed.

> changed. I'll test without that part and make a v2 patch if it works.
> 
> Btw., do you know why that 10000 iterations while loop is needed in
> musb_port_suspend to pass the OTG Protocol Test as indicated by the
> comment?

Unfortunately I don't. There are many mysteries in the musb drivers :)

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