Hi Sarah, Answering your question, yes, per our previous testing with our re-driver part, all ports are subject to suffer of the failing condition (Compliance Mode) after system resumes from Sleep/Hibernate no matter if ports entered to U0 before system suspension. That is why the original intention of the compliance mode patch is to delete the timer before system suspend and re-initialize it upon system resume. Unfortunately I didn't know that the timer is not deleted before system enters hibernation. Please let me know if there is something I could do for helping with this. Best Regards, Alexis Cortes. -----Original Message----- From: Sarah Sharp [mailto:sarah.a.sharp@xxxxxxxxxxxxxxx] Sent: Wednesday, February 20, 2013 5:10 PM To: Tony Camuso; Alan Stern Cc: rjw@xxxxxxx; linux-usb@xxxxxxxxxxxxxxx; Cortes, Alexis; dzickus@xxxxxxxxxx Subject: Re: [PATCH] xhci - correct comp_mode_recovery_timer on return from hibernate On Wed, Feb 20, 2013 at 12:35:11PM -0500, Alan Stern wrote: > On Wed, 20 Feb 2013, Don Zickus wrote: > > Hi Alan, > > > > I worked with Tony on this issue. The instrumentation we did was to > > add printks in the xhci_suspend, xhci_resume and > > compliance_mode_recovery_timer_init. > > > > What we noticed was that the xhci_suspend was not called during hibernate. > > I bet it really was called, but you didn't realize it because of the > way you monitored the output from the printks. You looked at the log > buffer or the kernel log file after resuming, right? You didn't have > a serial console attached to another machine, to capture the output as > it occurred. > > This makes a difference, because output that occurs after the memory > image is stored on disk will not be present in the image and hence > will not be visible after you resume from hibernation. > > Of course, in your case this doesn't matter. In the memory image, the > timer is active. Hence it is still active when the system resumes, > even though xhci_suspend _was_ called. Ah, I see now. So basically any memory changes in xhci_suspend will be wiped on resume, but any hardware state should be saved. I was initially concerned about the large number of memory writes in xhci_suspend (e.g. zeroing the command ring), but on resume from hibernate, xhci_resume basically ends up wiping the entire slate and re-initializing the host. It would be nice if xhci_suspend was passed a boolean to indicate it was being called as part of the hibernate process. That way we could skip a lot of unnecessary state saving. One thing that isn't clear to me is how hibernate and host reset effect the Synopsys part. Say that all ports have seen U0 before hibernate (and the timer wasn't running). If we resume from hibernate and reset the host, will it be possible to run into the re-driver issue with dead ports? Alex, do you know? I think the safe bet would be to assume that we can hit the bug after hibernate. That would mean we would have to delete a running timer on resume from hibernate, and let xhci_init re-initialize the U0 port state bitmask. Which is exactly what your first patch does: > /* If restore operation fails, re-initialize the HC during resume */ > if ((temp & STS_SRE) || hibernated) { > + > + if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && > + (!(xhci_all_ports_seen_u0(xhci)))) { > + del_timer_sync(&xhci->comp_mode_recovery_timer); > + xhci_dbg(xhci, "Compliance Mode Recovery Timer Deleted!\n"); > + } > + > /* Let the USB core know _both_ roothubs lost power. */ > usb_root_hub_lost_power(xhci->main_hcd->self.root_hub); > usb_root_hub_lost_power(xhci->shared_hcd->self.root_hub); > @@ -1058,7 +1065,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > * to suffer the Compliance Mode issue again. It doesn't matter if > * ports have entered previously to U0 before system's suspension. > */ > - if (xhci->quirks & XHCI_COMP_MODE_QUIRK) > + if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && (!hibernated)) > compliance_mode_recovery_timer_init(xhci); > > return retval; I think you should just clean up the patch description with Alan's explanation and resubmit it. Sarah Sharp -- 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
