[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAGCq0LYwrkCe4X77x__CC6qUPaSvRXoO3grKODfP+eSGZfNm7w@mail.gmail.com>
Date: Tue, 18 Jan 2022 14:50:43 +0800
From: Puma Hsu <pumahsu@...gle.com>
To: Mathias Nyman <mathias.nyman@...ux.intel.com>
Cc: Greg KH <gregkh@...uxfoundation.org>, mathias.nyman@...el.com,
Sergey Shtylyov <s.shtylyov@....ru>,
Albert Wang <albertccwang@...gle.com>,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
stable@...r.kernel.org
Subject: Re: [PATCH v3] xhci: re-initialize the HC during resume if HCE was set
On Mon, Jan 17, 2022 at 8:20 PM Mathias Nyman
<mathias.nyman@...ux.intel.com> wrote:
>
>
> >>> This seems like a big hammer for when the host controller throws an
> >>>> error. Why is this the only place that it should be checked for? What
> >>>> caused the error that can now allow it to be fixed?
> >>>
> >>> I believe this is not the only place that the host controller may set
> >>> HCE, the host controller may set HCE anytime it sees an error in my
> >>> opinion, not only in suspend or resume.
> >>
> >> Then where else should it be checked? Where else will your silicon set
> >> this bit as part of the normal operating process?
> >
> > We observed this flag while resume in our silicon so far. According to the XHCI
> > specification 4.24.1, “Software should implement an algorithm for checking the
> > HCE flag if the xHC is not responding.”, so maybe it would be better
> > to implement
> > a new API to recover host controller whenever the driver side finds no response
> > from host controller in the future.
> >
>
> As all the code to reset the host during resume already exists, and is well tried
> due to issues in resume being so common, I think it makes sense to add the HCE case
> here as well. It's a simple fix that makes the life of users better.
>
> That said we shouldn't hide the reason for reset like this.
> Print a debug message telling about the HCE so that everybody working with xHCI
> can see it and start fixing the rootcause.
I will print a debug message in next patch version.
> Another HCE check could be added to command timeout code, but just to show a
> warning for now.
> Reset might not always clear HCE, and we don't want to be stuck in a reset loop.
> This check could be a separate patch
Maybe I can try to make this in the future. Thank you for advising.
> -Mathias
Powered by blists - more mailing lists