On Tue, Mar 01, 2022 at 10:47:36AM +0200, Mathias Nyman wrote: > On 1.3.2022 6.03, Pavan Kondeti wrote: > > On Mon, Feb 28, 2022 at 05:49:49PM -0800, Jack Pham wrote: > >> Hi Mathias, > >> > >> On Thu, Feb 17, 2022 at 03:56:43PM +0200, Mathias Nyman wrote: > >>> xhci_reset() timeout was increased from 250ms to 10 seconds in order to > >>> give Renesas 720201 xHC enough time to get ready in probe. > >> > >> This suggests that the only place we really need the long timeout is > >> in xhci_gen_setup(). > >> > >>> @@ -1163,7 +1161,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > >>> xhci_dbg(xhci, "Stop HCD\n"); > >>> xhci_halt(xhci); > >>> xhci_zero_64b_regs(xhci); > >>> - retval = xhci_reset(xhci); > >>> + retval = xhci_reset(xhci, XHCI_RESET_LONG_USEC); > >> > >> Since preemption is disabled here (spin_lock_irq() is called near the > >> start of this function), shouldn't we also limit this to 250ms here in > >> xhci_resume() as well? > >>> The rationale of decreasing the timeout to 250 in certain places is based > > on the criticality of the operation but not on the preemption/irq state. > > Since xHC reset is critical in startup and resume, the 10 seconds timeout > > is enforced so that we don't break Renesas 720201 xHC. > > > > Since all of our internl test reports indicate that the timeout is happening > > from stop hcd, this patch is helping. > > > > This was pretty much my reasoning as well. > I could add a comment about this to the commit message > > In addition we want a targeted fix for a real world issue that we can send to > stable without changing too much, risking regressions. > Makes complete sense. Greg, Do you plan to include this patch? Thanks, Pavan
