xhci: re-initialize the HC during resume if HCE was set
Subsystems (3)
PATCH v6 1/1 xhci: re-initialize the HC during resume if HCE was set
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.
---
v2: Follow Sergey Shtylyov <[email protected]>'s comment.
v3: Add [email protected] for stable release.
v4: Refine the commit message.
v5: Add a debug log. Follow Mathias Nyman <[email protected]>'s comment.
v6: Fix the missing declaration for str.
drivers/usb/host/xhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/usb/host/xhci.c +5 -2
@@ -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))) {
Signature · 1 line
2.35.0.rc2.247.g8bbb082509-goog
Quoted 32 lines · Puma Hsu
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.
Signed-off-by Puma Hsu <[email protected]>---
v2: Follow Sergey Shtylyov <[email protected]>'s comment.
v3: Add [email protected] for stable release.
v4: Refine the commit message.
v5: Add a debug log. Follow Mathias Nyman <[email protected]>'s comment.
v6: Fix the missing declaration for str.drivers/usb/host/xhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)drivers/usb/host/xhci.c +1
-0@@ -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); }
drivers/usb/host/xhci.c int xhci_resume(struct xhci_hcd *xhci, bool hibernated) · line 1156
Quoted 10 lines
drivers/usb/host/xhci.c +4
-2-/* 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
---
drivers/usb/host/xhci.c +13 -6
@@ -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);
Quoted 50 lines · Mathias Nyman
Quoted 32 lines · Puma Hsu
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.
Signed-off-by Puma Hsu <[email protected]>---
v2: Follow Sergey Shtylyov <[email protected]>'s comment.
v3: Add [email protected] for stable release.
v4: Refine the commit message.
v5: Add a debug log. Follow Mathias Nyman <[email protected]>'s comment.
v6: Fix the missing declaration for str.drivers/usb/host/xhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)drivers/usb/host/xhci.c +1
-0@@ -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); }drivers/usb/host/xhci.c int xhci_resume(struct xhci_hcd *xhci, bool hibernated) · line 1156
Quoted 10 lines
drivers/usb/host/xhci.c +4
-2-/* 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.
Quoted 1 line
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.
Quoted 51 lines
---
drivers/usb/host/xhci.c +13
-6@@ -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);
Quoted 52 lines · Puma Hsu
Quoted 50 lines · Mathias Nyman
Quoted 32 lines · Puma Hsu
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.
Signed-off-by Puma Hsu <[email protected]>---
v2: Follow Sergey Shtylyov <[email protected]>'s comment.
v3: Add [email protected] for stable release.
v4: Refine the commit message.
v5: Add a debug log. Follow Mathias Nyman <[email protected]>'s comment.
v6: Fix the missing declaration for str.drivers/usb/host/xhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)drivers/usb/host/xhci.c +1
-0@@ -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); }drivers/usb/host/xhci.c int xhci_resume(struct xhci_hcd *xhci, bool hibernated) · line 1156
Quoted 10 lines
drivers/usb/host/xhci.c +4
-2-/* 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.
Quoted 6 lines
Quoted 1 line
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
Quoted 54 lines · Mathias Nyman
Quoted 52 lines · Puma Hsu
Quoted 50 lines · Mathias Nyman
Quoted 32 lines · Puma Hsu
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.
Signed-off-by Puma Hsu <[email protected]>---
v2: Follow Sergey Shtylyov <[email protected]>'s comment.
v3: Add [email protected] for stable release.
v4: Refine the commit message.
v5: Add a debug log. Follow Mathias Nyman <[email protected]>'s comment.
v6: Fix the missing declaration for str.drivers/usb/host/xhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)drivers/usb/host/xhci.c +1
-0@@ -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); }drivers/usb/host/xhci.c int xhci_resume(struct xhci_hcd *xhci, bool hibernated) · line 1156
Quoted 10 lines
drivers/usb/host/xhci.c +4
-2-/* 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.
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
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!
Quoted 3 lines
Thanks
-Mathias
Quoted 63 lines · Puma Hsu
Quoted 61 lines · Mathias Nyman
Quoted 58 lines · Puma Hsu
Quoted 50 lines · Mathias Nyman
Quoted 32 lines · Puma Hsu
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.
Signed-off-by Puma Hsu <[email protected]>---
v2: Follow Sergey Shtylyov <[email protected]>'s comment.
v3: Add [email protected] for stable release.
v4: Refine the commit message.
v5: Add a debug log. Follow Mathias Nyman <[email protected]>'s comment.
v6: Fix the missing declaration for str.drivers/usb/host/xhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)drivers/usb/host/xhci.c +1
-0@@ -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); }drivers/usb/host/xhci.c int xhci_resume(struct xhci_hcd *xhci, bool hibernated) · line 1156
Quoted 10 lines
drivers/usb/host/xhci.c +4
-2-/* 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.Quoted 1 line
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.
Quoted 6 lines
Quoted 1 line
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