Hi Mathias, > Sent: Tuesday, May 26, 2015 8:45 PM > > On 14.05.2015 12:39, Yoshihiro Shimoda wrote: > > The STS_HALT is not set until the CRR (CMD_RING_RUNNING) is cleared > > on specific xHCI controllers (e.g. R-Car SoCs) after this driver set > > the R/S (CMD_RUN) to 0. So, to avoid using a quirks XHCI_SLOW_SUSPEND, > > this patch adds to wait for the CRR to clear. > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx> > > --- > > My environment (R-Car H2) could work correctly. I would like to know > > that this patch causes side effect or not. Also I'd like to know > > this patch can avoid the XHCI_SLOW_SUSPEND quirk on other environment > > (Fresco Logic XHCI controller). > > > > Changes from v1: > > - Remove a abort code. > > - Wait for the CRR to clear after this driver set the R/S to 0. > > - Rebase on the usb.git / usb-next branch. > > (commit id = aa519be34f45954f33a6c20430deac8e544a180f) > > > > drivers/usb/host/xhci.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > > index ec8ac16..a357917 100644 > > --- a/drivers/usb/host/xhci.c > > +++ b/drivers/usb/host/xhci.c > > @@ -923,6 +923,15 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup) > > command &= ~CMD_RUN; > > writel(command, &xhci->op_regs->command); > > > > + /* > > + * The STS_HALT is not set until the CRR is cleared on specific > > + * xHCI controllers (e.g. R-Car SoCs) after this driver set the R/S > > + * to 0. So, to avoid using a quirks XHCI_SLOW_SUSPEND, this driver > > + * waits for the CRR to clear. > > + */ > > + xhci_handshake(&xhci->op_regs->cmd_ring, CMD_RING_RUNNING, 0, > > + 5 * 1000 * 1000); > > + > > /* Some chips from Fresco Logic need an extraordinary delay */ > > delay *= (xhci->quirks & XHCI_SLOW_SUSPEND) ? 10 : 1; > > > > > > Hi, sorry about the delay. Thank you for your reply! And, I'm sorry for the delayed response. > Does polling for the command ring running bit really help? > We are anyway polling for the HCHalted (STS_HALT) status bit right after this, > and it should indicate the command ring was halted as well. > > Is it possible that the R-Car xhci host controller halts just a tiny bit slower > than the default delay value, and the additional CRR check gives it > just enough time to HALT before timeout? Yes or No. If I used the XHCI_SLOW_SUSPEND quirk, the additional CRR check is not needed. In last October, I submitted a patch to use the quirk. However, I don't have any hardware documents why the R-Car xhci host controller needs this quirk: http://thread.gmane.org/gmane.linux.usb.general/115965/focus=116056 > Any chance you could compare the actual halt time using your patch against > the XHCI_SLOW_SUSPEND quirk? (time from clearing CMD_RUN until STS_HALT is set) Using my patch: xhci_handshake: CMD_RUN usec = 4930408 xhci_handshake: STS_HALT usec = 16000 So, total waiting time was about 70msec. (5000000 - 4930408 + 16000 - 16000 = 69592) Without my patch + using XHCI_SLOW_SUSPEND quirk: xhci_handshake: halt usec = 89796 So, total waiting time was about 70msec. (160000 - 89796 = 70204) Therefore, should I use the XHCI_SLOW_SUSPEND quirk instead of my patch? Best regards, Yoshihiro Shimoda > -Mathias -- 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
