| Message ID | 1644836663-29220-1-git-send-email-quic_pkondeti@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xhci: reduce xhci_handshake timeout in xhci_reset | expand |
On Mon, Feb 14, 2022 at 04:34:23PM +0530, Pavankumar Kondeti wrote: > From: Daehwan Jung <dh10.jung@samsung.com> > > xhci_reset() is called with interrupts disabled. Waiting 10 seconds for > controller reset and controller ready operations can be fatal to the > system when controller is timed out. Reduce the timeout to 1 second > and print a error message when the time out happens. > > Fixes: 22ceac191211 ("xhci: Increase reset timeout for Renesas 720201 host.") > Signed-off-by: Daehwan Jung <dh10.jung@samsung.com> > Signed-off-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com> > --- > drivers/usb/host/xhci.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index dc357ca..ec4df72 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -196,7 +196,7 @@ int xhci_reset(struct xhci_hcd *xhci) > udelay(1000); > > ret = xhci_handshake(&xhci->op_regs->command, > - CMD_RESET, 0, 10 * 1000 * 1000); > + CMD_RESET, 0, 1 * 1000 * 1000); > if (ret) > return ret; > > @@ -210,7 +210,7 @@ int xhci_reset(struct xhci_hcd *xhci) > * than status until the "Controller Not Ready" flag is cleared. > */ > ret = xhci_handshake(&xhci->op_regs->status, > - STS_CNR, 0, 10 * 1000 * 1000); > + STS_CNR, 0, 1 * 1000 * 1000); > > xhci->usb2_rhub.bus_state.port_c_suspend = 0; > xhci->usb2_rhub.bus_state.suspended_ports = 0; > -- > 2.7.4 > I do not see any "print an error message" change here. Where is that addition? confused, greg k-h
Hi Greg, On Mon, Feb 14, 2022 at 12:36:47PM +0100, Greg Kroah-Hartman wrote: > On Mon, Feb 14, 2022 at 04:34:23PM +0530, Pavankumar Kondeti wrote: > > From: Daehwan Jung <dh10.jung@samsung.com> > > > > xhci_reset() is called with interrupts disabled. Waiting 10 seconds for > > controller reset and controller ready operations can be fatal to the > > system when controller is timed out. Reduce the timeout to 1 second > > and print a error message when the time out happens. > > > > Fixes: 22ceac191211 ("xhci: Increase reset timeout for Renesas 720201 host.") > > Signed-off-by: Daehwan Jung <dh10.jung@samsung.com> > > Signed-off-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com> > > --- > > drivers/usb/host/xhci.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > > index dc357ca..ec4df72 100644 > > --- a/drivers/usb/host/xhci.c > > +++ b/drivers/usb/host/xhci.c > > @@ -196,7 +196,7 @@ int xhci_reset(struct xhci_hcd *xhci) > > udelay(1000); > > > > ret = xhci_handshake(&xhci->op_regs->command, > > - CMD_RESET, 0, 10 * 1000 * 1000); > > + CMD_RESET, 0, 1 * 1000 * 1000); > > if (ret) > > return ret; > > > > @@ -210,7 +210,7 @@ int xhci_reset(struct xhci_hcd *xhci) > > * than status until the "Controller Not Ready" flag is cleared. > > */ > > ret = xhci_handshake(&xhci->op_regs->status, > > - STS_CNR, 0, 10 * 1000 * 1000); > > + STS_CNR, 0, 1 * 1000 * 1000); > > > > xhci->usb2_rhub.bus_state.port_c_suspend = 0; > > xhci->usb2_rhub.bus_state.suspended_ports = 0; > > -- > > 2.7.4 > > > > I do not see any "print an error message" change here. Where is that > addition? > Thanks for taking a look. The "error messages prints" are in my working tree unstaged :-( sorry for the confusion. I will fix it and resend the patch. Thanks, Pavan
Hello! On 2/14/22 2:04 PM, Pavankumar Kondeti wrote: > From: Daehwan Jung <dh10.jung@samsung.com> > > xhci_reset() is called with interrupts disabled. Waiting 10 seconds for > controller reset and controller ready operations can be fatal to the > system when controller is timed out. Reduce the timeout to 1 second > and print a error message when the time out happens. Waiting 1 second with IRQs diabled is also hardly acceptable. :-/ > > Fixes: 22ceac191211 ("xhci: Increase reset timeout for Renesas 720201 host.") > Signed-off-by: Daehwan Jung <dh10.jung@samsung.com> > Signed-off-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com> [...] MBR, Sergey
On Mon, Feb 14, 2022 at 06:45:30PM +0300, Sergei Shtylyov wrote: > Hello! > > On 2/14/22 2:04 PM, Pavankumar Kondeti wrote: > > > From: Daehwan Jung <dh10.jung@samsung.com> > > > > xhci_reset() is called with interrupts disabled. Waiting 10 seconds for > > controller reset and controller ready operations can be fatal to the > > system when controller is timed out. Reduce the timeout to 1 second > > and print a error message when the time out happens. > > Waiting 1 second with IRQs diabled is also hardly acceptable. :-/ > Thanks for making this point clear. Having debugged several performance issue related to preemption and irqs disabled, I can't agree more. We had to chase down the code that disable scheduling for more than 2-3 msec in our downstream kernels on a mobile SoC. However in this case, we run into timeout very very rarely in which case 10 sec timeout is causing system hang like symptops which we want to avoid. Thanks, Pavan
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dc357ca..ec4df72 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -196,7 +196,7 @@ int xhci_reset(struct xhci_hcd *xhci) udelay(1000); ret = xhci_handshake(&xhci->op_regs->command, - CMD_RESET, 0, 10 * 1000 * 1000); + CMD_RESET, 0, 1 * 1000 * 1000); if (ret) return ret; @@ -210,7 +210,7 @@ int xhci_reset(struct xhci_hcd *xhci) * than status until the "Controller Not Ready" flag is cleared. */ ret = xhci_handshake(&xhci->op_regs->status, - STS_CNR, 0, 10 * 1000 * 1000); + STS_CNR, 0, 1 * 1000 * 1000); xhci->usb2_rhub.bus_state.port_c_suspend = 0; xhci->usb2_rhub.bus_state.suspended_ports = 0;