* [PATCH] xhci: re-initialize the HC during resume if HCE was set @ 2021-12-28 6:02 Puma Hsu 2021-12-28 8:26 ` Greg KH 2021-12-28 14:34 ` Sergey Shtylyov 0 siblings, 2 replies; 10+ messages in thread From: Puma Hsu @ 2021-12-28 6:02 UTC (permalink / raw) To: mathias.nyman, gregkh; +Cc: albertccwang, linux-usb, linux-kernel, Puma Hsu When HCE(Host Controller Error) is set, it means an internal error condition has been detected. It needs to re-initialize the HC too. Signed-off-by: Puma Hsu <pumahsu@google.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 dc357cabb265..c546d9533410 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1146,8 +1146,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) temp = readl(&xhci->op_regs->status); } - /* If restore operation fails, re-initialize the HC during resume */ - if ((temp & STS_SRE) || hibernated) { + /* If restore operation fails or HC error is detected, re-initialize the HC during resume */ + if ((temp & STS_SRE) || (temp & STS_HCE) || hibernated) { if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && !(xhci_all_ports_seen_u0(xhci))) { -- 2.34.1.448.ga2b2bfdf31-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] xhci: re-initialize the HC during resume if HCE was set 2021-12-28 6:02 [PATCH] xhci: re-initialize the HC during resume if HCE was set Puma Hsu @ 2021-12-28 8:26 ` Greg KH 2021-12-29 5:53 ` Puma Hsu 2021-12-28 14:34 ` Sergey Shtylyov 1 sibling, 1 reply; 10+ messages in thread From: Greg KH @ 2021-12-28 8:26 UTC (permalink / raw) To: Puma Hsu; +Cc: mathias.nyman, albertccwang, linux-usb, linux-kernel On Tue, Dec 28, 2021 at 02:02:46PM +0800, Puma Hsu wrote: > When HCE(Host Controller Error) is set, it means an internal > error condition has been detected. It needs to re-initialize > the HC too. > > Signed-off-by: Puma Hsu <pumahsu@google.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 dc357cabb265..c546d9533410 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -1146,8 +1146,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > temp = readl(&xhci->op_regs->status); > } > > - /* If restore operation fails, re-initialize the HC during resume */ > - if ((temp & STS_SRE) || hibernated) { > + /* If restore operation fails or HC error is detected, re-initialize the HC during resume */ > + if ((temp & STS_SRE) || (temp & STS_HCE) || hibernated) { > > if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && > !(xhci_all_ports_seen_u0(xhci))) { > -- > 2.34.1.448.ga2b2bfdf31-goog > What commit does this fix? Does it need to be backported to older kernels as well? thanks, greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xhci: re-initialize the HC during resume if HCE was set 2021-12-28 8:26 ` Greg KH @ 2021-12-29 5:53 ` Puma Hsu 2021-12-29 8:30 ` Greg KH 0 siblings, 1 reply; 10+ messages in thread From: Puma Hsu @ 2021-12-29 5:53 UTC (permalink / raw) To: Greg KH; +Cc: mathias.nyman, Albert Wang, linux-usb, linux-kernel This commit is not used to fix a specific commit. We find a condition that when XHCI runs the resume process but the HCE flag is set, then the Run/Stop bit of USBCMD cannot be set so that HC would not be enabled. In fact, HC may already meet a problem at this moment. Besides, in xHCI requirements specification revision 1.2, Table 5-21 BIT(12) claims that Software should re-initialize the xHC when HCE is set. Therefore, I think this commit could be the error handling for HCE. Thanks in advance. Thanks in advance. • Puma Hsu 許誌宏 • Software Engineer, Pixel Phone • Tel: +886 2 8729 0870 • pumahsu@google.com On Tue, Dec 28, 2021 at 4:26 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Tue, Dec 28, 2021 at 02:02:46PM +0800, Puma Hsu wrote: > > When HCE(Host Controller Error) is set, it means an internal > > error condition has been detected. It needs to re-initialize > > the HC too. > > > > Signed-off-by: Puma Hsu <pumahsu@google.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 dc357cabb265..c546d9533410 100644 > > --- a/drivers/usb/host/xhci.c > > +++ b/drivers/usb/host/xhci.c > > @@ -1146,8 +1146,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > > temp = readl(&xhci->op_regs->status); > > } > > > > - /* If restore operation fails, re-initialize the HC during resume */ > > - if ((temp & STS_SRE) || hibernated) { > > + /* If restore operation fails or HC error is detected, re-initialize the HC during resume */ > > + if ((temp & STS_SRE) || (temp & STS_HCE) || hibernated) { > > > > if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && > > !(xhci_all_ports_seen_u0(xhci))) { > > -- > > 2.34.1.448.ga2b2bfdf31-goog > > > > What commit does this fix? Does it need to be backported to older > kernels as well? > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xhci: re-initialize the HC during resume if HCE was set 2021-12-29 5:53 ` Puma Hsu @ 2021-12-29 8:30 ` Greg KH 2021-12-29 9:11 ` Puma Hsu 0 siblings, 1 reply; 10+ messages in thread From: Greg KH @ 2021-12-29 8:30 UTC (permalink / raw) To: Puma Hsu; +Cc: mathias.nyman, Albert Wang, linux-usb, linux-kernel A: http://en.wikipedia.org/wiki/Top_post Q: Were do I find info about this thing called top-posting? A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail? A: No. Q: Should I include quotations after my reply? http://daringfireball.net/2007/07/on_top On Wed, Dec 29, 2021 at 01:53:04PM +0800, Puma Hsu wrote: > This commit is not used to fix a specific commit. We find a condition > that when XHCI runs the resume process but the HCE flag is set, then > the Run/Stop bit of USBCMD cannot be set so that HC would not be > enabled. In fact, HC may already meet a problem at this moment. > Besides, in xHCI requirements specification revision 1.2, Table 5-21 > BIT(12) claims that Software should re-initialize the xHC when HCE is > set. Therefore, I think this commit could be the error handling for > HCE. So this does not actually fix an issue that you have seen in any device or testing? So it is not relevant for older kernels but just "nice to have"? How did you test this if you can not duplicate the problem? thanks, greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xhci: re-initialize the HC during resume if HCE was set 2021-12-29 8:30 ` Greg KH @ 2021-12-29 9:11 ` Puma Hsu 2021-12-29 9:51 ` Greg KH 0 siblings, 1 reply; 10+ messages in thread From: Puma Hsu @ 2021-12-29 9:11 UTC (permalink / raw) To: Greg KH; +Cc: mathias.nyman, Albert Wang, linux-usb, linux-kernel On Wed, Dec 29, 2021 at 4:30 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > A: http://en.wikipedia.org/wiki/Top_post > Q: Were do I find info about this thing called top-posting? > A: Because it messes up the order in which people normally read text. > Q: Why is top-posting such a bad thing? > A: Top-posting. > Q: What is the most annoying thing in e-mail? > > A: No. > Q: Should I include quotations after my reply? > > http://daringfireball.net/2007/07/on_top > > On Wed, Dec 29, 2021 at 01:53:04PM +0800, Puma Hsu wrote: > > This commit is not used to fix a specific commit. We find a condition > > that when XHCI runs the resume process but the HCE flag is set, then > > the Run/Stop bit of USBCMD cannot be set so that HC would not be > > enabled. In fact, HC may already meet a problem at this moment. > > Besides, in xHCI requirements specification revision 1.2, Table 5-21 > > BIT(12) claims that Software should re-initialize the xHC when HCE is > > set. Therefore, I think this commit could be the error handling for > > HCE. > > So this does not actually fix an issue that you have seen in any device > or testing? So it is not relevant for older kernels but just "nice to > have"? > > How did you test this if you can not duplicate the problem? > Yes, we actually see that the HCE may be detected while running xhci_resume on our product platform, so I'm able to verify this commit can fix such a condition. For older kernels, I'm not sure whether someone had ever met such issue, but I believe the Run/Stop bit of USBCMD cannot be set once HCE is raised. Thanks. Puma Hsu > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xhci: re-initialize the HC during resume if HCE was set 2021-12-29 9:11 ` Puma Hsu @ 2021-12-29 9:51 ` Greg KH 2021-12-29 10:21 ` Puma Hsu 0 siblings, 1 reply; 10+ messages in thread From: Greg KH @ 2021-12-29 9:51 UTC (permalink / raw) To: Puma Hsu; +Cc: mathias.nyman, Albert Wang, linux-usb, linux-kernel On Wed, Dec 29, 2021 at 05:11:47PM +0800, Puma Hsu wrote: > On Wed, Dec 29, 2021 at 4:30 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > A: http://en.wikipedia.org/wiki/Top_post > > Q: Were do I find info about this thing called top-posting? > > A: Because it messes up the order in which people normally read text. > > Q: Why is top-posting such a bad thing? > > A: Top-posting. > > Q: What is the most annoying thing in e-mail? > > > > A: No. > > Q: Should I include quotations after my reply? > > > > http://daringfireball.net/2007/07/on_top > > > > On Wed, Dec 29, 2021 at 01:53:04PM +0800, Puma Hsu wrote: > > > This commit is not used to fix a specific commit. We find a condition > > > that when XHCI runs the resume process but the HCE flag is set, then > > > the Run/Stop bit of USBCMD cannot be set so that HC would not be > > > enabled. In fact, HC may already meet a problem at this moment. > > > Besides, in xHCI requirements specification revision 1.2, Table 5-21 > > > BIT(12) claims that Software should re-initialize the xHC when HCE is > > > set. Therefore, I think this commit could be the error handling for > > > HCE. > > > > So this does not actually fix an issue that you have seen in any device > > or testing? So it is not relevant for older kernels but just "nice to > > have"? > > > > How did you test this if you can not duplicate the problem? > > > > Yes, we actually see that the HCE may be detected while running xhci_resume > on our product platform, so I'm able to verify this commit can fix > such a condition. Given that your product platform is an older kernel version than 5.17, I think that you also want this in the older kernel releases, so please mark it for stable backporting. thanks, greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xhci: re-initialize the HC during resume if HCE was set 2021-12-29 9:51 ` Greg KH @ 2021-12-29 10:21 ` Puma Hsu 2021-12-29 10:37 ` Greg KH 0 siblings, 1 reply; 10+ messages in thread From: Puma Hsu @ 2021-12-29 10:21 UTC (permalink / raw) To: Greg KH; +Cc: mathias.nyman, Albert Wang, linux-usb, linux-kernel On Wed, Dec 29, 2021 at 5:51 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Wed, Dec 29, 2021 at 05:11:47PM +0800, Puma Hsu wrote: > > On Wed, Dec 29, 2021 at 4:30 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > A: http://en.wikipedia.org/wiki/Top_post > > > Q: Were do I find info about this thing called top-posting? > > > A: Because it messes up the order in which people normally read text. > > > Q: Why is top-posting such a bad thing? > > > A: Top-posting. > > > Q: What is the most annoying thing in e-mail? > > > > > > A: No. > > > Q: Should I include quotations after my reply? > > > > > > http://daringfireball.net/2007/07/on_top > > > > > > On Wed, Dec 29, 2021 at 01:53:04PM +0800, Puma Hsu wrote: > > > > This commit is not used to fix a specific commit. We find a condition > > > > that when XHCI runs the resume process but the HCE flag is set, then > > > > the Run/Stop bit of USBCMD cannot be set so that HC would not be > > > > enabled. In fact, HC may already meet a problem at this moment. > > > > Besides, in xHCI requirements specification revision 1.2, Table 5-21 > > > > BIT(12) claims that Software should re-initialize the xHC when HCE is > > > > set. Therefore, I think this commit could be the error handling for > > > > HCE. > > > > > > So this does not actually fix an issue that you have seen in any device > > > or testing? So it is not relevant for older kernels but just "nice to > > > have"? > > > > > > How did you test this if you can not duplicate the problem? > > > > > > > Yes, we actually see that the HCE may be detected while running xhci_resume > > on our product platform, so I'm able to verify this commit can fix > > such a condition. > > Given that your product platform is an older kernel version than 5.17, I > think that you also want this in the older kernel releases, so please > mark it for stable backporting. > Thank you for advising. Could I know how to do this? Just add "Cc: <stable@vger.kernel.org>" to the commit message? Thanks. Puma Hsu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xhci: re-initialize the HC during resume if HCE was set 2021-12-29 10:21 ` Puma Hsu @ 2021-12-29 10:37 ` Greg KH 0 siblings, 0 replies; 10+ messages in thread From: Greg KH @ 2021-12-29 10:37 UTC (permalink / raw) To: Puma Hsu; +Cc: mathias.nyman, Albert Wang, linux-usb, linux-kernel On Wed, Dec 29, 2021 at 06:21:05PM +0800, Puma Hsu wrote: > On Wed, Dec 29, 2021 at 5:51 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Wed, Dec 29, 2021 at 05:11:47PM +0800, Puma Hsu wrote: > > > On Wed, Dec 29, 2021 at 4:30 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > A: http://en.wikipedia.org/wiki/Top_post > > > > Q: Were do I find info about this thing called top-posting? > > > > A: Because it messes up the order in which people normally read text. > > > > Q: Why is top-posting such a bad thing? > > > > A: Top-posting. > > > > Q: What is the most annoying thing in e-mail? > > > > > > > > A: No. > > > > Q: Should I include quotations after my reply? > > > > > > > > http://daringfireball.net/2007/07/on_top > > > > > > > > On Wed, Dec 29, 2021 at 01:53:04PM +0800, Puma Hsu wrote: > > > > > This commit is not used to fix a specific commit. We find a condition > > > > > that when XHCI runs the resume process but the HCE flag is set, then > > > > > the Run/Stop bit of USBCMD cannot be set so that HC would not be > > > > > enabled. In fact, HC may already meet a problem at this moment. > > > > > Besides, in xHCI requirements specification revision 1.2, Table 5-21 > > > > > BIT(12) claims that Software should re-initialize the xHC when HCE is > > > > > set. Therefore, I think this commit could be the error handling for > > > > > HCE. > > > > > > > > So this does not actually fix an issue that you have seen in any device > > > > or testing? So it is not relevant for older kernels but just "nice to > > > > have"? > > > > > > > > How did you test this if you can not duplicate the problem? > > > > > > > > > > Yes, we actually see that the HCE may be detected while running xhci_resume > > > on our product platform, so I'm able to verify this commit can fix > > > such a condition. > > > > Given that your product platform is an older kernel version than 5.17, I > > think that you also want this in the older kernel releases, so please > > mark it for stable backporting. > > > Thank you for advising. > Could I know how to do this? Just add "Cc: <stable@vger.kernel.org>" > to the commit message? Yes, please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html It should describe this well, if not, please let us know. thanks, greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xhci: re-initialize the HC during resume if HCE was set 2021-12-28 6:02 [PATCH] xhci: re-initialize the HC during resume if HCE was set Puma Hsu 2021-12-28 8:26 ` Greg KH @ 2021-12-28 14:34 ` Sergey Shtylyov 2021-12-29 5:55 ` Puma Hsu 1 sibling, 1 reply; 10+ messages in thread From: Sergey Shtylyov @ 2021-12-28 14:34 UTC (permalink / raw) To: Puma Hsu, mathias.nyman, gregkh; +Cc: albertccwang, linux-usb, linux-kernel Hello! On 12/28/21 9:02 AM, Puma Hsu wrote: > When HCE(Host Controller Error) is set, it means an internal > error condition has been detected. It needs to re-initialize > the HC too. > > Signed-off-by: Puma Hsu <pumahsu@google.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 dc357cabb265..c546d9533410 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -1146,8 +1146,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > temp = readl(&xhci->op_regs->status); > } > > - /* If restore operation fails, re-initialize the HC during resume */ > - if ((temp & STS_SRE) || hibernated) { > + /* If restore operation fails or HC error is detected, re-initialize the HC during resume */ > + if ((temp & STS_SRE) || (temp & STS_HCE) || hibernated) { if ((temp & (STS_SRE | STS_HCE)) || hibernated) { [...] MBR, Sergey ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xhci: re-initialize the HC during resume if HCE was set 2021-12-28 14:34 ` Sergey Shtylyov @ 2021-12-29 5:55 ` Puma Hsu 0 siblings, 0 replies; 10+ messages in thread From: Puma Hsu @ 2021-12-29 5:55 UTC (permalink / raw) To: Sergey Shtylyov Cc: mathias.nyman, Greg KH, Albert Wang, linux-usb, linux-kernel On Tue, Dec 28, 2021 at 10:34 PM Sergey Shtylyov <s.shtylyov@omp.ru> wrote: > > Hello! > > On 12/28/21 9:02 AM, Puma Hsu wrote: > > > When HCE(Host Controller Error) is set, it means an internal > > error condition has been detected. It needs to re-initialize > > the HC too. > > > > Signed-off-by: Puma Hsu <pumahsu@google.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 dc357cabb265..c546d9533410 100644 > > --- a/drivers/usb/host/xhci.c > > +++ b/drivers/usb/host/xhci.c > > @@ -1146,8 +1146,8 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) > > temp = readl(&xhci->op_regs->status); > > } > > > > - /* If restore operation fails, re-initialize the HC during resume */ > > - if ((temp & STS_SRE) || hibernated) { > > + /* If restore operation fails or HC error is detected, re-initialize the HC during resume */ > > + if ((temp & STS_SRE) || (temp & STS_HCE) || hibernated) { > > if ((temp & (STS_SRE | STS_HCE)) || hibernated) { > > [...] > > MBR, Sergey Thanks for advising, I will submit patch v2. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-12-29 10:37 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-12-28 6:02 [PATCH] xhci: re-initialize the HC during resume if HCE was set Puma Hsu 2021-12-28 8:26 ` Greg KH 2021-12-29 5:53 ` Puma Hsu 2021-12-29 8:30 ` Greg KH 2021-12-29 9:11 ` Puma Hsu 2021-12-29 9:51 ` Greg KH 2021-12-29 10:21 ` Puma Hsu 2021-12-29 10:37 ` Greg KH 2021-12-28 14:34 ` Sergey Shtylyov 2021-12-29 5:55 ` Puma Hsu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox