xhci: reduce xhci_handshake timeout in xhci_reset

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(-)

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

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