On Mon, Feb 14, 2022 at 01:29:12PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Feb 14, 2022 at 05:50:16PM +0530, Pavankumar Kondeti wrote:
> > From: Daehwan Jung <dh10.jung@xxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxx>
> > Signed-off-by: Pavankumar Kondeti <quic_pkondeti@xxxxxxxxxxx>
> > ---
> >
> > v2:
> > - Add error print statements in the code that change log refers to
> >
> > drivers/usb/host/xhci.c | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > index dc357ca..bb9ea3f 100644
> > --- a/drivers/usb/host/xhci.c
> > +++ b/drivers/usb/host/xhci.c
> > @@ -196,9 +196,11 @@ int xhci_reset(struct xhci_hcd *xhci)
> > udelay(1000);
> >
> > ret = xhci_handshake(&xhci->op_regs->command,
> > - CMD_RESET, 0, 10 * 1000 * 1000);
> > - if (ret)
> > + CMD_RESET, 0, 1 * 1000 * 1000);
> > + if (ret) {
> > + xhci_err(xhci, "Host controller reset timed out\n");
>
> A timeout is not the only error that could have happened here. So why
> claim that all errors are timeout errors?
Thanks for pointing this out. xhci_handshake() can return an error code
other than -ETIMEDOUT. ret == -ETIMEDOUT check needs to be added or
just print the ret error in the print message.
>
> How did you test this?
This is a hard to reproduce issue. So I have hacked the code to use 1 usec
instead of 1 sec as timeout and see running into the timeout issue. I know
this is not a perfect test. The test patch is included below.
Thanks,
Pavan
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index e95a5bc..6147544 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -37,6 +37,9 @@ static unsigned long long quirks;
module_param(quirks, ullong, S_IRUGO);
MODULE_PARM_DESC(quirks, "Bit flags for quirks to be enabled as default");
+static int reset_error;
+module_param(reset_error, int, S_IRUGO | S_IWUSR);
+
static bool td_on_ring(struct xhci_td *td, struct xhci_ring *ring)
{
struct xhci_segment *seg = ring->first_seg;
@@ -195,8 +198,15 @@ int xhci_reset(struct xhci_hcd *xhci)
if (xhci->quirks & XHCI_INTEL_HOST)
udelay(1000);
- ret = xhci_handshake(&xhci->op_regs->command,
- CMD_RESET, 0, 1 * 1000 * 1000);
+ if (reset_error) {
+ xhci_err(xhci, "forcing timeout\n");
+ ret = xhci_handshake(&xhci->op_regs->command,
+ CMD_RESET, 0, 1); /* 1 usec */
+ } else {
+ ret = xhci_handshake(&xhci->op_regs->command,
+ CMD_RESET, 0, 1 * 1000 * 1000);
+ }
+
if (ret) {
xhci_err(xhci, "Host controller reset timed out\n");
return ret;
