When HCE(Host Controller Error) is set, it means an internal error condition has been detected. Software needs to re-initialize the HC, so add this check in xhci resume.
Cc: stable@vger.kernel.org Signed-off-by: Puma Hsu pumahsu@google.com --- v2: Follow Sergey Shtylyov s.shtylyov@omp.ru's comment. v3: Add stable@vger.kernel.org for stable release. v4: Refine the commit message. v5: Add a debug log. Follow Mathias Nyman mathias.nyman@linux.intel.com's comment. v6: Fix the missing declaration for str.
drivers/usb/host/xhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dc357cabb265..6f1198068004 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) int retval = 0; bool comp_timer_running = false; bool pending_portevent = false; + char str[XHCI_MSG_MAX];
if (!hcd->state) return 0; @@ -1146,8 +1147,10 @@ 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 | STS_HCE)) || hibernated) { + xhci_warn(xhci, "re-initialize HC during resume, USBSTS:%s\n", + xhci_decode_usbsts(str, temp));
if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && !(xhci_all_ports_seen_u0(xhci))) {
On 29.1.2022 11.30, Puma Hsu wrote:
When HCE(Host Controller Error) is set, it means an internal error condition has been detected. Software needs to re-initialize the HC, so add this check in xhci resume.
Cc: stable@vger.kernel.org Signed-off-by: Puma Hsu pumahsu@google.com
v2: Follow Sergey Shtylyov s.shtylyov@omp.ru's comment. v3: Add stable@vger.kernel.org for stable release. v4: Refine the commit message. v5: Add a debug log. Follow Mathias Nyman mathias.nyman@linux.intel.com's comment. v6: Fix the missing declaration for str.
drivers/usb/host/xhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dc357cabb265..6f1198068004 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) int retval = 0; bool comp_timer_running = false; bool pending_portevent = false;
- char str[XHCI_MSG_MAX];
if (!hcd->state) return 0; @@ -1146,8 +1147,10 @@ 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 | STS_HCE)) || hibernated) {
xhci_warn(xhci, "re-initialize HC during resume, USBSTS:%s\n",xhci_decode_usbsts(str, temp));if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && !(xhci_all_ports_seen_u0(xhci))) {
Ended up modifying this patch a bit more than I first intended, - don't print warning in hibernation case, only error. - maybe using a lot of stack for a debug string isn't really needed. - make sure we read the usbsts register before checking for the HCE bit.
Does the below work for you? If yes, and you agree I'll apply it instead
---
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dc357cabb265..04ec2de158bf 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) int retval = 0; bool comp_timer_running = false; bool pending_portevent = false; + bool reinit_xhc = false;
if (!hcd->state) return 0; @@ -1107,10 +1108,11 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
spin_lock_irq(&xhci->lock); - if ((xhci->quirks & XHCI_RESET_ON_RESUME) || xhci->broken_suspend) - hibernated = true;
- if (!hibernated) { + if (hibernated || xhci->quirks & XHCI_RESET_ON_RESUME || xhci->broken_suspend) + reinit_xhc = true; + + if (!reinit_xhc) { /* * Some controllers might lose power during suspend, so wait * for controller not ready bit to clear, just as in xHC init. @@ -1143,12 +1145,17 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) spin_unlock_irq(&xhci->lock); return -ETIMEDOUT; } - temp = readl(&xhci->op_regs->status); }
- /* If restore operation fails, re-initialize the HC during resume */ - if ((temp & STS_SRE) || hibernated) { + temp = readl(&xhci->op_regs->status); + + /* re-initialize the HC on Restore Error, or Host Controller Error */ + if (temp & (STS_SRE | STS_HCE)) { + reinit_xhc = true; + xhci_warn(xhci, "xHC error in resume, USBSTS 0x%x, Reinit\n", temp); + }
+ if (reinit_xhc) { if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && !(xhci_all_ports_seen_u0(xhci))) { del_timer_sync(&xhci->comp_mode_recovery_timer);
On Thu, Feb 3, 2022 at 3:11 AM Mathias Nyman mathias.nyman@linux.intel.com wrote:
On 29.1.2022 11.30, Puma Hsu wrote:
When HCE(Host Controller Error) is set, it means an internal error condition has been detected. Software needs to re-initialize the HC, so add this check in xhci resume.
Cc: stable@vger.kernel.org Signed-off-by: Puma Hsu pumahsu@google.com
v2: Follow Sergey Shtylyov s.shtylyov@omp.ru's comment. v3: Add stable@vger.kernel.org for stable release. v4: Refine the commit message. v5: Add a debug log. Follow Mathias Nyman mathias.nyman@linux.intel.com's comment. v6: Fix the missing declaration for str.
drivers/usb/host/xhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dc357cabb265..6f1198068004 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) int retval = 0; bool comp_timer_running = false; bool pending_portevent = false;
char str[XHCI_MSG_MAX]; if (!hcd->state) return 0;@@ -1146,8 +1147,10 @@ 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 | STS_HCE)) || hibernated) {xhci_warn(xhci, "re-initialize HC during resume, USBSTS:%s\n",xhci_decode_usbsts(str, temp)); if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && !(xhci_all_ports_seen_u0(xhci))) {Ended up modifying this patch a bit more than I first intended,
- don't print warning in hibernation case, only error.
- maybe using a lot of stack for a debug string isn't really needed.
- make sure we read the usbsts register before checking for the HCE bit.
Does the below work for you? If yes, and you agree I'll apply it instead
Hi Mathias, Yes, your patch works for me, thanks! Will you submit a new patch? or should I update to a new version? Thanks.
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dc357cabb265..04ec2de158bf 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) int retval = 0; bool comp_timer_running = false; bool pending_portevent = false;
bool reinit_xhc = false; if (!hcd->state) return 0;@@ -1107,10 +1108,11 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) set_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
spin_lock_irq(&xhci->lock);
if ((xhci->quirks & XHCI_RESET_ON_RESUME) || xhci->broken_suspend)hibernated = true;if (!hibernated) {
if (hibernated || xhci->quirks & XHCI_RESET_ON_RESUME || xhci->broken_suspend)reinit_xhc = true;if (!reinit_xhc) { /* * Some controllers might lose power during suspend, so wait * for controller not ready bit to clear, just as in xHC init.@@ -1143,12 +1145,17 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) spin_unlock_irq(&xhci->lock); return -ETIMEDOUT; }
temp = readl(&xhci->op_regs->status); }/* If restore operation fails, re-initialize the HC during resume */if ((temp & STS_SRE) || hibernated) {
temp = readl(&xhci->op_regs->status);/* re-initialize the HC on Restore Error, or Host Controller Error */if (temp & (STS_SRE | STS_HCE)) {reinit_xhc = true;xhci_warn(xhci, "xHC error in resume, USBSTS 0x%x, Reinit\n", temp);}if (reinit_xhc) { if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && !(xhci_all_ports_seen_u0(xhci))) { del_timer_sync(&xhci->comp_mode_recovery_timer);
On 8.2.2022 9.11, Puma Hsu wrote:
On Thu, Feb 3, 2022 at 3:11 AM Mathias Nyman mathias.nyman@linux.intel.com wrote:
On 29.1.2022 11.30, Puma Hsu wrote:
When HCE(Host Controller Error) is set, it means an internal error condition has been detected. Software needs to re-initialize the HC, so add this check in xhci resume.
Cc: stable@vger.kernel.org Signed-off-by: Puma Hsu pumahsu@google.com
v2: Follow Sergey Shtylyov s.shtylyov@omp.ru's comment. v3: Add stable@vger.kernel.org for stable release. v4: Refine the commit message. v5: Add a debug log. Follow Mathias Nyman mathias.nyman@linux.intel.com's comment. v6: Fix the missing declaration for str.
drivers/usb/host/xhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dc357cabb265..6f1198068004 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) int retval = 0; bool comp_timer_running = false; bool pending_portevent = false;
char str[XHCI_MSG_MAX]; if (!hcd->state) return 0;@@ -1146,8 +1147,10 @@ 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 | STS_HCE)) || hibernated) {xhci_warn(xhci, "re-initialize HC during resume, USBSTS:%s\n",xhci_decode_usbsts(str, temp)); if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && !(xhci_all_ports_seen_u0(xhci))) {Ended up modifying this patch a bit more than I first intended,
- don't print warning in hibernation case, only error.
- maybe using a lot of stack for a debug string isn't really needed.
- make sure we read the usbsts register before checking for the HCE bit.
Does the below work for you? If yes, and you agree I'll apply it instead
Hi Mathias, Yes, your patch works for me, thanks! Will you submit a new patch? or should I update to a new version? Thanks.
I'll submit it
Thanks -Mathias
On Thu, Feb 10, 2022 at 7:08 PM Mathias Nyman mathias.nyman@linux.intel.com wrote:
On 8.2.2022 9.11, Puma Hsu wrote:
On Thu, Feb 3, 2022 at 3:11 AM Mathias Nyman mathias.nyman@linux.intel.com wrote:
On 29.1.2022 11.30, Puma Hsu wrote:
When HCE(Host Controller Error) is set, it means an internal error condition has been detected. Software needs to re-initialize the HC, so add this check in xhci resume.
Cc: stable@vger.kernel.org Signed-off-by: Puma Hsu pumahsu@google.com
v2: Follow Sergey Shtylyov s.shtylyov@omp.ru's comment. v3: Add stable@vger.kernel.org for stable release. v4: Refine the commit message. v5: Add a debug log. Follow Mathias Nyman mathias.nyman@linux.intel.com's comment. v6: Fix the missing declaration for str.
drivers/usb/host/xhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dc357cabb265..6f1198068004 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) int retval = 0; bool comp_timer_running = false; bool pending_portevent = false;
char str[XHCI_MSG_MAX]; if (!hcd->state) return 0;@@ -1146,8 +1147,10 @@ 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 | STS_HCE)) || hibernated) {xhci_warn(xhci, "re-initialize HC during resume, USBSTS:%s\n",xhci_decode_usbsts(str, temp)); if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && !(xhci_all_ports_seen_u0(xhci))) {Ended up modifying this patch a bit more than I first intended,
- don't print warning in hibernation case, only error.
- maybe using a lot of stack for a debug string isn't really needed.
- make sure we read the usbsts register before checking for the HCE bit.
Does the below work for you? If yes, and you agree I'll apply it instead
Hi Mathias, Yes, your patch works for me, thanks! Will you submit a new patch? or should I update to a new version? Thanks.
I'll submit it
Hi Mathias, Could I know when you will submit the new patch? We will sync the patch to our project. Thank you for your effort!
Thanks -Mathias
On 15.2.2022 14.05, Puma Hsu wrote:
On Thu, Feb 10, 2022 at 7:08 PM Mathias Nyman mathias.nyman@linux.intel.com wrote:
On 8.2.2022 9.11, Puma Hsu wrote:
On Thu, Feb 3, 2022 at 3:11 AM Mathias Nyman mathias.nyman@linux.intel.com wrote:
On 29.1.2022 11.30, Puma Hsu wrote:
When HCE(Host Controller Error) is set, it means an internal error condition has been detected. Software needs to re-initialize the HC, so add this check in xhci resume.
Cc: stable@vger.kernel.org Signed-off-by: Puma Hsu pumahsu@google.com
v2: Follow Sergey Shtylyov s.shtylyov@omp.ru's comment. v3: Add stable@vger.kernel.org for stable release. v4: Refine the commit message. v5: Add a debug log. Follow Mathias Nyman mathias.nyman@linux.intel.com's comment. v6: Fix the missing declaration for str.
drivers/usb/host/xhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dc357cabb265..6f1198068004 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1091,6 +1091,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) int retval = 0; bool comp_timer_running = false; bool pending_portevent = false;
char str[XHCI_MSG_MAX]; if (!hcd->state) return 0;@@ -1146,8 +1147,10 @@ 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 | STS_HCE)) || hibernated) {xhci_warn(xhci, "re-initialize HC during resume, USBSTS:%s\n",xhci_decode_usbsts(str, temp)); if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) && !(xhci_all_ports_seen_u0(xhci))) {Ended up modifying this patch a bit more than I first intended,
- don't print warning in hibernation case, only error.
- maybe using a lot of stack for a debug string isn't really needed.
- make sure we read the usbsts register before checking for the HCE bit.
Does the below work for you? If yes, and you agree I'll apply it instead
Hi Mathias, Yes, your patch works for me, thanks! Will you submit a new patch? or should I update to a new version? Thanks.
I'll submit it
Hi Mathias, Could I know when you will submit the new patch? We will sync the patch to our project. Thank you for your effort!
Just did, thanks for the reminder.
-Mathias
linux-stable-mirror@lists.linaro.org
-
Mathias Nyman -
Puma Hsu