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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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
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
• [email protected]
On Tue, Dec 28, 2021 at 4:26 PM Greg KH <[email protected]> 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 <[email protected]>
> > ---
> > 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
On Tue, Dec 28, 2021 at 10:34 PM Sergey Shtylyov <[email protected]> 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 <[email protected]>
> > ---
> > 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.
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
On Wed, Dec 29, 2021 at 4:30 PM Greg KH <[email protected]> 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
On Wed, Dec 29, 2021 at 05:11:47PM +0800, Puma Hsu wrote:
> On Wed, Dec 29, 2021 at 4:30 PM Greg KH <[email protected]> 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
On Wed, Dec 29, 2021 at 5:51 PM Greg KH <[email protected]> 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 <[email protected]> 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: <[email protected]>"
to the commit message?
Thanks.
Puma Hsu
On Wed, Dec 29, 2021 at 06:21:05PM +0800, Puma Hsu wrote:
> On Wed, Dec 29, 2021 at 5:51 PM Greg KH <[email protected]> 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 <[email protected]> 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: <[email protected]>"
> 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