On Fri, 20 Sep 2013, Mehresh Ramneek-B31383 wrote: > From: Alan Stern [mailto:stern@xxxxxxxxxxxxxxxxxxx] > Sent: Thursday, September 19, 2013 7:03 PM > To: Mehresh Ramneek-B31383 > Cc: linux-usb@xxxxxxxxxxxxxxx > Subject: Re: [PATCH]fsl/usb: Workarourd for USB erratum-A005697 > > On Thu, 19 Sep 2013, Ramneek Mehresh wrote: > > > As per USB specification, in Suspend state the status bit does not > > change until the port is suspended. However, there may be a delay in > > suspending a port if there is a transaction currently in progress on > > the bus. > > > > In the USBDR controller, the PORTSCx[SUSP] bit changes immediately > > when the application sets it and not when the port is actually > > suspended > > > > Workaround for this issue involves waiting for a minimum of 10ms to > > allow the controller to go into SUSPEND state before proceeding ahead > > Why is this workaround needed? Does anything go wrong if it isn't applied? > [Ramneek]: This workaround is needed because we have this issue with the controller. > In some protocols like HNP, a notification needs to be sent to another OTG device > as soon as current controller port goes into SUSPEND state. However, this > notification could be false if the port is still transmitting some data and the > controller informs system s/w that the port is already in suspend state. Which subroutine in which source file sends the HNP notification? > > --- a/drivers/usb/host/ehci-hub.c > > +++ b/drivers/usb/host/ehci-hub.c > > @@ -284,6 +284,8 @@ static int ehci_bus_suspend (struct usb_hcd *hcd) > > else if ((t1 & PORT_PE) && !(t1 & PORT_SUSPEND)) { > > t2 |= PORT_SUSPEND; > > set_bit(port, &ehci->bus_suspended); > > + if (ehci_has_fsl_susp_errata(ehci)) > > + usleep_range(10000, 20000); > > } This is the wrong place to add a delay. For one thing, this isn't where the port gets put into suspend -- that occurs about 17 lines later. For another, ehci_bus_suspend() gets called when the root hub is suspended. But you want to implement this delay when the port gets suspended, which is in ehci_hub_control(). Also, you shouldn't use usleep_range(). Call ehci_handshake() instead. And be certain to drop ehci->lock while you are waiting for the port to go into suspend. 10 ms is too long to hold a spinlock. 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
